On Wed, 21 Sep 2022 08:07:15 -0700, Gupta, Anshuman wrote: > Hi Anshuman, > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 55c35903adca..956e5298ef1e 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -6644,6 +6644,12 @@ > > #define DG1_PCODE_STATUS 0x7E > > #define DG1_UNCORE_GET_INIT_STATUS 0x0 > > #define DG1_UNCORE_INIT_STATUS_COMPLETE 0x1 > > +#define PCODE_POWER_SETUP 0x7C > > +#define POWER_SETUP_SUBCOMMAND_READ_I1 0x4 > > +#define POWER_SETUP_SUBCOMMAND_WRITE_I1 0x5 > > +#define POWER_SETUP_I1_WATTS REG_BIT(31) > > +#define POWER_SETUP_I1_SHIFT 6 /* 10.6 fixed point format */ > Could please add some comment to explain, why POWER_SETUP_I1_SHIFT = 6, > what is excatly 10.6 fixed point format ? > With that. > Reviewed-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx> 10.6 fixed point format means a 16 bit number is represented as x.y where x are the top 10 bits and y are the bottom 6 bits. The float value of this 16 bit number is (x + (y / 2^6)), so (x + (y / 64)). For example the number 0x8008 will have the value (1 * 2^9 + 8 / 2^6) == 512.125. Note that the hexadecimal number 0x8008 == 32776 and 512.125 == 32776 / 64 which is why POWER_SETUP_I1_SHIFT is 6 (2^6 == 64). Similarly, the 8.8 fixed point format is explained in gt/intel_gt_sysfs_pm.c. Do you think this needs a comment? I thought "10.6 fixed point format" is a sufficient hint (fixed point numbers are fairly well known). An even trickier data format is in the patch "drm/i915/hwmon: Expose power1_max_interval" in hwm_power1_max_interval_show() but I think I have a long comment there. Thanks. -- Ashutosh
