drm/dp: Add backpointer to drm_device in drm_dp_aux
authorLyude Paul <lyude@redhat.com>
Fri, 23 Apr 2021 18:42:55 +0000 (14:42 -0400)
committerLyude Paul <lyude@redhat.com>
Tue, 27 Apr 2021 22:43:42 +0000 (18:43 -0400)
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.

v3:
* Add WARN_ON_ONCE() to drm_dp_aux_register() if drm_dev isn't filled out

Signed-off-by: Lyude Paul <lyude@redhat.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210423184309.207645-4-lyude@redhat.com
Reviewed-by: Dave Airlie <airlied@redhat.com>
20 files changed:
drivers/gpu/drm/amd/amdgpu/atombios_dp.c
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
drivers/gpu/drm/bridge/tc358767.c
drivers/gpu/drm/bridge/ti-sn65dsi86.c
drivers/gpu/drm/drm_dp_aux_dev.c
drivers/gpu/drm/drm_dp_helper.c
drivers/gpu/drm/drm_dp_mst_topology.c
drivers/gpu/drm/i915/display/intel_dp_aux.c
drivers/gpu/drm/msm/edp/edp.h
drivers/gpu/drm/msm/edp/edp_aux.c
drivers/gpu/drm/msm/edp/edp_ctrl.c
drivers/gpu/drm/nouveau/nouveau_connector.c
drivers/gpu/drm/radeon/atombios_dp.c
drivers/gpu/drm/tegra/dpaux.c
drivers/gpu/drm/xlnx/zynqmp_dp.c
include/drm/drm_dp_helper.h

index a3ba9ca..062625a 100644 (file)
@@ -188,6 +188,8 @@ 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(&amdgpu_connector->ddc_bus->aux);
        amdgpu_connector->ddc_bus->has_aux = true;
 }
index 73cdb9f..997567f 100644 (file)
@@ -433,6 +433,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
                kasprintf(GFP_KERNEL, "AMDGPU DM aux hw bus %d",
                          link_index);
        aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer;
+       aconnector->dm_dp_aux.aux.drm_dev = dm->ddev;
        aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc;
 
        drm_dp_aux_init(&aconnector->dm_dp_aux.aux);
