Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210423184309.207645-1-lyude@redhat.com> <20210423184309.207645-3-lyude@redhat.com>
In-Reply-To: <20210423184309.207645-3-lyude@redhat.com>
From:   Ilia Mirkin <imirkin@alum.mit.edu>
Date:   Fri, 23 Apr 2021 17:07:04 -0400
Message-ID: <CAKb7UvgpMO60gRzmJY0V5nOsT8u9y2hFLazmVJ+pEiedEKOOhQ@mail.gmail.com>
Subject: Re: [Nouveau] [PATCH v4 02/17] drm/nouveau/kms/nv50-: Move AUX
 adapter reg to connector late register/early unregister
To:     Lyude Paul <lyude@redhat.com>
Cc:     dri-devel <dri-devel@lists.freedesktop.org>,
        Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
        nouveau <nouveau@lists.freedesktop.org>,
        amd-gfx mailing list <amd-gfx@lists.freedesktop.org>,
        =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= <ville.syrjala@linux.intel.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Thierry Reding <thierry.reding@gmail.com>,
        David Airlie <airlied@linux.ie>,
        Ben Skeggs <bskeggs@redhat.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Some trivia, no comment on the real logic of the changes:

On Fri, Apr 23, 2021 at 2:43 PM Lyude Paul <lyude@redhat.com> wrote:
>
> Since AUX adapters on nouveau have their respective DRM connectors as
> parents, we need to make sure that we register then after their connectors.

then -> them

>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 25 ++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 61e6d7412505..c04044be3d32 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -401,7 +401,6 @@ nouveau_connector_destroy(struct drm_connector *connector)
>         drm_connector_cleanup(connector);
>         if (nv_connector->aux.transfer) {
>                 drm_dp_cec_unregister_connector(&nv_connector->aux);
> -               drm_dp_aux_unregister(&nv_connector->aux);
>                 kfree(nv_connector->aux.name);
>         }
>         kfree(connector);
> @@ -905,13 +904,29 @@ nouveau_connector_late_register(struct drm_connector *connector)
>         int ret;
>
>         ret = nouveau_backlight_init(connector);
> +       if (ret)
> +               return ret;
>
> +       if (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> +           connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> +               ret = drm_dp_aux_register(&nouveau_connector(connector)->aux);
> +               if (ret)
> +                       goto backlight_fini;
> +       }
> +
> +       return 0;
> +backlight_fini:
> +       nouveau_backlight_fini(connector);
>         return ret;
>  }
>
>  static void
>  nouveau_connector_early_unregister(struct drm_connector *connector)
>  {
> +       if (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> +           connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
> +               drm_dp_aux_unregister(&nouveau_connector(connector)->aux);
> +
>         nouveau_backlight_fini(connector);
>  }
>
> @@ -1343,14 +1358,14 @@ nouveau_connector_create(struct drm_device *dev,
>                 snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
>                          dcbe->hasht, dcbe->hashm);
>                 nv_connector->aux.name = kstrdup(aux_name, GFP_KERNEL);
> -               ret = drm_dp_aux_register(&nv_connector->aux);
> +               drm_dp_aux_init(&nv_connector->aux);
>                 if (ret) {
> -                       NV_ERROR(drm, "failed to register aux channel\n");
> +                       NV_ERROR(drm, "Failed to init AUX adapter for sor-%04x-%04x: %d\n",

Maybe just use aux_name instead of rebuilding the string again?

> +                                dcbe->hasht, dcbe->hashm, ret);
>                         kfree(nv_connector);
>                         return ERR_PTR(ret);
>                 }
> -               funcs = &nouveau_connector_funcs;
> -               break;
> +               fallthrough;
>         default:
>                 funcs = &nouveau_connector_funcs;
>                 break;
> --
> 2.30.2
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau