From: Matt Roper <matthew.d.roper@intel.com> To: Badal Nilawar <badal.nilawar@intel.com> Cc: andi.shyti@intel.com, anshuman.gupta@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, ashutosh.dixit@intel.com, jon.ewins@intel.com, rodrigo.vivi@intel.com, vinay.belgaumkar@intel.com Subject: Re: [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL Date: Mon, 19 Sep 2022 15:49:17 -0700 [thread overview] Message-ID: <Yyjx7Tgto5rpAM+B@mdroper-desk1.amr.corp.intel.com> (raw) In-Reply-To: <YyjxVHRRTOZAjQod@mdroper-desk1.amr.corp.intel.com> On Mon, Sep 19, 2022 at 03:46:47PM -0700, Matt Roper wrote: > On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote: > > Updated the CAGF functions to get actual resolved frequency of > > 3D and SAMedia > > > > Bspec: 66300 > > > > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++ > > drivers/gpu/drm/i915/gt/intel_rps.c | 6 +++++- > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > index 2275ee47da95..7819d32db956 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > @@ -1510,6 +1510,14 @@ > > #define VLV_RENDER_C0_COUNT _MMIO(0x138118) > > #define VLV_MEDIA_C0_COUNT _MMIO(0x13811c) > > > > +/* > > + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/ > > + * 3D - 0x0C60 , SAMedia - 0x380C60 > > + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE > > + */ Also, this comment is unnecessary. This is already how all GT registers work so there's no reason to state this again on one one random register. > > +#define MTL_MIRROR_TARGET_WP1 _MMIO(0x0C60) > > +#define MTL_CAGF_MASK REG_GENMASK(8, 0) > > + > > This register is at the wrong place in the file (and is misformatted). > - Keep it sorted with respect to the other registers in the file. > - Write it as "0xc60" for consistency with all the other registers > (i.e., lower-case hex, no unnecessary 0 prefix). > - The whitespace between the name and the REG_GENMASK should be tabs, > not spaces, ensuring it's lined up with the other definitions. > > i915_reg.h turned into a huge mess over time because it wasn't > consistently organized or formatted so nobody knew what to do when > adding new registers. We're trying to do a better job of following > consistent rules with the new register headers so that we don't wind up > with the same confusion again. > > > #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4)) > > #define GEN11_CSME (31) > > #define GEN11_GUNIT (28) > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c > > index 17b40b625e31..c2349949ebae 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > > @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat) > > > > if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) > > cagf = (rpstat >> 8) & 0xff; > > + else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) > > + cagf = rpstat & MTL_CAGF_MASK; > > Generally we try to put the newer platform at the top of if/else > ladders. So this new MTL code should come before the VLV/CHV branch. > > > else if (GRAPHICS_VER(i915) >= 9) > > cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT; > > else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) > > @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps) > > vlv_punit_get(i915); > > freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS); > > vlv_punit_put(i915); > > - } else if (GRAPHICS_VER(i915) >= 6) { > > + } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) > > + freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1); > > Same here. > > > Matt > > > + else if (GRAPHICS_VER(i915) >= 6) { > > freq = intel_uncore_read(uncore, GEN6_RPSTAT1); > > } else { > > freq = intel_uncore_read(uncore, MEMSTAT_ILK); > > -- > > 2.25.1 > > > > -- > Matt Roper > Graphics Software Engineer > VTT-OSGC Platform Enablement > Intel Corporation -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation
WARNING: multiple messages have this Message-ID (diff)
From: Matt Roper <matthew.d.roper@intel.com> To: Badal Nilawar <badal.nilawar@intel.com> Cc: andi.shyti@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, rodrigo.vivi@intel.com Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL Date: Mon, 19 Sep 2022 15:49:17 -0700 [thread overview] Message-ID: <Yyjx7Tgto5rpAM+B@mdroper-desk1.amr.corp.intel.com> (raw) In-Reply-To: <YyjxVHRRTOZAjQod@mdroper-desk1.amr.corp.intel.com> On Mon, Sep 19, 2022 at 03:46:47PM -0700, Matt Roper wrote: > On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote: > > Updated the CAGF functions to get actual resolved frequency of > > 3D and SAMedia > > > > Bspec: 66300 > > > > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++ > > drivers/gpu/drm/i915/gt/intel_rps.c | 6 +++++- > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > index 2275ee47da95..7819d32db956 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > @@ -1510,6 +1510,14 @@ > > #define VLV_RENDER_C0_COUNT _MMIO(0x138118) > > #define VLV_MEDIA_C0_COUNT _MMIO(0x13811c) > > > > +/* > > + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/ > > + * 3D - 0x0C60 , SAMedia - 0x380C60 > > + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE > > + */ Also, this comment is unnecessary. This is already how all GT registers work so there's no reason to state this again on one one random register. > > +#define MTL_MIRROR_TARGET_WP1 _MMIO(0x0C60) > > +#define MTL_CAGF_MASK REG_GENMASK(8, 0) > > + > > This register is at the wrong place in the file (and is misformatted). > - Keep it sorted with respect to the other registers in the file. > - Write it as "0xc60" for consistency with all the other registers > (i.e., lower-case hex, no unnecessary 0 prefix). > - The whitespace between the name and the REG_GENMASK should be tabs, > not spaces, ensuring it's lined up with the other definitions. > > i915_reg.h turned into a huge mess over time because it wasn't > consistently organized or formatted so nobody knew what to do when > adding new registers. We're trying to do a better job of following > consistent rules with the new register headers so that we don't wind up > with the same confusion again. > > > #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4)) > > #define GEN11_CSME (31) > > #define GEN11_GUNIT (28) > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c > > index 17b40b625e31..c2349949ebae 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > > @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat) > > > > if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) > > cagf = (rpstat >> 8) & 0xff; > > + else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) > > + cagf = rpstat & MTL_CAGF_MASK; > > Generally we try to put the newer platform at the top of if/else > ladders. So this new MTL code should come before the VLV/CHV branch. > > > else if (GRAPHICS_VER(i915) >= 9) > > cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT; > > else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) > > @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps) > > vlv_punit_get(i915); > > freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS); > > vlv_punit_put(i915); > > - } else if (GRAPHICS_VER(i915) >= 6) { > > + } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) > > + freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1); > > Same here. > > > Matt > > > + else if (GRAPHICS_VER(i915) >= 6) { > > freq = intel_uncore_read(uncore, GEN6_RPSTAT1); > > } else { > > freq = intel_uncore_read(uncore, MEMSTAT_ILK); > > -- > > 2.25.1 > > > > -- > Matt Roper > Graphics Software Engineer > VTT-OSGC Platform Enablement > Intel Corporation -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation
next prev parent reply other threads:[~2022-09-19 22:49 UTC|newest] Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-09-19 11:59 [PATCH 0/2] i915: CAGF and RC6 changes for MTL Badal Nilawar 2022-09-19 11:59 ` [Intel-gfx] " Badal Nilawar 2022-09-19 11:59 ` [PATCH 1/2] drm/i915/mtl: Modify CAGF functions " Badal Nilawar 2022-09-19 11:59 ` [Intel-gfx] " Badal Nilawar 2022-09-19 16:49 ` Andi Shyti 2022-09-19 17:21 ` Nilawar, Badal 2022-09-19 17:21 ` Nilawar, Badal 2022-10-15 3:34 ` Dixit, Ashutosh 2022-10-15 3:34 ` Dixit, Ashutosh 2022-09-19 22:46 ` Matt Roper 2022-09-19 22:46 ` [Intel-gfx] " Matt Roper 2022-09-19 22:49 ` Matt Roper [this message] 2022-09-19 22:49 ` Matt Roper 2022-10-15 3:34 ` Dixit, Ashutosh 2022-10-15 3:34 ` [Intel-gfx] " Dixit, Ashutosh 2022-09-19 11:59 ` [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia Badal Nilawar 2022-09-19 11:59 ` [Intel-gfx] " Badal Nilawar 2022-09-19 12:13 ` Jani Nikula 2022-09-19 12:13 ` [Intel-gfx] " Jani Nikula 2022-09-20 3:33 ` Dixit, Ashutosh 2022-09-20 3:33 ` [Intel-gfx] " Dixit, Ashutosh 2022-09-20 8:06 ` Jani Nikula 2022-09-20 8:06 ` [Intel-gfx] " Jani Nikula 2022-10-15 3:38 ` Dixit, Ashutosh 2022-10-15 3:38 ` Dixit, Ashutosh 2022-09-19 12:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success for i915: CAGF and RC6 changes for MTL (rev4) Patchwork 2022-09-19 14:32 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " 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=Yyjx7Tgto5rpAM+B@mdroper-desk1.amr.corp.intel.com \ --to=matthew.d.roper@intel.com \ --cc=andi.shyti@intel.com \ --cc=anshuman.gupta@intel.com \ --cc=ashutosh.dixit@intel.com \ --cc=badal.nilawar@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=jon.ewins@intel.com \ --cc=rodrigo.vivi@intel.com \ --cc=vinay.belgaumkar@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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.