drm_dp_mst_topology_mgr_cbs.register_connector callbacks are identical amongst every driver and don't do anything other than calling drm_connector_register(). drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical amongst every driver and don't do anything other than cleaning up the connector((drm_connector_unregister()/drm_connector_put())) except for amdgpu_dm driver where some amdgpu_dm specific code in there.
This series aims to cleaup these drm_dp_mst_topology_mgr_cbs hooks.
Pankaj Bharadiya (5): drm: Register connector instead of calling register_connector callback drm: Remove dp mst register connector callbacks drm/dp_mst: Remove register_connector callback drm: Add drm_dp_destroy_connector helper and use it drm: Remove drm dp mst destroy_connector callbacks
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 6 ------ drivers/gpu/drm/drm_dp_mst_topology.c | 18 +++++++++++++++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 ---------------- drivers/gpu/drm/nouveau/dispnv50/disp.c | 19 ------------------- drivers/gpu/drm/radeon/radeon_dp_mst.c | 17 ----------------- include/drm/drm_dp_mst_helper.h | 1 - 6 files changed, 15 insertions(+), 62 deletions(-)
drm_dp_mst_topology_mgr_cbs.register_connector callbacks are literally identical amongst every driver and don't do anything other than calling drm_connector_register(). Hence call drm_connector_register() directly instead of a callback.
This is the preparatory step for removing the drm_dp_mst_topology_mgr_cbs.register_connector callback hook.
Signed-off-by: Pankaj Bharadiya pankaj.laxminarayan.bharadiya@intel.com Suggested-by: Emil Velikov emil.velikov@collabora.com Suggested-by: Lyude Paul lyude@redhat.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 6c62ad8f4414..53d10d6c91d9 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2175,7 +2175,7 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb, drm_connector_set_tile_property(port->connector); }
- mgr->cbs->register_connector(port->connector); + drm_connector_register(port->connector); return;
error:
drm_dp_mst_port_add_connector() directly calls the drm_connector_register() now and drm_dp_mst_topology_mgr_cbs.register_connector callback is not getting called anymore.
Hence remove all drm_dp_mst_topology_mgr_cbs.register_connector callbacks.
This is the preparatory step for removing the drm_dp_mst_topology_mgr_cbs.register_connector callback hook.
The removal is done with below sementic patch:
@r1@ identifier func, E; @@ struct drm_dp_mst_topology_cbs E = { ..., - .register_connector = func };
@delete depends on r1@ identifier r1.func; @@ - static void func(...){...}
Signed-off-by: Pankaj Bharadiya pankaj.laxminarayan.bharadiya@intel.com Suggested-by: Emil Velikov emil.velikov@collabora.com Suggested-by: Lyude Paul lyude@redhat.com --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 6 ------ drivers/gpu/drm/i915/display/intel_dp_mst.c | 6 ------ drivers/gpu/drm/nouveau/dispnv50/disp.c | 7 ------- drivers/gpu/drm/radeon/radeon_dp_mst.c | 6 ------ 4 files changed, 25 deletions(-)
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 00c8627eb60e..5ee2dc4597f0 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 @@ -458,15 +458,9 @@ static void dm_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr, drm_connector_put(connector); }
-static void dm_dp_mst_register_connector(struct drm_connector *connector) -{ - drm_connector_register(connector); -} - static const struct drm_dp_mst_topology_cbs dm_mst_cbs = { .add_connector = dm_dp_add_mst_connector, .destroy_connector = dm_dp_destroy_mst_connector, - .register_connector = dm_dp_mst_register_connector };
void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index d53978ed3c12..c6574a0a15b9 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -719,11 +719,6 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo return NULL; }
-static void intel_dp_register_mst_connector(struct drm_connector *connector) -{ - drm_connector_register(connector); -} - static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr, struct drm_connector *connector) { @@ -735,7 +730,6 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
static const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = intel_dp_add_mst_connector, - .register_connector = intel_dp_register_mst_connector, .destroy_connector = intel_dp_destroy_mst_connector, };
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 4e164ad8003f..4e3917073797 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1267,12 +1267,6 @@ nv50_mstm_destroy_connector(struct drm_dp_mst_topology_mgr *mgr, drm_connector_put(&mstc->connector); }
-static void -nv50_mstm_register_connector(struct drm_connector *connector) -{ - drm_connector_register(connector); -} - static struct drm_connector * nv50_mstm_add_connector(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path) @@ -1291,7 +1285,6 @@ nv50_mstm_add_connector(struct drm_dp_mst_topology_mgr *mgr, static const struct drm_dp_mst_topology_cbs nv50_mstm = { .add_connector = nv50_mstm_add_connector, - .register_connector = nv50_mstm_register_connector, .destroy_connector = nv50_mstm_destroy_connector, };
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c index 5a9fb0ad175a..795e2df773ae 100644 --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c @@ -301,11 +301,6 @@ static struct drm_connector *radeon_dp_add_mst_connector(struct drm_dp_mst_topol return connector; }
-static void radeon_dp_register_mst_connector(struct drm_connector *connector) -{ - drm_connector_register(connector); -} - static void radeon_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr, struct drm_connector *connector) { @@ -318,7 +313,6 @@ static void radeon_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
static const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = radeon_dp_add_mst_connector, - .register_connector = radeon_dp_register_mst_connector, .destroy_connector = radeon_dp_destroy_mst_connector, };
Now drm_dp_mst_topology_cbs.register_connector callback is not getting used anymore hence remove it.
Signed-off-by: Pankaj Bharadiya pankaj.laxminarayan.bharadiya@intel.com Suggested-by: Emil Velikov emil.velikov@collabora.com Suggested-by: Lyude Paul lyude@redhat.com --- include/drm/drm_dp_mst_helper.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 5483f888712a..885ada3c15a5 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -479,7 +479,6 @@ struct drm_dp_mst_topology_mgr; struct drm_dp_mst_topology_cbs { /* create a connector for a port */ struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path); - void (*register_connector)(struct drm_connector *connector); void (*destroy_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_connector *connector); };
drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical amongst every driver and don't do anything other than cleaning up the connector (drm_connector_unregister()/drm_connector_put()) except for amdgpu_dm driver where some amdgpu_dm specific code in there which I an not sure if it should stay or not.
Create and use a helper which calls driver's destroy_connector hook if available otherwise does cleanup internally.
This is the step towards removing identical hooks from every driver.
Signed-off-by: Pankaj Bharadiya pankaj.laxminarayan.bharadiya@intel.com Suggested-by: Emil Velikov emil.velikov@collabora.com Suggested-by: Lyude Paul lyude@redhat.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 53d10d6c91d9..8374cac74ccc 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4667,11 +4667,23 @@ static void drm_dp_tx_work(struct work_struct *work) mutex_unlock(&mgr->qlock); }
+static inline void drm_dp_destroy_connector(struct drm_dp_mst_port *port) +{ + if (!port->connector) + return; + + if (port->mgr->cbs->destroy_connector) { + port->mgr->cbs->destroy_connector(port->mgr, port->connector); + } else { + drm_connector_unregister(port->connector); + drm_connector_put(port->connector); + } +} + static inline void drm_dp_delayed_destroy_port(struct drm_dp_mst_port *port) { - if (port->connector) - port->mgr->cbs->destroy_connector(port->mgr, port->connector); + drm_dp_destroy_connector(port);
drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE, port->mcs); drm_dp_mst_put_port_malloc(port);
drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical amongst every driver and don't do anything other than cleaning up the connector((drm_connector_unregister()/drm_connector_put())) except for amdgpu_dm driver where some amdgpu_dm specific code in there.
This connector cleaning up is now being handled in the drm core so driver destroy_connector callbacks are not needed (except for amdgpu_dm) hence remove them.
Removal is done with below sementic patch:
@r1@ identifier func, E; @@ struct drm_dp_mst_topology_cbs E = { ..., - .destroy_connector = func };
@delete depends on r1@ identifier r1.func; @@ - static void func(...){...}
Signed-off-by: Pankaj Bharadiya pankaj.laxminarayan.bharadiya@intel.com Suggested-by: Emil Velikov emil.velikov@collabora.com Suggested-by: Lyude Paul lyude@redhat.com --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 10 ---------- drivers/gpu/drm/nouveau/dispnv50/disp.c | 12 ------------ drivers/gpu/drm/radeon/radeon_dp_mst.c | 11 ----------- 3 files changed, 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index c6574a0a15b9..a47d14b0c140 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -719,18 +719,8 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo return NULL; }
-static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr, - struct drm_connector *connector) -{ - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); - drm_connector_unregister(connector); - - drm_connector_put(connector); -} - static const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = intel_dp_add_mst_connector, - .destroy_connector = intel_dp_destroy_mst_connector, };
static struct intel_dp_mst_encoder * diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 4e3917073797..4d1c58468dbc 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1256,17 +1256,6 @@ nv50_mstm_prepare(struct nv50_mstm *mstm) } }
-static void -nv50_mstm_destroy_connector(struct drm_dp_mst_topology_mgr *mgr, - struct drm_connector *connector) -{ - struct nv50_mstc *mstc = nv50_mstc(connector); - - drm_connector_unregister(&mstc->connector); - - drm_connector_put(&mstc->connector); -} - static struct drm_connector * nv50_mstm_add_connector(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path) @@ -1285,7 +1274,6 @@ nv50_mstm_add_connector(struct drm_dp_mst_topology_mgr *mgr, static const struct drm_dp_mst_topology_cbs nv50_mstm = { .add_connector = nv50_mstm_add_connector, - .destroy_connector = nv50_mstm_destroy_connector, };
void diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c index 795e2df773ae..008308780443 100644 --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c @@ -301,19 +301,8 @@ static struct drm_connector *radeon_dp_add_mst_connector(struct drm_dp_mst_topol return connector; }
-static void radeon_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr, - struct drm_connector *connector) -{ - drm_connector_unregister(connector); - drm_connector_cleanup(connector); - - kfree(connector); - DRM_DEBUG_KMS("\n"); -} - static const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = radeon_dp_add_mst_connector, - .destroy_connector = radeon_dp_destroy_mst_connector, };
static struct
On Sat, 2020-03-07 at 14:00 +0530, Pankaj Bharadiya wrote:
drm_dp_mst_topology_mgr_cbs.register_connector callbacks are identical amongst every driver and don't do anything other than calling drm_connector_register(). drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical amongst every driver and don't do anything other than cleaning up the connector((drm_connector_unregister()/drm_connector_put())) except for amdgpu_dm driver where some amdgpu_dm specific code in there.
Yeah that amdgpu destruction code kinda stinks a little bit :. I think we can just drop some of it and move the rest into their connector destruction callbacks.
For the whole series: Reviewed-by: Lyude Paul lyude@redhat.com
I'm going to go ahead and let the maintainers know I'm going to push this (since there's some minor changes here outside of drm-misc), and push this to drm-misc-next. Then I'll go and write some patches to remove the leftover amd bits and drop the callback for good (I'll cc it to you as well).
This series aims to cleaup these drm_dp_mst_topology_mgr_cbs hooks.
Pankaj Bharadiya (5): drm: Register connector instead of calling register_connector callback drm: Remove dp mst register connector callbacks drm/dp_mst: Remove register_connector callback drm: Add drm_dp_destroy_connector helper and use it drm: Remove drm dp mst destroy_connector callbacks
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 6 ------ drivers/gpu/drm/drm_dp_mst_topology.c | 18 +++++++++++++++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 ---------------- drivers/gpu/drm/nouveau/dispnv50/disp.c | 19 ------------------- drivers/gpu/drm/radeon/radeon_dp_mst.c | 17 ----------------- include/drm/drm_dp_mst_helper.h | 1 - 6 files changed, 15 insertions(+), 62 deletions(-)
On Mon, Mar 9, 2020 at 4:27 PM Lyude Paul lyude@redhat.com wrote:
On Sat, 2020-03-07 at 14:00 +0530, Pankaj Bharadiya wrote:
drm_dp_mst_topology_mgr_cbs.register_connector callbacks are identical amongst every driver and don't do anything other than calling drm_connector_register(). drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical amongst every driver and don't do anything other than cleaning up the connector((drm_connector_unregister()/drm_connector_put())) except for amdgpu_dm driver where some amdgpu_dm specific code in there.
Yeah that amdgpu destruction code kinda stinks a little bit :. I think we can just drop some of it and move the rest into their connector destruction callbacks.
For the whole series: Reviewed-by: Lyude Paul lyude@redhat.com
I'm going to go ahead and let the maintainers know I'm going to push this (since there's some minor changes here outside of drm-misc), and push this to drm-misc-next. Then I'll go and write some patches to remove the leftover amd bits and drop the callback for good (I'll cc it to you as well).
Series is: Reviewed-by: Alex Deucher alexander.deucher@amd.com
This series aims to cleaup these drm_dp_mst_topology_mgr_cbs hooks.
Pankaj Bharadiya (5): drm: Register connector instead of calling register_connector callback drm: Remove dp mst register connector callbacks drm/dp_mst: Remove register_connector callback drm: Add drm_dp_destroy_connector helper and use it drm: Remove drm dp mst destroy_connector callbacks
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 6 ------ drivers/gpu/drm/drm_dp_mst_topology.c | 18 +++++++++++++++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 ---------------- drivers/gpu/drm/nouveau/dispnv50/disp.c | 19 ------------------- drivers/gpu/drm/radeon/radeon_dp_mst.c | 17 ----------------- include/drm/drm_dp_mst_helper.h | 1 - 6 files changed, 15 insertions(+), 62 deletions(-)
-- Cheers, Lyude Paul (she/her) Associate Software Engineer at Red Hat
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, 9 Mar 2020 at 20:27, Lyude Paul lyude@redhat.com wrote:
On Sat, 2020-03-07 at 14:00 +0530, Pankaj Bharadiya wrote:
drm_dp_mst_topology_mgr_cbs.register_connector callbacks are identical amongst every driver and don't do anything other than calling drm_connector_register(). drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical amongst every driver and don't do anything other than cleaning up the connector((drm_connector_unregister()/drm_connector_put())) except for amdgpu_dm driver where some amdgpu_dm specific code in there.
Yeah that amdgpu destruction code kinda stinks a little bit :. I think we can just drop some of it and move the rest into their connector destruction callbacks.
For the whole series: Reviewed-by: Lyude Paul lyude@redhat.com
I'm going to go ahead and let the maintainers know I'm going to push this (since there's some minor changes here outside of drm-misc), and push this to drm-misc-next. Then I'll go and write some patches to remove the leftover amd bits and drop the callback for good (I'll cc it to you as well).
Thanks for following on these Pankaj.
For the series: Reviewed-by: Emil Velikov emil.velikov@collabora.com
-Emil
dri-devel@lists.freedesktop.org