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@xxxxxxxxx>
> ---
> 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)
