[PATCH v2 1/2] drm/xe: Make read_perf_limit_reasons globally accessible

Rodrigo Vivi rodrigo.vivi at intel.com
Wed Feb 14 13:54:54 UTC 2024


On Tue, Feb 13, 2024 at 06:12:11PM -0800, John Harrison wrote:
> On 2/13/2024 07:22, Rodrigo Vivi wrote:
> > On Mon, Feb 12, 2024 at 04:34:25PM -0800, John.C.Harrison at Intel.com wrote:
> > > From: John Harrison <John.C.Harrison at Intel.com>
> > > 
> > > Other driver code beyond the sysfs interface wants to know about
> > > throttling. So move the query function out of sysfs.
> > > 
> > > Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_gt_freq.c           | 15 +++++++++++
> > >   drivers/gpu/drm/xe/xe_gt_freq.h           |  4 +++
> > >   drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c | 31 ++++++++---------------
> > >   3 files changed, 29 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_freq.c b/drivers/gpu/drm/xe/xe_gt_freq.c
> > > index e5b0f4ecdbe8..51645a24009e 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_freq.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt_freq.c
> > > @@ -11,7 +11,10 @@
> > >   #include <drm/drm_managed.h>
> > >   #include <drm/drm_print.h>
> > > +#include "regs/xe_gt_regs.h"
> > >   #include "xe_device_types.h"
> > > +#include "xe_mmio.h"
> > > +#include "xe_gt.h"
> > >   #include "xe_gt_sysfs.h"
> > >   #include "xe_gt_throttle_sysfs.h"
> > >   #include "xe_guc_pc.h"
> > > @@ -220,3 +223,15 @@ void xe_gt_freq_init(struct xe_gt *gt)
> > >   	xe_gt_throttle_sysfs_init(gt);
> > >   }
> > > +
> > > +u32 xe_read_perf_limit_reasons(struct xe_gt *gt)
> > > +{
> > > +	u32 reg;
> > > +
> > > +	if (xe_gt_is_media_type(gt))
> > > +		reg = xe_mmio_read32(gt, MTL_MEDIA_PERF_LIMIT_REASONS);
> > > +	else
> > > +		reg = xe_mmio_read32(gt, GT0_PERF_LIMIT_REASONS);
> > > +
> > > +	return reg;
> > > +}
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_freq.h b/drivers/gpu/drm/xe/xe_gt_freq.h
> > > index f3fe3c90491a..89be518b4967 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_freq.h
> > > +++ b/drivers/gpu/drm/xe/xe_gt_freq.h
> > > @@ -6,8 +6,12 @@
> > >   #ifndef _XE_GT_FREQ_H_
> > >   #define _XE_GT_FREQ_H_
> > > +#include <linux/types.h>
> > > +
> > >   struct xe_gt;
> > >   void xe_gt_freq_init(struct xe_gt *gt);
> > > +u32 xe_read_perf_limit_reasons(struct xe_gt *gt);
> > this component is xe_gt_freq, not xe_ nor xe_read or xe_read_perf.
> > Please use the right namespace prefixes.
> Argh! Forgot to update that.
> 
> > 
> > But besides the namespace, I don't believe that these limits
> > belong to xe_gt_freq anyway... why are you taking that from the
> > throttle reasons and moving here?
> > 
> > What about removing the 'sysfs' name from the xe_gt_throttle
> > component and making that to export a function
> > xe_gt_throttle_get_limit_reasons(struct xe_gt *gt)
> > ?
> Because that seems like a lot of unnecessary churn. The intention is to
> simply add the throttle register to debug prints so that CI bug reports can
> be triaged more effectively. Re-writing the entire sysfs file to split it up
> into multiple units is not worth the effort when all that is required is to
> export the function to read a register.

No, that's not what I meant... I don't believe we need to split the file.
just remove the 'sysfs' portion of the name of the component itself,
so we have a component responsible for providing throttle information, either
with direct exposing the sysfs and also with the exported functions that can
be accessed by any other component that might make usage of the throttle
information.

