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@xxxxxxxxxxxxxxx> wrote: > > On Thu, 01 Feb 2024, Manasi Navare <navaremanasi@xxxxxxxxxxxx> wrote: > > On Thu, Feb 1, 2024 at 1:15 AM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: > >> On Wed, 31 Jan 2024, Manasi Navare <navaremanasi@xxxxxxxxxxxx> 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
