On 7/18/2023 2:54 PM, Nautiyal, Ankit K wrote:
> 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.
Sorry this is incorrect example. Need to get the difference, for engine
and pps to calculate the offset correctly.
Regards,
Ankit
>
> 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)
