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

Manasi Navare navaremanasi at chromium.org
Fri Feb 2 20:32:22 UTC 2024


Thanks Jani, that makes sense and thanks for adding them in your suggestion.

I have made the necessary changes addressing all your review comments
and will send out a V2 for the patch.

Regards
Manasi

On Thu, Feb 1, 2024 at 3:34 PM Jani Nikula <jani.nikula at linux.intel.com> wrote:
>
> On Thu, 01 Feb 2024, Manasi Navare <navaremanasi at chromium.org> wrote:
> > On Thu, Feb 1, 2024 at 1:15 AM Jani Nikula <jani.nikula at linux.intel.com> wrote:
> >> On Wed, 31 Jan 2024, Manasi Navare <navaremanasi at 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


More information about the Intel-gfx mailing list