intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Manasi Navare <navaremanasi@chromium.org>
Cc: intel-gfx@lists.freedesktop.org,
	Suraj Kandpal <suraj.kandpal@intel.com>,
	Ankit Nautiyal <ankit.k.nautiyal@intel.com>,
	Animesh Manna <animesh.manna@intel.com>,
	Sean Paul <sean@poorly.run>,
	Drew Davenport <ddavenport@chromium.org>
Subject: Re: [PATCH] drm/i915/display/vdsc: Fix the macro that calculates DSCC_/DSCA_ PPS reg address
Date: Fri, 02 Feb 2024 01:34:09 +0200	[thread overview]
Message-ID: <87sf2bg3ny.fsf@intel.com> (raw)
In-Reply-To: <CAE72mN=xxBKGyU++pmez3fGkvuY5o8rz5OHAirxpUC_ksT3dBw@mail.gmail.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

  reply	other threads:[~2024-02-01 23:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-31 21:19 [PATCH] drm/i915/display/vdsc: Fix the macro that calculates DSCC_/DSCA_ PPS reg address Manasi Navare
2024-02-01  9:15 ` Jani Nikula
2024-02-01 20:16   ` Manasi Navare
2024-02-01 23:34     ` Jani Nikula [this message]
2024-02-02 20:32       ` Manasi Navare
2024-02-01 17:15 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2024-02-01 20:41 ` ✓ Fi.CI.BAT: success " Patchwork
2024-02-01 22:52 ` ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87sf2bg3ny.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=animesh.manna@intel.com \
    --cc=ankit.k.nautiyal@intel.com \
    --cc=ddavenport@chromium.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=navaremanasi@chromium.org \
    --cc=sean@poorly.run \
    --cc=suraj.kandpal@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).