Re: [PATCH 13/19] drm/gma500: Unify struct *_intel_lvds_helper_funcs

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 13.06.22 um 14:34 schrieb Patrik Jakobsson:
> These now only contains generic gma functions so create
> gma_lvds_helper_funcs that both PSB and CDV can use. Oaktrail still
> needs the modeset callback refactored to align with PSB and CDV.
>
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@xxxxxxxxx>
> ---
>   drivers/gpu/drm/gma500/cdv_intel_lvds.c | 11 +----------
>   drivers/gpu/drm/gma500/gma_lvds.c       | 14 +++++++++++---
>   drivers/gpu/drm/gma500/gma_lvds.h       |  5 ++---
>   drivers/gpu/drm/gma500/psb_intel_lvds.c | 10 +---------
>   4 files changed, 15 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> index ddfb976b6059..80ccc00c47e5 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> @@ -136,15 +136,6 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
>   	return 0;
>   }
>   
> -static const struct drm_encoder_helper_funcs
> -					cdv_intel_lvds_helper_funcs = {
> -	.dpms = gma_lvds_encoder_dpms,
> -	.mode_fixup = gma_lvds_mode_fixup,
> -	.prepare = gma_lvds_prepare,
> -	.mode_set = gma_lvds_mode_set,
> -	.commit = gma_lvds_commit,
> -};
> -
>   static const struct drm_connector_helper_funcs
>   				cdv_intel_lvds_connector_helper_funcs = {
>   	.get_modes = cdv_intel_lvds_get_modes,
> @@ -286,7 +277,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
>   	gma_connector_attach_encoder(gma_connector, gma_encoder);
>   	gma_encoder->type = INTEL_OUTPUT_LVDS;
>   
> -	drm_encoder_helper_add(encoder, &cdv_intel_lvds_helper_funcs);
> +	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
>   	drm_connector_helper_add(connector,
>   				 &cdv_intel_lvds_connector_helper_funcs);
>   	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
> index 215bf8f7d41f..bf9fa5ebdbd3 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.c
> +++ b/drivers/gpu/drm/gma500/gma_lvds.c
> @@ -299,9 +299,9 @@ void gma_lvds_commit(struct drm_encoder *encoder)
>   	gma_lvds_set_power(dev, true);
>   }
>   
> -void gma_lvds_mode_set(struct drm_encoder *encoder,
> -		       struct drm_display_mode *mode,
> -		       struct drm_display_mode *adjusted_mode)
> +static void gma_lvds_mode_set(struct drm_encoder *encoder,
> +			      struct drm_display_mode *mode,
> +			      struct drm_display_mode *adjusted_mode)
>   {
>   	struct drm_device *dev = encoder->dev;
>   	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> @@ -334,3 +334,11 @@ void gma_lvds_mode_set(struct drm_encoder *encoder,
>   	REG_WRITE(PFIT_CONTROL, pfit_control);
>   }
>   
> +const struct drm_encoder_helper_funcs gma_lvds_helper_funcs = {
> +	.dpms = gma_lvds_encoder_dpms,
> +	.mode_fixup = gma_lvds_mode_fixup,
> +	.prepare = gma_lvds_prepare,
> +	.mode_set = gma_lvds_mode_set,
> +	.commit = gma_lvds_commit,
> +};
> +

Alternatively, you could use an initializer macro, such as

#define GMA_LVDS_HELPER_FUNCS \
  .dpms = ..
  .mode_fixup = ...
  ...

and use it to initialize the per-model model instances. This would allow 
to keep each instance as 'static const'.  The linker should be able to 
sort out duplicates.

Best regards
Thomas

> diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
> index ebba869a25b7..3c47bea859ad 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.h
> +++ b/drivers/gpu/drm/gma500/gma_lvds.h
> @@ -34,8 +34,7 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
>   			 struct drm_display_mode *adjusted_mode);
>   void gma_lvds_prepare(struct drm_encoder *encoder);
>   void gma_lvds_commit(struct drm_encoder *encoder);
> -void gma_lvds_mode_set(struct drm_encoder *encoder,
> -		       struct drm_display_mode *mode,
> -		       struct drm_display_mode *adjusted_mode);
> +
> +extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
>   
>   #endif
> diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> index 553f6cb5f322..29a9b4ea5803 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> @@ -236,14 +236,6 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
>   	return -1;
>   }
>   
> -static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
> -	.dpms = gma_lvds_encoder_dpms,
> -	.mode_fixup = gma_lvds_mode_fixup,
> -	.prepare = gma_lvds_prepare,
> -	.mode_set = gma_lvds_mode_set,
> -	.commit = gma_lvds_commit,
> -};
> -
>   const struct drm_connector_helper_funcs
>   				psb_intel_lvds_connector_helper_funcs = {
>   	.get_modes = psb_intel_lvds_get_modes,
> @@ -329,7 +321,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
>   	gma_connector_attach_encoder(gma_connector, gma_encoder);
>   	gma_encoder->type = INTEL_OUTPUT_LVDS;
>   
> -	drm_encoder_helper_add(encoder, &psb_intel_lvds_helper_funcs);
> +	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
>   	drm_connector_helper_add(connector,
>   				 &psb_intel_lvds_connector_helper_funcs);
>   	connector->display_info.subpixel_order = SubPixelHorizontalRGB;

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

Attachment: OpenPGP_signature
Description: OpenPGP digital signature