> 
> John.
> 
> 
> > 
> > Thanks,
> > Rodrigo.
> > 
> > > +
> > >   #endif
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c b/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> > > index 63d640591a52..89d9f89962ad 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> > > @@ -8,6 +8,7 @@
> > >   #include <regs/xe_gt_regs.h>
> > >   #include "xe_device.h"
> > >   #include "xe_gt.h"
> > > +#include "xe_gt_freq.h"
> > >   #include "xe_gt_sysfs.h"
> > >   #include "xe_gt_throttle_sysfs.h"
> > >   #include "xe_mmio.h"
> > > @@ -34,77 +35,65 @@ dev_to_gt(struct device *dev)
> > >   	return kobj_to_gt(dev->kobj.parent);
> > >   }
> > > -static u32 read_perf_limit_reasons(struct xe_gt *gt)
> > > -{
> > > -	u32 reg;
> > > -
> > > -	if (xe_gt_is_media_type(gt))
> > > -		reg = xe_mmio_read32(gt, MTL_MEDIA_PERF_LIMIT_REASONS);
> > > -	else
> > > -		reg = xe_mmio_read32(gt, GT0_PERF_LIMIT_REASONS);
> > > -
> > > -	return reg;
> > > -}
> > > -
> > >   static u32 read_status(struct xe_gt *gt)
> > >   {
> > > -	u32 status = read_perf_limit_reasons(gt) & GT0_PERF_LIMIT_REASONS_MASK;
> > > +	u32 status = xe_read_perf_limit_reasons(gt) & GT0_PERF_LIMIT_REASONS_MASK;
> > >   	return status;
> > >   }
> > >   static u32 read_reason_pl1(struct xe_gt *gt)
> > >   {
> > > -	u32 pl1 = read_perf_limit_reasons(gt) & POWER_LIMIT_1_MASK;
> > > +	u32 pl1 = xe_read_perf_limit_reasons(gt) & POWER_LIMIT_1_MASK;
> > >   	return pl1;
> > >   }
> > >   static u32 read_reason_pl2(struct xe_gt *gt)
> > >   {
> > > -	u32 pl2 = read_perf_limit_reasons(gt) & POWER_LIMIT_2_MASK;
> > > +	u32 pl2 = xe_read_perf_limit_reasons(gt) & POWER_LIMIT_2_MASK;
> > >   	return pl2;
> > >   }
> > >   static u32 read_reason_pl4(struct xe_gt *gt)
> > >   {
> > > -	u32 pl4 = read_perf_limit_reasons(gt) & POWER_LIMIT_4_MASK;
> > > +	u32 pl4 = xe_read_perf_limit_reasons(gt) & POWER_LIMIT_4_MASK;
> > >   	return pl4;
> > >   }
> > >   static u32 read_reason_thermal(struct xe_gt *gt)
> > >   {
> > > -	u32 thermal = read_perf_limit_reasons(gt) & THERMAL_LIMIT_MASK;
> > > +	u32 thermal = xe_read_perf_limit_reasons(gt) & THERMAL_LIMIT_MASK;
> > >   	return thermal;
> > >   }
> > >   static u32 read_reason_prochot(struct xe_gt *gt)
> > >   {
> > > -	u32 prochot = read_perf_limit_reasons(gt) & PROCHOT_MASK;
> > > +	u32 prochot = xe_read_perf_limit_reasons(gt) & PROCHOT_MASK;
> > >   	return prochot;
> > >   }
> > >   static u32 read_reason_ratl(struct xe_gt *gt)
> > >   {
> > > -	u32 ratl = read_perf_limit_reasons(gt) & RATL_MASK;
> > > +	u32 ratl = xe_read_perf_limit_reasons(gt) & RATL_MASK;
> > >   	return ratl;
> > >   }
> > >   static u32 read_reason_vr_thermalert(struct xe_gt *gt)
> > >   {
> > > -	u32 thermalert = read_perf_limit_reasons(gt) & VR_THERMALERT_MASK;
> > > +	u32 thermalert = xe_read_perf_limit_reasons(gt) & VR_THERMALERT_MASK;
> > >   	return thermalert;
> > >   }
> > >   static u32 read_reason_vr_tdc(struct xe_gt *gt)
> > >   {
> > > -	u32 tdc = read_perf_limit_reasons(gt) & VR_TDC_MASK;
> > > +	u32 tdc = xe_read_perf_limit_reasons(gt) & VR_TDC_MASK;
> > >   	return tdc;
> > >   }
> > > -- 
> > > 2.43.0
> > > 
> 


More information about the Intel-xe mailing list