index aa6cda4..e33cd07 100644 (file)
@@ -537,6 +537,7 @@ static int anx6345_bridge_attach(struct drm_bridge *bridge,
        /* Register aux channel */
        anx6345->aux.name = "DP-AUX";
        anx6345->aux.dev = &anx6345->client->dev;
+       anx6345->aux.drm_dev = bridge->dev;
        anx6345->aux.transfer = anx6345_aux_transfer;
 
        err = drm_dp_aux_register(&anx6345->aux);
index f205586..5e6a0ed 100644 (file)
@@ -905,6 +905,7 @@ static int anx78xx_bridge_attach(struct drm_bridge *bridge,
        /* Register aux channel */
        anx78xx->aux.name = "DP-AUX";
        anx78xx->aux.dev = &anx78xx->client->dev;
+       anx78xx->aux.drm_dev = bridge->dev;
        anx78xx->aux.transfer = anx78xx_aux_transfer;
 
        err = drm_dp_aux_register(&anx78xx->aux);
index f115233..550814c 100644 (file)
@@ -1765,6 +1765,7 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
        dp->aux.name = "DP-AUX";
        dp->aux.transfer = analogix_dpaux_transfer;
        dp->aux.dev = dp->dev;
+       dp->aux.drm_dev = drm_dev;
 
        ret = drm_dp_aux_register(&dp->aux);
        if (ret)
index 49e4c34..0cd8f40 100644 (file)
@@ -1719,6 +1719,7 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
 
        dev_dbg(mhdp->dev, "%s\n", __func__);
 
+       mhdp->aux.drm_dev = bridge->dev;
        ret = drm_dp_aux_register(&mhdp->aux);
        if (ret < 0)
                return ret;
index da89922..23a6f90 100644 (file)
@@ -1414,6 +1414,7 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
        if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
                return 0;
 
+       tc->aux.drm_dev = drm;
        ret = drm_dp_aux_register(&tc->aux);
        if (ret < 0)
                return ret;
index 51db30d..5670671 100644 (file)
@@ -350,6 +350,7 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
                return -EINVAL;
        }
 
+       pdata->aux.drm_dev = bridge->dev;
        ret = drm_dp_aux_register(&pdata->aux);
        if (ret < 0) {
                drm_err(bridge->dev, "Failed to register DP AUX channel: %d\n", ret);
index e25181b..06b374c 100644 (file)
@@ -278,6 +278,12 @@ void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
        if (!aux_dev) /* attach must have failed */
                return;
 
+       /*
+        * As some AUX adapters may exist as platform devices which outlive their respective DRM
+        * devices, we clear drm_dev to ensure that we never accidentally reference a stale pointer
+        */
+       aux->drm_dev = NULL;
+
        mutex_lock(&aux_idr_mutex);
        idr_remove(&aux_idr, aux_dev->index);
        mutex_unlock(&aux_idr_mutex);
index cb2f53e..ad73d72 100644 (file)
@@ -1767,6 +1767,8 @@ int drm_dp_aux_register(struct drm_dp_aux *aux)
 {
        int ret;
 
+       WARN_ON_ONCE(!aux->drm_dev);
+
        if (!aux->ddc.algo)
                drm_dp_aux_init(aux);
 
index 1590144..276f7f0 100644 (file)
@@ -2350,6 +2350,7 @@ drm_dp_mst_add_port(struct drm_device *dev,
        port->aux.is_remote = true;
 
        /* initialize the MST downstream port's AUX crc work queue */
+       port->aux.drm_dev = dev;
        drm_dp_remote_aux_init(&port->aux);
 
        /*
index 7e83bc2..c4b446d 100644 (file)
@@ -682,6 +682,7 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
        else
                intel_dp->get_aux_send_ctl = g4x_get_aux_send_ctl;
 
+       intel_dp->aux.drm_dev = &dev_priv->drm;
        drm_dp_aux_init(&intel_dp->aux);
 
        /* Failure to allocate our preferred name is not critical */
index eb34243..8590f2c 100644 (file)
@@ -46,8 +46,7 @@ void edp_bridge_destroy(struct drm_bridge *bridge);
 struct drm_connector *msm_edp_connector_init(struct msm_edp *edp);
 
 /* AUX */
-void *msm_edp_aux_init(struct device *dev, void __iomem *regbase,
-                       struct drm_dp_aux **drm_aux);
+void *msm_edp_aux_init(struct msm_edp *edp, void __iomem *regbase, struct drm_dp_aux **drm_aux);
 void msm_edp_aux_destroy(struct device *dev, struct edp_aux *aux);
 irqreturn_t msm_edp_aux_irq(struct edp_aux *aux, u32 isr);
 void msm_edp_aux_ctrl(struct edp_aux *aux, int enable);
index df10a01..e3d85c6 100644 (file)
@@ -184,9 +184,9 @@ unlock_exit:
        return ret;
 }
 
-void *msm_edp_aux_init(struct device *dev, void __iomem *regbase,
-       struct drm_dp_aux **drm_aux)
+void *msm_edp_aux_init(struct msm_edp *edp, void __iomem *regbase, struct drm_dp_aux **drm_aux)
 {
+       struct device *dev = &edp->pdev->dev;
        struct edp_aux *aux = NULL;
        int ret;
 
@@ -201,6 +201,7 @@ void *msm_edp_aux_init(struct device *dev, void __iomem *regbase,
 
        aux->drm_aux.name = "msm_edp_aux";
        aux->drm_aux.dev = dev;
+       aux->drm_aux.drm_dev = edp->dev;
        aux->drm_aux.transfer = edp_aux_transfer;
        ret = drm_dp_aux_register(&aux->drm_aux);
        if (ret) {
index 0d9657c..57af3d8 100644 (file)
@@ -1153,7 +1153,7 @@ int msm_edp_ctrl_init(struct msm_edp *edp)
        }
 
        /* Init aux and phy */
-       ctrl->aux = msm_edp_aux_init(dev, ctrl->base, &ctrl->drm_aux);
+       ctrl->aux = msm_edp_aux_init(edp, ctrl->base, &ctrl->drm_aux);
        if (!ctrl->aux || !ctrl->drm_aux) {
                pr_err("%s:failed to init aux\n", __func__);
                return -ENOMEM;
index c04044b..7f38788 100644 (file)
@@ -1354,6 +1354,7 @@ nouveau_connector_create(struct drm_device *dev,
        case DRM_MODE_CONNECTOR_DisplayPort:
        case DRM_MODE_CONNECTOR_eDP:
                nv_connector->aux.dev = connector->kdev;
+               nv_connector->aux.drm_dev = dev;
                nv_connector->aux.transfer = nouveau_connector_aux_xfer;
                snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
                         dcbe->hasht, dcbe->hashm);
index 15b00a3..c50c504 100644 (file)
@@ -232,6 +232,7 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
 
        radeon_connector->ddc_bus->rec.hpd = radeon_connector->hpd.hpd;
        radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
+       radeon_connector->ddc_bus->aux.drm_dev = radeon_connector->base.dev;
        if (ASIC_IS_DCE5(rdev)) {
                if (radeon_auxch)
                        radeon_connector->ddc_bus->aux.transfer = radeon_dp_aux_transfer_native;
index ea56c6e..7d7cc90 100644 (file)
@@ -719,6 +719,7 @@ int drm_dp_aux_attach(struct drm_dp_aux *aux, struct tegra_output *output)
        unsigned long timeout;
        int err;
 
+       aux->drm_dev = output->connector.dev;
        err = drm_dp_aux_register(aux);
        if (err < 0)
                return err;
index 59d1fb0..7e5e893 100644 (file)
@@ -1069,6 +1069,7 @@ static int zynqmp_dp_aux_init(struct zynqmp_dp *dp)
 
        dp->aux.name = "ZynqMP DP AUX";
        dp->aux.dev = dp->dev;
+       dp->aux.drm_dev = dp->drm;
        dp->aux.transfer = zynqmp_dp_aux_transfer;
 
        return drm_dp_aux_register(&dp->aux);
index 1e85c20..95efe37 100644 (file)
@@ -1840,6 +1840,8 @@ struct drm_dp_aux_cec {
  * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
  * @ddc: I2C adapter that can be used for I2C-over-AUX communication
  * @dev: pointer to struct device that is the parent for this AUX channel
+ * @drm_dev: pointer to the &drm_device that owns this AUX channel. Beware, this
+ * may be %NULL before drm_dp_aux_register() has been called.
  * @crtc: backpointer to the crtc that is currently using this AUX channel
  * @hw_mutex: internal mutex used for locking transfers
  * @crc_work: worker that captures CRCs for each frame
@@ -1847,7 +1849,11 @@ struct drm_dp_aux_cec {
  * @transfer: transfers a message representing a single AUX transaction
  *
  * The @dev field should be set to a pointer to the device that implements the
- * AUX channel.
+ * AUX channel. As well, the @drm_dev field should be set to the &drm_device
+ * that will be using this AUX channel as early as possible. For many graphics
+ * drivers this should happen before drm_dp_aux_init(), however it's perfectly
+ * fine to set this field later so long as it's assigned before calling
+ * drm_dp_aux_register().
  *
  * The @name field may be used to specify the name of the I2C adapter. If set to
  * %NULL, dev_name() of @dev will be used.
@@ -1879,6 +1885,7 @@ struct drm_dp_aux {
        const char *name;
        struct i2c_adapter ddc;
        struct device *dev;
+       struct drm_device *drm_dev;
        struct drm_crtc *crtc;
        struct mutex hw_mutex;
        struct work_struct crc_work;