[Intel-gfx] [PATCH v3 4/7] drm/i915/vdsc: Use MACRO to cleanup intel_dsc_get_pps_reg

Nautiyal, Ankit K ankit.k.nautiyal at intel.com
Tue Jul 18 09:24:08 UTC 2023


On 7/17/2023 3:39 PM, Suraj Kandpal wrote:
> Use a Macro to clean up intel_dsc_get_pps_reg so that we don't have
> to replicate so many if/else blocks.
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_vdsc.c | 124 ++++++----------------
>   1 file changed, 32 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 6d319f351a12..ed8fda431226 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -313,131 +313,71 @@ static void intel_dsc_get_pps_reg(struct intel_crtc_state *crtc_state, int pps,
>   
>   	pipe_dsc = is_pipe_dsc(crtc, cpu_transcoder);
>   
> +#define PRE_MTL_GET_DSC_REGISTER(pps, is_pipe_dsc, pipe) do { \
> +	if (is_pipe_dsc) { \
> +		*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_##pps(pipe); \
> +		*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_##pps(pipe); \
> +	} else { \
> +		*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_##pps; \
> +		*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_##pps; \

I think it will be more efficient to add an enum intel_dsc_engine and 
have a function that returns the reg for given, pps, engine and pipe.

something like:

static i915_reg_t intel_dsc_get_pps_reg(const struct intel_crtc_state 
*crtc_state, int pps, enum vdsc_engine vdsc)

Since offsets are regularly placed, we can have something like : 
ICL_DSC_PICTURE_PARAMETER_SET(engine, pps, pipe) and

DSC_PICTURE_PARAMETER_SET(engine, pps) for ICL EDP/DSI case.

Roughly it will be Offset of PPS Reg for Engine 0 + engine * 0x800 + pps 
* 4.

Though this will require quite some changes in reg macros in 
intel_vdsc_reg.h ,and also need to have a new enum for intel_dsc_engine,

but IMHO it will be easier to get the required PPS reg this way.

Regards,

Ankit


> +	} \
> +} while (0)
> +
> +#define MTL_GET_DSC_REGISTER(pps, pipe) do { \
> +		*dsc_reg0 = MTL_DSC0_PICTURE_PARAMETER_SET_##pps(pipe); \
> +		*dsc_reg1 = MTL_DSC1_PICTURE_PARAMETER_SET_##pps(pipe); \
> +} while (0)
> +
>   	switch (pps) {
>   	case 0:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_0(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_0;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_0;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(0, pipe_dsc, pipe);
>   		break;
>   	case 1:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_1(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_1;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_1;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(1, pipe_dsc, pipe);
>   		break;
>   	case 2:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_2(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_2(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_2;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_2;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(2, pipe_dsc, pipe);
>   		break;
>   	case 3:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_3(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_3(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_3;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_3;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(3, pipe_dsc, pipe);
>   		break;
>   	case 4:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_4(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_4(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_4;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_4;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(4, pipe_dsc, pipe);
>   		break;
>   	case 5:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_5(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_5(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_5;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_5;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(5, pipe_dsc, pipe);
>   		break;
>   	case 6:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_6(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_6(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_6;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_6;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(6, pipe_dsc, pipe);
>   		break;
>   	case 7:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_7(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_7(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_7;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_7;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(7, pipe_dsc, pipe);
>   		break;
>   	case 8:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_8(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_8(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_8;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_8;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(8, pipe_dsc, pipe);
>   		break;
>   	case 9:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_9(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_9(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_9;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_9;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(9, pipe_dsc, pipe);
>   		break;
>   	case 10:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_10(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_10(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_10;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_10;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(10, pipe_dsc, pipe);
>   		break;
>   	case 16:
> -		if (pipe_dsc) {
> -			*dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_16(pipe);
> -			*dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_16(pipe);
> -		} else {
> -			*dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_16;
> -			*dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_16;
> -		}
> +		PRE_MTL_GET_DSC_REGISTER(16, pipe_dsc, pipe);
>   		break;
> -	/*
> -	 * Since PPS_17 and PPS_18 were introduced from MTL dsc check
> -	 * need not be done
> -	 */
>   	case 17:
> -		*dsc_reg0 = MTL_DSC0_PICTURE_PARAMETER_SET_17(pipe);
> -		*dsc_reg1 = MTL_DSC1_PICTURE_PARAMETER_SET_17(pipe);
> +		MTL_GET_DSC_REGISTER(17, pipe);
>   		break;
>   	case 18:
> -		*dsc_reg0 = MTL_DSC0_PICTURE_PARAMETER_SET_18(pipe);
> -		*dsc_reg1 = MTL_DSC1_PICTURE_PARAMETER_SET_18(pipe);
> +		MTL_GET_DSC_REGISTER(18, pipe);
>   		break;
>   	default:
>   		MISSING_CASE(pps);
>   		break;
>   	}
> +
> +#undef PRE_MTL_GET_DSC_REGISTER
> +#undef MTL_GET_DSC_REGISTER
>   }
>   
>   static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)


More information about the Intel-gfx mailing list