Re: [Intel-gfx] [PATCH v5 7/7] drm/i915/gt: Adding new sysfs frequency attributes

[Andi Shyti]

Hi,

I forgot to add some note to this patch...

[...]

> +static ssize_t throttle_reason_status_show(struct device *dev,
> +                                        struct device_attribute *attr,
> +                                        char *buff)
> +{
> +     struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +     struct intel_rps *rps = &gt->rps;
> +     bool status = !!intel_rps_read_throttle_reason_status(rps);

why are these boolean? Can't we send whatever we read from the
register?

[...]

> +#define GT0_PERF_LIMIT_REASONS               _MMIO(0x1381A8)
> +#define   GT0_PERF_LIMIT_REASONS_MASK        0x00000de3

This mask is really weird! Sujaritha, can you please explain it?

It looks something like this,

  REG_GENMASK(11, 6) | REG_GENMASK(2, 0)

But I don't know if it improves any readability, in any case, the
mask is not clear.

> +#define   PROCHOT_MASK                       BIT(1)
> +#define   THERMAL_LIMIT_MASK         BIT(2)
> +#define   RATL_MASK                  BIT(6)
> +#define   VR_THERMALERT_MASK         BIT(7)
> +#define   VR_TDC_MASK                        BIT(8)
> +#define   POWER_LIMIT_4_MASK         BIT(9)
> +#define   POWER_LIMIT_1_MASK         BIT(11)
> +#define   POWER_LIMIT_2_MASK         BIT(12)

I hope I got these right. Sujaritha, can you please check?

Andi