* [PATCH v2 02/20] drm/tegra: Don't register DP AUX channels before connectors
[not found] <20210326203807.105754-1-lyude@redhat.com>
@ 2021-03-26 20:37 ` Lyude Paul
2021-03-29 6:51 ` Thierry Reding
2021-03-26 20:37 ` [PATCH v2 05/20] drm/dp: Add backpointer to drm_device in drm_dp_aux Lyude Paul
1 sibling, 1 reply; 5+ messages in thread
From: Lyude Paul @ 2021-03-26 20:37 UTC (permalink / raw)
To: nouveau, dri-devel, amd-gfx, intel-gfx
Cc: Thierry Reding, David Airlie, Daniel Vetter, Jonathan Hunter,
open list:DRM DRIVERS FOR NVIDIA TEGRA, open list
As pointed out by the documentation for drm_dp_aux_register(),
drm_dp_aux_init() should be used in situations where the AUX channel for a
display driver can potentially be registered before it's respective DRM
driver. This is the case with Tegra, since the DP aux channel exists as a
platform device instead of being a grandchild of the DRM device.
Since we're about to add a backpointer to a DP AUX channel's respective DRM
device, let's fix this so that we don't potentially allow userspace to use
the AUX channel before we've associated it with it's DRM connector.
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/tegra/dpaux.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 105fb9cdbb3b..ea56c6ec25e4 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -534,9 +534,7 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
dpaux->aux.transfer = tegra_dpaux_transfer;
dpaux->aux.dev = &pdev->dev;
- err = drm_dp_aux_register(&dpaux->aux);
- if (err < 0)
- return err;
+ drm_dp_aux_init(&dpaux->aux);
/*
* Assume that by default the DPAUX/I2C pads will be used for HDMI,
@@ -589,8 +587,6 @@ static int tegra_dpaux_remove(struct platform_device *pdev)
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
- drm_dp_aux_unregister(&dpaux->aux);
-
mutex_lock(&dpaux_lock);
list_del(&dpaux->list);
mutex_unlock(&dpaux_lock);
@@ -723,6 +719,10 @@ int drm_dp_aux_attach(struct drm_dp_aux *aux, struct tegra_output *output)
unsigned long timeout;
int err;
+ err = drm_dp_aux_register(aux);
+ if (err < 0)
+ return err;
+
output->connector.polled = DRM_CONNECTOR_POLL_HPD;
dpaux->output = output;
@@ -760,6 +760,7 @@ int drm_dp_aux_detach(struct drm_dp_aux *aux)
unsigned long timeout;
int err;
+ drm_dp_aux_unregister(aux);
disable_irq(dpaux->irq);
if (dpaux->output->panel) {
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 05/20] drm/dp: Add backpointer to drm_device in drm_dp_aux
[not found] <20210326203807.105754-1-lyude@redhat.com>
2021-03-26 20:37 ` [PATCH v2 02/20] drm/tegra: Don't register DP AUX channels before connectors Lyude Paul
@ 2021-03-26 20:37 ` Lyude Paul
2021-03-29 7:08 ` Thierry Reding
2021-04-01 13:31 ` Jani Nikula
1 sibling, 2 replies; 5+ messages in thread
From: Lyude Paul @ 2021-03-26 20:37 UTC (permalink / raw)
To: nouveau, dri-devel, amd-gfx, intel-gfx
Cc: Alex Deucher, Christian König, David Airlie, Daniel Vetter,
Harry Wentland, Leo Li, Andrzej Hajda, Neil Armstrong,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Rob Clark, Sean Paul, Ben Skeggs,
Thierry Reding, Jonathan Hunter, Hyun Kwon, Michal Simek,
Luben Tuikov, Oleg Vasilev, Mikita Lipski, Eryk Brol,
Rodrigo Siqueira, Jerry (Fangzhi) Zuo, Chris Park, Sam Ravnborg,
Joe Perches, Vasily Khoruzhick, Guido Günther,
Boris Brezillon, Andy Yan, Marek Szyprowski, Tomi Valkeinen,
Swapnil Jakhade, Yuti Amonkar, Jyri Sarha, Julia Lawall,
Ville Syrjälä, Imre Deak, Matt Roper, Lucas De Marchi,
open list, open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVERS FOR NVIDIA TEGRA,
moderated list:ARM/ZYNQ ARCHITECTURE
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 <lyude@redhat.com>
---
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(&amdgpu_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 73cdb9fe981a..b8b69c33fb2b 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
@@ -434,6 +434,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(&aconnector->dm_dp_aux.aux);
drm_dp_cec_register_connector(&aconnector->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 = &anx6345->client->dev;
+ anx6345->aux.drm_dev = bridge->dev;
anx6345->aux.transfer = anx6345_aux_transfer;
err = drm_dp_aux_register(&anx6345->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 = &anx78xx->client->dev;
+ anx78xx->aux.drm_dev = bridge->dev;
anx78xx->aux.transfer = anx78xx_aux_transfer;
err = drm_dp_aux_register(&anx78xx->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 = "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)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index d966a33743b5..fe821ad628ec 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1674,6 +1674,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;
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index da89922721ed..23a6f90b694b 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -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;
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 88df4dd0f39d..8e24272bbf00 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -362,6 +362,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);
diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c
index e25181bf2c48..06b374cae956 100644
--- a/drivers/gpu/drm/drm_dp_aux_dev.c
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -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);
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 42a0c6888c33..1a7a8b085de4 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -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);
/*
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
index 7e83bc2cc34a..c4b446d6a042 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
@@ -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 */
diff --git a/drivers/gpu/drm/msm/edp/edp.h b/drivers/gpu/drm/msm/edp/edp.h
index eb34243dad53..8590f2ce274d 100644
--- a/drivers/gpu/drm/msm/edp/edp.h
+++ b/drivers/gpu/drm/msm/edp/edp.h
@@ -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);
diff --git a/drivers/gpu/drm/msm/edp/edp_aux.c b/drivers/gpu/drm/msm/edp/edp_aux.c
index df10a0196d94..e3d85c622cfb 100644
--- a/drivers/gpu/drm/msm/edp/edp_aux.c
+++ b/drivers/gpu/drm/msm/edp/edp_aux.c
@@ -184,9 +184,9 @@ static ssize_t edp_aux_transfer(struct drm_dp_aux *drm_aux,
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) {
diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c b/drivers/gpu/drm/msm/edp/edp_ctrl.c
index 0d9657cc70db..57af3d8b6699 100644
--- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
+++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
@@ -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;
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index bfce762adcf0..d1b49508ecb9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1355,6 +1355,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);
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index 15b00a347560..c50c504bad50 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -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;
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index ea56c6ec25e4..7d7cc90b6fc9 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -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;
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 99158ee67d02..8272eee03adc 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -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);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5efa0d329b67..4abe0cea1302 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1838,6 +1838,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
@@ -1845,7 +1847,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.
@@ -1877,6 +1883,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;
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 02/20] drm/tegra: Don't register DP AUX channels before connectors
2021-03-26 20:37 ` [PATCH v2 02/20] drm/tegra: Don't register DP AUX channels before connectors Lyude Paul
@ 2021-03-29 6:51 ` Thierry Reding
0 siblings, 0 replies; 5+ messages in thread
From: Thierry Reding @ 2021-03-29 6:51 UTC (permalink / raw)
To: Lyude Paul
Cc: nouveau, dri-devel, amd-gfx, intel-gfx, David Airlie,
Daniel Vetter, Jonathan Hunter,
open list:DRM DRIVERS FOR NVIDIA TEGRA, open list
[-- Attachment #1: Type: text/plain, Size: 868 bytes --]
On Fri, Mar 26, 2021 at 04:37:49PM -0400, Lyude Paul wrote:
> As pointed out by the documentation for drm_dp_aux_register(),
> drm_dp_aux_init() should be used in situations where the AUX channel for a
> display driver can potentially be registered before it's respective DRM
> driver. This is the case with Tegra, since the DP aux channel exists as a
> platform device instead of being a grandchild of the DRM device.
>
> Since we're about to add a backpointer to a DP AUX channel's respective DRM
> device, let's fix this so that we don't potentially allow userspace to use
> the AUX channel before we've associated it with it's DRM connector.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
> drivers/gpu/drm/tegra/dpaux.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 05/20] drm/dp: Add backpointer to drm_device in drm_dp_aux
2021-03-26 20:37 ` [PATCH v2 05/20] drm/dp: Add backpointer to drm_device in drm_dp_aux Lyude Paul
@ 2021-03-29 7:08 ` Thierry Reding
2021-04-01 13:31 ` Jani Nikula
1 sibling, 0 replies; 5+ messages in thread
From: Thierry Reding @ 2021-03-29 7:08 UTC (permalink / raw)
To: Lyude Paul
Cc: nouveau, dri-devel, amd-gfx, intel-gfx, Alex Deucher,
Christian König, David Airlie, Daniel Vetter, Harry Wentland,
Leo Li, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Rob Clark, Sean Paul, Ben Skeggs,
Jonathan Hunter, Hyun Kwon, Michal Simek, Luben Tuikov,
Oleg Vasilev, Mikita Lipski, Eryk Brol, Rodrigo Siqueira,
Jerry (Fangzhi) Zuo, Chris Park, Sam Ravnborg, Joe Perches,
Vasily Khoruzhick, Guido Günther, Boris Brezillon, Andy Yan,
Marek Szyprowski, Tomi Valkeinen, Swapnil Jakhade, Yuti Amonkar,
Jyri Sarha, Julia Lawall, Ville Syrjälä, Imre Deak,
Matt Roper, Lucas De Marchi, open list,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVERS FOR NVIDIA TEGRA,
moderated list:ARM/ZYNQ ARCHITECTURE
[-- Attachment #1: Type: text/plain, Size: 943 bytes --]
On Fri, Mar 26, 2021 at 04:37:52PM -0400, 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.
>
> 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 <lyude@redhat.com>
> ---
[...]
> drivers/gpu/drm/tegra/dpaux.c | 1 +
[...]
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 05/20] drm/dp: Add backpointer to drm_device in drm_dp_aux
2021-03-26 20:37 ` [PATCH v2 05/20] drm/dp: Add backpointer to drm_device in drm_dp_aux Lyude Paul
2021-03-29 7:08 ` Thierry Reding
@ 2021-04-01 13:31 ` Jani Nikula
1 sibling, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2021-04-01 13:31 UTC (permalink / raw)
To: Lyude Paul, nouveau, dri-devel, amd-gfx, intel-gfx
Cc: Alex Deucher, Christian König, David Airlie, Daniel Vetter,
Harry Wentland, Leo Li, Andrzej Hajda, Neil Armstrong,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Joonas Lahtinen, Rodrigo Vivi, Rob Clark, Sean Paul, Ben Skeggs,
Thierry Reding, Jonathan Hunter, Hyun Kwon, Michal Simek,
Luben Tuikov, Oleg Vasilev, Mikita Lipski, Eryk Brol,
Rodrigo Siqueira, Jerry (Fangzhi) Zuo, Chris Park, Sam Ravnborg,
Joe Perches, Vasily Khoruzhick, Guido Günther,
Boris Brezillon, Andy Yan, Marek Szyprowski, Tomi Valkeinen,
Swapnil Jakhade, Yuti Amonkar, Jyri Sarha, Julia Lawall,
Ville Syrjälä, Imre Deak, Matt Roper, Lucas De Marchi,
open list, open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVERS FOR NVIDIA TEGRA,
moderated list:ARM/ZYNQ ARCHITECTURE
On Fri, 26 Mar 2021, Lyude Paul <lyude@redhat.com> wrote:
> * 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().
Perhaps add a follow-up patch to actually ensure this is the case in
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.
> @@ -1877,6 +1883,7 @@ struct drm_dp_aux {
> const char *name;
> struct i2c_adapter ddc;
> struct device *dev;
> + struct drm_device *drm_dev;
Bikeshed, I would probably have called it just drm for brevity, but no
strong feelings.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-01 17:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210326203807.105754-1-lyude@redhat.com>
2021-03-26 20:37 ` [PATCH v2 02/20] drm/tegra: Don't register DP AUX channels before connectors Lyude Paul
2021-03-29 6:51 ` Thierry Reding
2021-03-26 20:37 ` [PATCH v2 05/20] drm/dp: Add backpointer to drm_device in drm_dp_aux Lyude Paul
2021-03-29 7:08 ` Thierry Reding
2021-04-01 13:31 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).