Re: [RFC] drm/connector: Set the default callback function for drm_connector_funcs

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3541 bytes --]

Hi

Am 08.01.21 um 08:54 schrieb Tian Tao:
> The member functions of drm_connector_funcs are not specific to each
> manufacturer's driver, so drm_connector_funcs is allowed to use default
> values, which prevents all drivers from setting the same member
> functions for drm_connector_funcs.

I don't think that's a good idea.

> 
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> ---
>   drivers/gpu/drm/drm_connector.c                  | 7 ++++++-
>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 1 -
>   include/drm/drm_connector.h                      | 2 +-
>   3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 98b6ec4..356d8a3 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -25,6 +25,7 @@
>   #include <drm/drm_encoder.h>
>   #include <drm/drm_utils.h>
>   #include <drm/drm_print.h>
> +#include <drm/drm_probe_helper.h>
>   #include <drm/drm_drv.h>
>   #include <drm/drm_file.h>
>   #include <drm/drm_sysfs.h>
> @@ -216,7 +217,7 @@ void drm_connector_free_work_fn(struct work_struct *work)
>    */
>   int drm_connector_init(struct drm_device *dev,
>   		       struct drm_connector *connector,
> -		       const struct drm_connector_funcs *funcs,
> +		       struct drm_connector_funcs *funcs,

Drivers cannot legally declare the funcs instance as static const. 
Having static const allows for write protected pages.

>   		       int connector_type)
>   {
>   	struct drm_mode_config *config = &dev->mode_config;
> @@ -228,6 +229,10 @@ int drm_connector_init(struct drm_device *dev,
>   		(!funcs->atomic_destroy_state ||
>   		 !funcs->atomic_duplicate_state));
>   
> +	if (!funcs->fill_modes)
> +		funcs->fill_modes = &drm_helper_probe_single_connector_modes;

It's not clear that this is really the correct function for this driver.

Best regards
Thomas

> +
> +
>   	ret = __drm_mode_object_add(dev, &connector->base,
>   				    DRM_MODE_OBJECT_CONNECTOR,
>   				    false, drm_connector_free);
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> index c76f996..7d3b662 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> @@ -64,7 +64,6 @@ static const struct drm_connector_helper_funcs
>   };
>   
>   static const struct drm_connector_funcs hibmc_connector_funcs = {
> -	.fill_modes = drm_helper_probe_single_connector_modes,
>   	.destroy = hibmc_connector_destroy,
>   	.reset = drm_atomic_helper_connector_reset,
>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 1922b27..4810583 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1561,7 +1561,7 @@ struct drm_connector {
>   
>   int drm_connector_init(struct drm_device *dev,
>   		       struct drm_connector *connector,
> -		       const struct drm_connector_funcs *funcs,
> +		       struct drm_connector_funcs *funcs,
>   		       int connector_type);
>   int drm_connector_init_with_ddc(struct drm_device *dev,
>   				struct drm_connector *connector,
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel