From: Matt Roper <matthew.d.roper@intel.com> To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Jani Nikula <jani.nikula@intel.com>, Intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, Tvrtko Ursulin <tvrtko.ursulin@intel.com> Subject: Re: [PATCH 1/2] drm/i915: Don't use DRM_DEBUG_WARN_ON for unexpected l3bank/mslice config Date: Wed, 4 May 2022 09:48:31 -0700 [thread overview] Message-ID: <YnKuX0F0bDBF5ahP@mdroper-desk1.amr.corp.intel.com> (raw) In-Reply-To: <20220504120715.911045-1-tvrtko.ursulin@linux.intel.com> On Wed, May 04, 2022 at 01:07:14PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > DRM_DEBUG_WARN_ON should only be used when we are certain CI is guaranteed > to exercise a certain code path, so in case of values coming from MMIO > reads we cannot be sure CI will have all the possible SKUs and parts. > > Use drm_warn instead and move logging to init phase while at it. Changing to drm_warn looks good, although moving the location changes the intent a bit; I think originally the idea was to warn if we were trying to do a steering lookup for a type that we never initialized (e.g., an LNCF lookup for a !HAS_MSLICES platform where we never even read the register in the first place). But I don't think we've ever made a mistake that would cause us to trip the warning, so it probably isn't terribly important to keep it there. Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c > index 53307ca0eed0..c474e5c3ea5e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -153,11 +153,14 @@ int intel_gt_init_mmio(struct intel_gt *gt) > * An mslice is unavailable only if both the meml3 for the slice is > * disabled *and* all of the DSS in the slice (quadrant) are disabled. > */ > - if (HAS_MSLICES(i915)) > + if (HAS_MSLICES(i915)) { > gt->info.mslice_mask = > slicemask(gt, GEN_DSS_PER_MSLICE) | > (intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3) & > GEN12_MEML3_EN_MASK); > + if (!gt->info.mslice_mask) /* should be impossible! */ > + drm_warn(&i915->drm, "mslice mask all zero!\n"); > + } > > if (IS_DG2(i915)) { > gt->steering_table[MSLICE] = xehpsdv_mslice_steering_table; > @@ -171,6 +174,8 @@ int intel_gt_init_mmio(struct intel_gt *gt) > gt->info.l3bank_mask = > ~intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3) & > GEN10_L3BANK_MASK; > + if (!gt->info.l3bank_mask) /* should be impossible! */ > + drm_warn(&i915->drm, "L3 bank mask is all zero!\n"); > } else if (HAS_MSLICES(i915)) { > MISSING_CASE(INTEL_INFO(i915)->platform); > } > @@ -882,20 +887,14 @@ static void intel_gt_get_valid_steering(struct intel_gt *gt, > { > switch (type) { > case L3BANK: > - GEM_DEBUG_WARN_ON(!gt->info.l3bank_mask); /* should be impossible! */ > - > *sliceid = 0; /* unused */ > *subsliceid = __ffs(gt->info.l3bank_mask); > break; > case MSLICE: > - GEM_DEBUG_WARN_ON(!gt->info.mslice_mask); /* should be impossible! */ > - > *sliceid = __ffs(gt->info.mslice_mask); > *subsliceid = 0; /* unused */ > break; > case LNCF: > - GEM_DEBUG_WARN_ON(!gt->info.mslice_mask); /* should be impossible! */ > - > /* > * An LNCF is always present if its mslice is present, so we > * can safely just steer to LNCF 0 in all cases. > -- > 2.32.0 > -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795
WARNING: multiple messages have this Message-ID (diff)
From: Matt Roper <matthew.d.roper@intel.com> To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Jani Nikula <jani.nikula@intel.com>, Intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Don't use DRM_DEBUG_WARN_ON for unexpected l3bank/mslice config Date: Wed, 4 May 2022 09:48:31 -0700 [thread overview] Message-ID: <YnKuX0F0bDBF5ahP@mdroper-desk1.amr.corp.intel.com> (raw) In-Reply-To: <20220504120715.911045-1-tvrtko.ursulin@linux.intel.com> On Wed, May 04, 2022 at 01:07:14PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > DRM_DEBUG_WARN_ON should only be used when we are certain CI is guaranteed > to exercise a certain code path, so in case of values coming from MMIO > reads we cannot be sure CI will have all the possible SKUs and parts. > > Use drm_warn instead and move logging to init phase while at it. Changing to drm_warn looks good, although moving the location changes the intent a bit; I think originally the idea was to warn if we were trying to do a steering lookup for a type that we never initialized (e.g., an LNCF lookup for a !HAS_MSLICES platform where we never even read the register in the first place). But I don't think we've ever made a mistake that would cause us to trip the warning, so it probably isn't terribly important to keep it there. Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c > index 53307ca0eed0..c474e5c3ea5e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -153,11 +153,14 @@ int intel_gt_init_mmio(struct intel_gt *gt) > * An mslice is unavailable only if both the meml3 for the slice is > * disabled *and* all of the DSS in the slice (quadrant) are disabled. > */ > - if (HAS_MSLICES(i915)) > + if (HAS_MSLICES(i915)) { > gt->info.mslice_mask = > slicemask(gt, GEN_DSS_PER_MSLICE) | > (intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3) & > GEN12_MEML3_EN_MASK); > + if (!gt->info.mslice_mask) /* should be impossible! */ > + drm_warn(&i915->drm, "mslice mask all zero!\n"); > + } > > if (IS_DG2(i915)) { > gt->steering_table[MSLICE] = xehpsdv_mslice_steering_table; > @@ -171,6 +174,8 @@ int intel_gt_init_mmio(struct intel_gt *gt) > gt->info.l3bank_mask = > ~intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3) & > GEN10_L3BANK_MASK; > + if (!gt->info.l3bank_mask) /* should be impossible! */ > + drm_warn(&i915->drm, "L3 bank mask is all zero!\n"); > } else if (HAS_MSLICES(i915)) { > MISSING_CASE(INTEL_INFO(i915)->platform); > } > @@ -882,20 +887,14 @@ static void intel_gt_get_valid_steering(struct intel_gt *gt, > { > switch (type) { > case L3BANK: > - GEM_DEBUG_WARN_ON(!gt->info.l3bank_mask); /* should be impossible! */ > - > *sliceid = 0; /* unused */ > *subsliceid = __ffs(gt->info.l3bank_mask); > break; > case MSLICE: > - GEM_DEBUG_WARN_ON(!gt->info.mslice_mask); /* should be impossible! */ > - > *sliceid = __ffs(gt->info.mslice_mask); > *subsliceid = 0; /* unused */ > break; > case LNCF: > - GEM_DEBUG_WARN_ON(!gt->info.mslice_mask); /* should be impossible! */ > - > /* > * An LNCF is always present if its mslice is present, so we > * can safely just steer to LNCF 0 in all cases. > -- > 2.32.0 > -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795
next prev parent reply other threads:[~2022-05-04 16:48 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-04 12:07 [PATCH 1/2] drm/i915: Don't use DRM_DEBUG_WARN_ON for unexpected l3bank/mslice config Tvrtko Ursulin 2022-05-04 12:07 ` [Intel-gfx] " Tvrtko Ursulin 2022-05-04 12:07 ` [PATCH 2/2] drm/i915: Don't use DRM_DEBUG_WARN_ON for ring unexpectedly not idle Tvrtko Ursulin 2022-05-04 12:07 ` [Intel-gfx] " Tvrtko Ursulin 2022-05-05 8:02 ` Mika Kuoppala 2022-05-05 8:02 ` [Intel-gfx] " Mika Kuoppala 2022-05-04 13:59 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Don't use DRM_DEBUG_WARN_ON for unexpected l3bank/mslice config Patchwork 2022-05-04 16:48 ` Matt Roper [this message] 2022-05-04 16:48 ` [Intel-gfx] [PATCH 1/2] " Matt Roper 2022-05-04 17:59 ` Tvrtko Ursulin 2022-05-04 17:59 ` [Intel-gfx] " Tvrtko Ursulin 2022-05-04 18:17 ` Matt Roper 2022-05-04 18:17 ` [Intel-gfx] " Matt Roper 2022-05-05 11:02 ` Tvrtko Ursulin 2022-05-05 11:02 ` [Intel-gfx] " Tvrtko Ursulin 2022-05-05 16:05 ` Matt Roper 2022-05-05 16:05 ` [Intel-gfx] " Matt Roper 2022-05-04 20:43 ` [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] " 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=YnKuX0F0bDBF5ahP@mdroper-desk1.amr.corp.intel.com \ --to=matthew.d.roper@intel.com \ --cc=Intel-gfx@lists.freedesktop.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=jani.nikula@intel.com \ --cc=tvrtko.ursulin@intel.com \ --cc=tvrtko.ursulin@linux.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.