Re: [Intel-gfx] [PATCH 7/8] drm/i915/sdvo: Reduce copy-pasta in output setup

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 26 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Avoid having to call the output init function for each
> output type separately. We can just call the right one
> based on the "class" of the output.
>
> Technically we could just walk the bits of the bitmask
> but that could change the order in which we initialize
> the outputs. To avoid any behavioural changes keep to
> the same explicit probe order as before.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_sdvo.c | 66 +++++++++++------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 4784c05a1b71..58d147cc7633 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2922,11 +2922,38 @@ static u16 intel_sdvo_filter_output_flags(u16 flags)
>  	return flags;
>  }
>  
> +static bool intel_sdvo_output_init(struct intel_sdvo *sdvo, u16 type)
> +{
> +	if (type & SDVO_TMDS_MASK)
> +		return intel_sdvo_dvi_init(sdvo, type);
> +	else if (type & SDVO_TV_MASK)
> +		return intel_sdvo_tv_init(sdvo, type);
> +	else if (type & SDVO_RGB_MASK)
> +		return intel_sdvo_analog_init(sdvo, type);
> +	else if (type & SDVO_LVDS_MASK)
> +		return intel_sdvo_lvds_init(sdvo, type);
> +	else
> +		return false;
> +}
> +
>  static bool
>  intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo)
>  {
> +	static const u16 probe_order[] = {
> +		SDVO_OUTPUT_TMDS0,
> +		SDVO_OUTPUT_TMDS1,
> +		/* TV has no XXX1 function block */
> +		SDVO_OUTPUT_SVID0,
> +		SDVO_OUTPUT_CVBS0,
> +		SDVO_OUTPUT_YPRPB0,
> +		SDVO_OUTPUT_RGB0,
> +		SDVO_OUTPUT_RGB1,
> +		SDVO_OUTPUT_LVDS0,
> +		SDVO_OUTPUT_LVDS1,
> +	};
>  	struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
>  	u16 flags;
> +	int i;
>  
>  	flags = intel_sdvo_filter_output_flags(intel_sdvo->caps.output_flags);
>  
> @@ -2940,42 +2967,15 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo)
>  
>  	intel_sdvo_select_ddc_bus(i915, intel_sdvo);
>  
> -	if (flags & SDVO_OUTPUT_TMDS0)
> -		if (!intel_sdvo_dvi_init(intel_sdvo, SDVO_OUTPUT_TMDS0))
> -			return false;
> -
> -	if (flags & SDVO_OUTPUT_TMDS1)
> -		if (!intel_sdvo_dvi_init(intel_sdvo, SDVO_OUTPUT_TMDS1))
> -			return false;
> -
> -	/* TV has no XXX1 function block */
> -	if (flags & SDVO_OUTPUT_SVID0)
> -		if (!intel_sdvo_tv_init(intel_sdvo, SDVO_OUTPUT_SVID0))
> -			return false;
> -
> -	if (flags & SDVO_OUTPUT_CVBS0)
> -		if (!intel_sdvo_tv_init(intel_sdvo, SDVO_OUTPUT_CVBS0))
> -			return false;
> +	for (i = 0; i < ARRAY_SIZE(probe_order); i++) {
> +		u16 type = flags & probe_order[i];
>  
> -	if (flags & SDVO_OUTPUT_YPRPB0)
> -		if (!intel_sdvo_tv_init(intel_sdvo, SDVO_OUTPUT_YPRPB0))
> -			return false;
> -
> -	if (flags & SDVO_OUTPUT_RGB0)
> -		if (!intel_sdvo_analog_init(intel_sdvo, SDVO_OUTPUT_RGB0))
> -			return false;
> -
> -	if (flags & SDVO_OUTPUT_RGB1)
> -		if (!intel_sdvo_analog_init(intel_sdvo, SDVO_OUTPUT_RGB1))
> -			return false;
> -
> -	if (flags & SDVO_OUTPUT_LVDS0)
> -		if (!intel_sdvo_lvds_init(intel_sdvo, SDVO_OUTPUT_LVDS0))
> -			return false;
> +		if (!type)
> +			continue;
>  
> -	if (flags & SDVO_OUTPUT_LVDS1)
> -		if (!intel_sdvo_lvds_init(intel_sdvo, SDVO_OUTPUT_LVDS1))
> +		if (!intel_sdvo_output_init(intel_sdvo, type))
>  			return false;
> +	}
>  
>  	intel_sdvo->base.pipe_mask = ~0;

-- 
Jani Nikula, Intel Open Source Graphics Center