Re: [PATCH 15/30] drm/dp: Add backpointer to drm_device in drm_dp_aux
On Sun, 2021-02-21 at 20:21 +0200, Laurent Pinchart wrote: > Hi Lyude, > > Thank you for the patch. > > On Fri, Feb 19, 2021 at 04:53:11PM -0500, Lyude Paul wrote: > > This is something that we've wanted for a while now: the ability to > > actually look up the respective drm_device for a given drm_dp_aux struct. > > This will also allow us to transition over to using the drm_dbg_*() helpers > > for debug message printing, as we'll finally have a drm_device to reference > > for doing so. > > Isn't it better to use the existing dev field ? If you have multiple DP > outputs for one DRM device, using the DRM device name in debug messages > won't tell which output the message corresponds to. Well the thing is dev won't tell you that either - on most SoCs the DP AUX adapter is it's own platform device that isn't a child of the DRM device. Also, that's what the ability to name AUX channels is supposed to be there for. > > Note that there is one limitation with this - because some DP AUX adapters > > exist as platform devices which are initialized independently of their > > respective DRM devices, one cannot rely on drm_dp_aux->drm_dev to always be > > non-NULL until drm_dp_aux_register() has been called. We make sure to point > > this out in the documentation for struct drm_dp_aux. > > > > Signed-off-by: Lyude Paul > > --- > > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 1 + > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 1 + > > drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 1 + > > drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 1 + > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + > > drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 1 + > > drivers/gpu/drm/bridge/tc358767.c | 1 + > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 1 + > > drivers/gpu/drm/drm_dp_aux_dev.c | 6 ++ > > drivers/gpu/drm/drm_dp_mst_topology.c | 1 + > > drivers/gpu/drm/i915/display/intel_dp_aux.c | 1 + > > drivers/gpu/drm/msm/edp/edp.h | 3 +-- > > drivers/gpu/drm/msm/edp/edp_aux.c | 5 +++-- > > drivers/gpu/drm/msm/edp/edp_ctrl.c | 2 +- > > drivers/gpu/drm/nouveau/nouveau_connector.c | 1 + > > drivers/gpu/drm/radeon/atombios_dp.c | 1 + > > drivers/gpu/drm/tegra/dpaux.c | 1 + > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 1 + > > include/drm/drm_dp_helper.h | 9 - > > 19 files changed, 33 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > > b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > > index a3ba9ca11e98..6d35da65e09f 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > > @@ -188,6 +188,7 @@ void amdgpu_atombios_dp_aux_init(struct amdgpu_connector > > *amdgpu_connector) > > { > > amdgpu_connector->ddc_bus->rec.hpd = amdgpu_connector->hpd.hpd; > > amdgpu_connector->ddc_bus->aux.transfer = > > amdgpu_atombios_dp_aux_transfer; > > + amdgpu_connector->ddc_bus->aux.drm_dev = amdgpu_connector->base.dev; > > drm_dp_aux_init(_connector->ddc_bus->aux); > > amdgpu_connector->ddc_bus->has_aux = true; > > } > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > index 41b09ab22233..163641b44339 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > @@ -431,6 +431,7 @@ void amdgpu_dm_initialize_dp_connector(struct > > amdgpu_display_manager *dm, > > link_index); > > aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; > > aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; > > + aconnector->dm_dp_aux.aux.drm_dev = dm->ddev; > > > > drm_dp_aux_init(>dm_dp_aux.aux); > > drm_dp_cec_register_connector(>dm_dp_aux.aux, > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > > b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > > index aa6cda458eb9..e33cd077595a 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > > @@ -537,6 +537,7 @@ static int anx6345_bridge_attach(struct drm_bridge > > *bridge, > > /* Register aux channel */ > > anx6345->aux.name = "DP-AUX"; > > anx6345->aux.dev = >client->dev; > > + anx6345->aux.drm_dev = bridge->dev; > > anx6345->aux.transfer = anx6345_aux_transfer; > > > > err = drm_dp_aux_register(>aux); > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c > >
Re: [PATCH 15/30] drm/dp: Add backpointer to drm_device in drm_dp_aux
Hi Lyude, Thank you for the patch. On Fri, Feb 19, 2021 at 04:53:11PM -0500, Lyude Paul wrote: > This is something that we've wanted for a while now: the ability to > actually look up the respective drm_device for a given drm_dp_aux struct. > This will also allow us to transition over to using the drm_dbg_*() helpers > for debug message printing, as we'll finally have a drm_device to reference > for doing so. Isn't it better to use the existing dev field ? If you have multiple DP outputs for one DRM device, using the DRM device name in debug messages won't tell which output the message corresponds to. > Note that there is one limitation with this - because some DP AUX adapters > exist as platform devices which are initialized independently of their > respective DRM devices, one cannot rely on drm_dp_aux->drm_dev to always be > non-NULL until drm_dp_aux_register() has been called. We make sure to point > this out in the documentation for struct drm_dp_aux. > > Signed-off-by: Lyude Paul > --- > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 1 + > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 1 + > drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 1 + > drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 1 + > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + > drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 1 + > drivers/gpu/drm/bridge/tc358767.c| 1 + > drivers/gpu/drm/bridge/ti-sn65dsi86.c| 1 + > drivers/gpu/drm/drm_dp_aux_dev.c | 6 ++ > drivers/gpu/drm/drm_dp_mst_topology.c| 1 + > drivers/gpu/drm/i915/display/intel_dp_aux.c | 1 + > drivers/gpu/drm/msm/edp/edp.h| 3 +-- > drivers/gpu/drm/msm/edp/edp_aux.c| 5 +++-- > drivers/gpu/drm/msm/edp/edp_ctrl.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_connector.c | 1 + > drivers/gpu/drm/radeon/atombios_dp.c | 1 + > drivers/gpu/drm/tegra/dpaux.c| 1 + > drivers/gpu/drm/xlnx/zynqmp_dp.c | 1 + > include/drm/drm_dp_helper.h | 9 - > 19 files changed, 33 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > index a3ba9ca11e98..6d35da65e09f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c > @@ -188,6 +188,7 @@ void amdgpu_atombios_dp_aux_init(struct amdgpu_connector > *amdgpu_connector) > { > amdgpu_connector->ddc_bus->rec.hpd = amdgpu_connector->hpd.hpd; > amdgpu_connector->ddc_bus->aux.transfer = > amdgpu_atombios_dp_aux_transfer; > + amdgpu_connector->ddc_bus->aux.drm_dev = amdgpu_connector->base.dev; > drm_dp_aux_init(_connector->ddc_bus->aux); > amdgpu_connector->ddc_bus->has_aux = true; > } > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 41b09ab22233..163641b44339 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -431,6 +431,7 @@ void amdgpu_dm_initialize_dp_connector(struct > amdgpu_display_manager *dm, > link_index); > aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; > aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; > + aconnector->dm_dp_aux.aux.drm_dev = dm->ddev; > > drm_dp_aux_init(>dm_dp_aux.aux); > drm_dp_cec_register_connector(>dm_dp_aux.aux, > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > index aa6cda458eb9..e33cd077595a 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > @@ -537,6 +537,7 @@ static int anx6345_bridge_attach(struct drm_bridge > *bridge, > /* Register aux channel */ > anx6345->aux.name = "DP-AUX"; > anx6345->aux.dev = >client->dev; > + anx6345->aux.drm_dev = bridge->dev; > anx6345->aux.transfer = anx6345_aux_transfer; > > err = drm_dp_aux_register(>aux); > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c > b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c > index f20558618220..5e6a0ed39199 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c > @@ -905,6 +905,7 @@ static int anx78xx_bridge_attach(struct drm_bridge > *bridge, > /* Register aux channel */ > anx78xx->aux.name = "DP-AUX"; > anx78xx->aux.dev = >client->dev; > + anx78xx->aux.drm_dev = bridge->dev; > anx78xx->aux.transfer = anx78xx_aux_transfer; > > err =
[PATCH 15/30] drm/dp: Add backpointer to drm_device in drm_dp_aux
This is something that we've wanted for a while now: the ability to actually look up the respective drm_device for a given drm_dp_aux struct. This will also allow us to transition over to using the drm_dbg_*() helpers for debug message printing, as we'll finally have a drm_device to reference for doing so. Note that there is one limitation with this - because some DP AUX adapters exist as platform devices which are initialized independently of their respective DRM devices, one cannot rely on drm_dp_aux->drm_dev to always be non-NULL until drm_dp_aux_register() has been called. We make sure to point this out in the documentation for struct drm_dp_aux. Signed-off-by: Lyude Paul --- drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 1 + .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 1 + drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 1 + drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 1 + drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 1 + drivers/gpu/drm/bridge/tc358767.c| 1 + drivers/gpu/drm/bridge/ti-sn65dsi86.c| 1 + drivers/gpu/drm/drm_dp_aux_dev.c | 6 ++ drivers/gpu/drm/drm_dp_mst_topology.c| 1 + drivers/gpu/drm/i915/display/intel_dp_aux.c | 1 + drivers/gpu/drm/msm/edp/edp.h| 3 +-- drivers/gpu/drm/msm/edp/edp_aux.c| 5 +++-- drivers/gpu/drm/msm/edp/edp_ctrl.c | 2 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 1 + drivers/gpu/drm/radeon/atombios_dp.c | 1 + drivers/gpu/drm/tegra/dpaux.c| 1 + drivers/gpu/drm/xlnx/zynqmp_dp.c | 1 + include/drm/drm_dp_helper.h | 9 - 19 files changed, 33 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c index a3ba9ca11e98..6d35da65e09f 100644 --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c @@ -188,6 +188,7 @@ void amdgpu_atombios_dp_aux_init(struct amdgpu_connector *amdgpu_connector) { amdgpu_connector->ddc_bus->rec.hpd = amdgpu_connector->hpd.hpd; amdgpu_connector->ddc_bus->aux.transfer = amdgpu_atombios_dp_aux_transfer; + amdgpu_connector->ddc_bus->aux.drm_dev = amdgpu_connector->base.dev; drm_dp_aux_init(_connector->ddc_bus->aux); amdgpu_connector->ddc_bus->has_aux = true; } diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 41b09ab22233..163641b44339 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -431,6 +431,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, link_index); aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; + aconnector->dm_dp_aux.aux.drm_dev = dm->ddev; drm_dp_aux_init(>dm_dp_aux.aux); drm_dp_cec_register_connector(>dm_dp_aux.aux, diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c index aa6cda458eb9..e33cd077595a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c @@ -537,6 +537,7 @@ static int anx6345_bridge_attach(struct drm_bridge *bridge, /* Register aux channel */ anx6345->aux.name = "DP-AUX"; anx6345->aux.dev = >client->dev; + anx6345->aux.drm_dev = bridge->dev; anx6345->aux.transfer = anx6345_aux_transfer; err = drm_dp_aux_register(>aux); diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c index f20558618220..5e6a0ed39199 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c @@ -905,6 +905,7 @@ static int anx78xx_bridge_attach(struct drm_bridge *bridge, /* Register aux channel */ anx78xx->aux.name = "DP-AUX"; anx78xx->aux.dev = >client->dev; + anx78xx->aux.drm_dev = bridge->dev; anx78xx->aux.transfer = anx78xx_aux_transfer; err = drm_dp_aux_register(>aux); diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index f115233b1cb9..550814ca2139 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1765,6 +1765,7 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev) dp->aux.name =