From: Sean Anderson <sean.anderson@linux.dev>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
dri-devel@lists.freedesktop.org
Cc: linux-arm-kernel@lists.infradead.org,
David Airlie <airlied@gmail.com>,
linux-kernel@vger.kernel.org, Daniel Vetter <daniel@ffwll.ch>,
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
Michal Simek <michal.simek@amd.com>,
Sean Anderson <sean.anderson@linux.dev>
Subject: [PATCH v5 01/10] drm: zynqmp_kms: Fix AUX bus not getting unregistered
Date: Fri, 3 May 2024 15:29:13 -0400 [thread overview]
Message-ID: <20240503192922.2172314-2-sean.anderson@linux.dev> (raw)
In-Reply-To: <20240503192922.2172314-1-sean.anderson@linux.dev>
drm_encoder_cleanup is responsible for calling drm_bridge_detach for
each bridge attached to the encoder. zynqmp_dp_bridge_detach is in turn
responsible for unregistering the AUX bus. However, we never ended up
calling drm_encoder_cleanup in the remove or error paths, so the AUX bus
would stick around after the rest of the driver had been removed.
I don't really understand why drm_mode_config_cleanup doesn't call
drm_encoder_cleanup for us. It will call destroy (which for
simple_encoder is drm_encoder_cleanup) on encoders in the mode_config's
encoder_list.
Should drm_encoder_cleanup get called before or after
drm_atomic_helper_shutdown?
Fixes: 2dfd045c8435 ("drm: xlnx: zynqmp_dpsub: Register AUX bus at bridge attach time")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
Changes in v5:
- New
drivers/gpu/drm/xlnx/zynqmp_kms.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
index 43bf416b33d5..f25583ce92e6 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
@@ -433,23 +433,28 @@ static int zynqmp_dpsub_kms_init(struct zynqmp_dpsub *dpsub)
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret) {
dev_err(dpsub->dev, "failed to attach bridge to encoder\n");
- return ret;
+ goto err_encoder;
}
/* Create the connector for the chain of bridges. */
connector = drm_bridge_connector_init(&dpsub->drm->dev, encoder);
if (IS_ERR(connector)) {
dev_err(dpsub->dev, "failed to created connector\n");
- return PTR_ERR(connector);
+ ret = PTR_ERR(connector);
+ goto err_encoder;
}
ret = drm_connector_attach_encoder(connector, encoder);
if (ret < 0) {
dev_err(dpsub->dev, "failed to attach connector to encoder\n");
- return ret;
+ goto err_encoder;
}
return 0;
+
+err_encoder:
+ drm_encoder_cleanup(encoder);
+ return ret;
}
static void zynqmp_dpsub_drm_release(struct drm_device *drm, void *res)
@@ -529,5 +534,6 @@ void zynqmp_dpsub_drm_cleanup(struct zynqmp_dpsub *dpsub)
drm_dev_unregister(drm);
drm_atomic_helper_shutdown(drm);
+ drm_encoder_cleanup(&dpsub->drm->encoder);
drm_kms_helper_poll_fini(drm);
}
--
2.35.1.1320.gc452695387.dirty
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Sean Anderson <sean.anderson@linux.dev>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
dri-devel@lists.freedesktop.org
Cc: linux-arm-kernel@lists.infradead.org,
David Airlie <airlied@gmail.com>,
linux-kernel@vger.kernel.org, Daniel Vetter <daniel@ffwll.ch>,
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
Michal Simek <michal.simek@amd.com>,
Sean Anderson <sean.anderson@linux.dev>
Subject: [PATCH v5 01/10] drm: zynqmp_kms: Fix AUX bus not getting unregistered
Date: Fri, 3 May 2024 15:29:13 -0400 [thread overview]
Message-ID: <20240503192922.2172314-2-sean.anderson@linux.dev> (raw)
In-Reply-To: <20240503192922.2172314-1-sean.anderson@linux.dev>
drm_encoder_cleanup is responsible for calling drm_bridge_detach for
each bridge attached to the encoder. zynqmp_dp_bridge_detach is in turn
responsible for unregistering the AUX bus. However, we never ended up
calling drm_encoder_cleanup in the remove or error paths, so the AUX bus
would stick around after the rest of the driver had been removed.
I don't really understand why drm_mode_config_cleanup doesn't call
drm_encoder_cleanup for us. It will call destroy (which for
simple_encoder is drm_encoder_cleanup) on encoders in the mode_config's
encoder_list.
Should drm_encoder_cleanup get called before or after
drm_atomic_helper_shutdown?
Fixes: 2dfd045c8435 ("drm: xlnx: zynqmp_dpsub: Register AUX bus at bridge attach time")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
Changes in v5:
- New
drivers/gpu/drm/xlnx/zynqmp_kms.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
index 43bf416b33d5..f25583ce92e6 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
@@ -433,23 +433,28 @@ static int zynqmp_dpsub_kms_init(struct zynqmp_dpsub *dpsub)
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret) {
dev_err(dpsub->dev, "failed to attach bridge to encoder\n");
- return ret;
+ goto err_encoder;
}
/* Create the connector for the chain of bridges. */
connector = drm_bridge_connector_init(&dpsub->drm->dev, encoder);
if (IS_ERR(connector)) {
dev_err(dpsub->dev, "failed to created connector\n");
- return PTR_ERR(connector);
+ ret = PTR_ERR(connector);
+ goto err_encoder;
}
ret = drm_connector_attach_encoder(connector, encoder);
if (ret < 0) {
dev_err(dpsub->dev, "failed to attach connector to encoder\n");
- return ret;
+ goto err_encoder;
}
return 0;
+
+err_encoder:
+ drm_encoder_cleanup(encoder);
+ return ret;
}
static void zynqmp_dpsub_drm_release(struct drm_device *drm, void *res)
@@ -529,5 +534,6 @@ void zynqmp_dpsub_drm_cleanup(struct zynqmp_dpsub *dpsub)
drm_dev_unregister(drm);
drm_atomic_helper_shutdown(drm);
+ drm_encoder_cleanup(&dpsub->drm->encoder);
drm_kms_helper_poll_fini(drm);
}
--
2.35.1.1320.gc452695387.dirty
next prev parent reply other threads:[~2024-05-03 19:29 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 19:29 [PATCH v5 00/10] drm: zynqmp_dp: IRQ cleanups and debugfs support Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` Sean Anderson [this message]
2024-05-03 19:29 ` [PATCH v5 01/10] drm: zynqmp_kms: Fix AUX bus not getting unregistered Sean Anderson
2024-05-03 19:29 ` [PATCH v5 02/10] drm: zynqmp_dp: Rearrange zynqmp_dp for better padding Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` [PATCH v5 03/10] drm: zynqmp_dp: Don't delay work Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` [PATCH v5 04/10] drm: zynqmp_dp: Add locking Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` [PATCH v5 05/10] drm: zynqmp_dp: Don't retrain the link in our IRQ Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` [PATCH v5 06/10] drm: zynqmp_dp: Convert to a hard IRQ Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` [PATCH v5 07/10] drm: zynqmp_dp: Use AUX IRQs instead of polling Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` [PATCH v5 08/10] drm: zynqmp_dp: Split off several helper functions Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` [PATCH v5 09/10] drm: zynqmp_dp: Take dp->lock in zynqmp_dp_hpd_work_func Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-03 19:29 ` [PATCH v5 10/10] drm: zynqmp_dp: Add debugfs interface for compliance testing Sean Anderson
2024-05-03 19:29 ` Sean Anderson
2024-05-31 16:27 ` [PATCH v5 00/10] drm: zynqmp_dp: IRQ cleanups and debugfs support Sean Anderson
2024-05-31 16:27 ` Sean Anderson
2024-06-17 7:47 ` Tomi Valkeinen
2024-06-17 14:48 ` Sean Anderson
2024-08-08 12:46 ` Tomi Valkeinen
2024-08-09 18:55 ` Sean Anderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240503192922.2172314-2-sean.anderson@linux.dev \
--to=sean.anderson@linux.dev \
--cc=airlied@gmail.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=michal.simek@amd.com \
--cc=mripard@kernel.org \
--cc=tomi.valkeinen@ideasonboard.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.