Re: [PATCH] drm/i915/display/vdsc: Fix the macro that calculates DSCC_/DSCA_ PPS reg address

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

On Thu, 01 Feb 2024, Manasi Navare <navaremanasi@chromium.org> wrote:
> On Thu, Feb 1, 2024 at 1:15 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>> On Wed, 31 Jan 2024, Manasi Navare <navaremanasi@chromium.org> wrote:
>> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
>> > index 64f440fdc22b..db29660b74f3 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
>> > @@ -51,8 +51,8 @@
>> >  #define DSCC_PICTURE_PARAMETER_SET_0         _MMIO(0x6BA00)
>> >  #define _DSCA_PPS_0                          0x6B200
>> >  #define _DSCC_PPS_0                          0x6BA00
>> > -#define DSCA_PPS(pps)                                _MMIO(_DSCA_PPS_0 + (pps) * 4)
>> > -#define DSCC_PPS(pps)                                _MMIO(_DSCC_PPS_0 + (pps) * 4)
>> > +#define DSCA_PPS(pps)                                ((pps < 12) ? _MMIO(_DSCA_PPS_0 + (pps) * 4):_MMIO(_DSCA_PPS_0 + (pps + 12) * 4))
>> > +#define DSCC_PPS(pps)                                ((pps < 12) ? _MMIO(_DSCC_PPS_0 + (pps) * 4):_MMIO(_DSCC_PPS_0 + (pps + 12) * 4))
>>
>> There's no need to duplicate so much here, this could be just:
>>
>>         _MMIO(_DSCC_PPS_0 + ((pps) < 12 ? (pps) : (pps) + 12) * 4)
>
> Yes thanks for suggesting the simplification
>
>
>>
>> Also the macro arguments need to be wrapped in braces.
>
> Your above suggestion should work as is right?
> #define DSCC_PPS(pps)                 _MMIO(_DSCC_PPS_0 + ((pps) < 12
> ? (pps) : (pps) + 12) * 4)
>
> Where are you suggesting extra braces?

I've added them in my suggestion.

The original had (pps < 12) and (pps + 12), which would fail if someone
passed in an expression with a lower precedence than < or +.

BR,
Jani.


-- 
Jani Nikula, Intel