[PATCH v5 04/11] drm/connector: unregister CEC data

Dmitry Baryshkov posted 11 patches 10 months, 1 week ago
There is a newer version of this series
[PATCH v5 04/11] drm/connector: unregister CEC data
Posted by Dmitry Baryshkov 10 months, 1 week ago
In order to make sure that CEC adapters or notifiers are unregistered
and CEC-related data is properly destroyed make drm_connector_cleanup()
call CEC's unregister() callback.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_connector.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
 }
 EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
 
+static void drm_connector_cec_unregister(struct drm_connector *connector)
+{
+	if (connector->cec.funcs &&
+	    connector->cec.funcs->unregister)
+		connector->cec.funcs->unregister(connector);
+}
+
 /**
  * drm_connector_cleanup - cleans up an initialised connector
  * @connector: connector to cleanup
@@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
 
 	platform_device_unregister(connector->hdmi_audio.codec_pdev);
 
+	drm_connector_cec_unregister(connector);
+
 	if (connector->privacy_screen) {
 		drm_privacy_screen_put(connector->privacy_screen);
 		connector->privacy_screen = NULL;

-- 
2.39.5
Re: [PATCH v5 04/11] drm/connector: unregister CEC data
Posted by Maxime Ripard 10 months ago
Hi,

On Mon, Apr 07, 2025 at 06:11:01PM +0300, Dmitry Baryshkov wrote:
> In order to make sure that CEC adapters or notifiers are unregistered
> and CEC-related data is properly destroyed make drm_connector_cleanup()
> call CEC's unregister() callback.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
>  }
>  EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
>  
> +static void drm_connector_cec_unregister(struct drm_connector *connector)
> +{
> +	if (connector->cec.funcs &&
> +	    connector->cec.funcs->unregister)
> +		connector->cec.funcs->unregister(connector);
> +}
> +
>  /**
>   * drm_connector_cleanup - cleans up an initialised connector
>   * @connector: connector to cleanup
> @@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
>  
>  	platform_device_unregister(connector->hdmi_audio.codec_pdev);
>  
> +	drm_connector_cec_unregister(connector);
> +

Actually, since we know that the HDMI connector is drm-managed, why
can't we make the call to connector->cec.funcs->unregister a drm-managed
action registered by drm_connector_hdmi_cec_register?

Maxime
Re: [PATCH v5 04/11] drm/connector: unregister CEC data
Posted by Dmitry Baryshkov 10 months ago
On 14/04/2025 17:47, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Apr 07, 2025 at 06:11:01PM +0300, Dmitry Baryshkov wrote:
>> In order to make sure that CEC adapters or notifiers are unregistered
>> and CEC-related data is properly destroyed make drm_connector_cleanup()
>> call CEC's unregister() callback.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
>>   }
>>   EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
>>   
>> +static void drm_connector_cec_unregister(struct drm_connector *connector)
>> +{
>> +	if (connector->cec.funcs &&
>> +	    connector->cec.funcs->unregister)
>> +		connector->cec.funcs->unregister(connector);
>> +}
>> +
>>   /**
>>    * drm_connector_cleanup - cleans up an initialised connector
>>    * @connector: connector to cleanup
>> @@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
>>   
>>   	platform_device_unregister(connector->hdmi_audio.codec_pdev);
>>   
>> +	drm_connector_cec_unregister(connector);
>> +
> 
> Actually, since we know that the HDMI connector is drm-managed, why
> can't we make the call to connector->cec.funcs->unregister a drm-managed
> action registered by drm_connector_hdmi_cec_register?

I haven't settled yet in my mind whether we can/should also use this 
infrastructure for drm_dp_cec management. So, at this point, I'd prefer 
to keep a non-managed unregister function. Once we settle on something 
for drm_dp_cec, we can switch to drmm.

-- 
With best wishes
Dmitry
Re: [PATCH v5 04/11] drm/connector: unregister CEC data
Posted by Maxime Ripard 9 months, 2 weeks ago
On Tue, Apr 15, 2025 at 12:03:23PM +0300, Dmitry Baryshkov wrote:
> On 14/04/2025 17:47, Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Apr 07, 2025 at 06:11:01PM +0300, Dmitry Baryshkov wrote:
> > > In order to make sure that CEC adapters or notifiers are unregistered
> > > and CEC-related data is properly destroyed make drm_connector_cleanup()
> > > call CEC's unregister() callback.
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > ---
> > >   drivers/gpu/drm/drm_connector.c | 9 +++++++++
> > >   1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
> > >   }
> > >   EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
> > > +static void drm_connector_cec_unregister(struct drm_connector *connector)
> > > +{
> > > +	if (connector->cec.funcs &&
> > > +	    connector->cec.funcs->unregister)
> > > +		connector->cec.funcs->unregister(connector);
> > > +}
> > > +
> > >   /**
> > >    * drm_connector_cleanup - cleans up an initialised connector
> > >    * @connector: connector to cleanup
> > > @@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
> > >   	platform_device_unregister(connector->hdmi_audio.codec_pdev);
> > > +	drm_connector_cec_unregister(connector);
> > > +
> > 
> > Actually, since we know that the HDMI connector is drm-managed, why
> > can't we make the call to connector->cec.funcs->unregister a drm-managed
> > action registered by drm_connector_hdmi_cec_register?
> 
> I haven't settled yet in my mind whether we can/should also use this
> infrastructure for drm_dp_cec management. So, at this point, I'd prefer to
> keep a non-managed unregister function. Once we settle on something for
> drm_dp_cec, we can switch to drmm.

I'd rather do the opposite. Let's go for drmm for now, and if we need to
change it for DP, we can always change it.

"Nothing is so permanent as a temporary solution", so I'd rather have
the natural and consistent one for now :)

Maxime
Re: [PATCH v5 04/11] drm/connector: unregister CEC data
Posted by Dmitry Baryshkov 9 months, 2 weeks ago
On Tue, 29 Apr 2025 at 18:35, Maxime Ripard <mripard@kernel.org> wrote:
>
> On Tue, Apr 15, 2025 at 12:03:23PM +0300, Dmitry Baryshkov wrote:
> > On 14/04/2025 17:47, Maxime Ripard wrote:
> > > Hi,
> > >
> > > On Mon, Apr 07, 2025 at 06:11:01PM +0300, Dmitry Baryshkov wrote:
> > > > In order to make sure that CEC adapters or notifiers are unregistered
> > > > and CEC-related data is properly destroyed make drm_connector_cleanup()
> > > > call CEC's unregister() callback.
> > > >
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > > ---
> > > >   drivers/gpu/drm/drm_connector.c | 9 +++++++++
> > > >   1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
> > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > @@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
> > > >   }
> > > >   EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
> > > > +static void drm_connector_cec_unregister(struct drm_connector *connector)
> > > > +{
> > > > + if (connector->cec.funcs &&
> > > > +     connector->cec.funcs->unregister)
> > > > +         connector->cec.funcs->unregister(connector);
> > > > +}
> > > > +
> > > >   /**
> > > >    * drm_connector_cleanup - cleans up an initialised connector
> > > >    * @connector: connector to cleanup
> > > > @@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
> > > >           platform_device_unregister(connector->hdmi_audio.codec_pdev);
> > > > + drm_connector_cec_unregister(connector);
> > > > +
> > >
> > > Actually, since we know that the HDMI connector is drm-managed, why
> > > can't we make the call to connector->cec.funcs->unregister a drm-managed
> > > action registered by drm_connector_hdmi_cec_register?
> >
> > I haven't settled yet in my mind whether we can/should also use this
> > infrastructure for drm_dp_cec management. So, at this point, I'd prefer to
> > keep a non-managed unregister function. Once we settle on something for
> > drm_dp_cec, we can switch to drmm.
>
> I'd rather do the opposite. Let's go for drmm for now, and if we need to
> change it for DP, we can always change it.
>
> "Nothing is so permanent as a temporary solution", so I'd rather have
> the natural and consistent one for now :)

Ack, I'll change that for the next version.

-- 
With best wishes
Dmitry
Re: [PATCH v5 04/11] drm/connector: unregister CEC data
Posted by Maxime Ripard 10 months ago
On Mon, 7 Apr 2025 18:11:01 +0300, Dmitry Baryshkov wrote:
> In order to make sure that CEC adapters or notifiers are unregistered
> and CEC-related data is properly destroyed make drm_connector_cleanup()
> call CEC's unregister() callback.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime