Re: [PATCH v4 01/19] drm/encoder: make encoder control functions optional

[Sam Ravnborg <sam@ravnborg.org>]

Hi Philipp,
On Tue, Dec 08, 2020 at 04:54:33PM +0100, Philipp Zabel wrote:
> Simple managed encoders do not require the .destroy callback,
> make the whole funcs structure optional.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> New in v4.
> ---
>  drivers/gpu/drm/drm_encoder.c     | 4 ++--
>  drivers/gpu/drm/drm_mode_config.c | 5 +++--
>  include/drm/drm_encoder.h         | 2 +-
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index e555281f43d4..b0b86a3c08f5 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -72,7 +72,7 @@ int drm_encoder_register_all(struct drm_device *dev)
>  	int ret = 0;
>  
>  	drm_for_each_encoder(encoder, dev) {
> -		if (encoder->funcs->late_register)
> +		if (encoder->funcs && encoder->funcs->late_register)
>  			ret = encoder->funcs->late_register(encoder);
>  		if (ret)
>  			return ret;
> @@ -86,7 +86,7 @@ void drm_encoder_unregister_all(struct drm_device *dev)
>  	struct drm_encoder *encoder;
>  
>  	drm_for_each_encoder(encoder, dev) {
> -		if (encoder->funcs->early_unregister)
> +		if (encoder->funcs && encoder->funcs->early_unregister)
>  			encoder->funcs->early_unregister(encoder);
>  	}
>  }
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index f1affc1bb679..87e144155456 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -195,7 +195,7 @@ void drm_mode_config_reset(struct drm_device *dev)
>  			crtc->funcs->reset(crtc);
>  
>  	drm_for_each_encoder(encoder, dev)
> -		if (encoder->funcs->reset)
> +		if (encoder->funcs && encoder->funcs->reset)
>  			encoder->funcs->reset(encoder);
>  
>  	drm_connector_list_iter_begin(dev, &conn_iter);
> @@ -487,7 +487,8 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>  
>  	list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
>  				 head) {
> -		encoder->funcs->destroy(encoder);
> +		if (encoder->funcs)
> +			encoder->funcs->destroy(encoder);

So late_register and early_unregister are both optional.
But if encoder->funcs is set then the destroy callback is mandatory.

I am just wondering if this is intended.
Reding the documnetation of drm_encoder_funcs thist matches the
documentation but anyway..

With this comment considered,
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel