> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit@xxxxxxxxx>
> Sent: Monday, December 5, 2022 7:10 AM
> To: Gupta, Anshuman <anshuman.gupta@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Nilawar, Badal
> <badal.nilawar@xxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>
> Subject: Re: [PATCH] drm/i915/hwmon: Silence "mailbox access failed"
> warning in snb_pcode_read
>
> On Sat, 03 Dec 2022 01:47:06 -0800, Gupta, Anshuman wrote:
> >
>
> Hi Anshuman,
>
> > > > hwm_pcode_read_i1 is called during i915 load. This results in the
> > > > following warning from snb_pcode_read because
> > > > POWER_SETUP_SUBCOMMAND_READ_I1 is unsupported on DG1/DG2.
> > > >
> > > > [drm:snb_pcode_read [i915]] warning: pcode (read from mbox 47c) \
> > > > mailbox access failed for snb_pcode_read_p
> > > > [i915]: -6
> > > >
> > > > The code handles the unsupported command but the warning in dmesg
> > > > is a red herring which has resulted in a couple of bugs being filed.
> > > > Therefore silence the warning by avoiding calling snb_pcode_read_p
> > > > for > > DG1/DG2.
> > > >
> > > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_hwmon.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c
> > > > b/drivers/gpu/drm/i915/i915_hwmon.c
> > > > index c588a17f97e98..cca7a4350ec8f 100644
> > > > --- a/drivers/gpu/drm/i915/i915_hwmon.c
> > > > +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> > > > @@ -293,6 +293,10 @@ static const struct hwmon_channel_info
> > > > *hwm_gt_info[] = {
> > > > /* I1 is exposed as power_crit or as curr_crit depending on bit
> > > > 31 */ static int hwm_pcode_read_i1(struct drm_i915_private *i915,
> > > > u32 *uval) {
> > > > + /* Avoid ILLEGAL_SUBCOMMAND "mailbox access failed" warning in
> > > > snb_pcode_read */
> > > > + if (IS_DG1(i915) || IS_DG2(i915))
> > > > + return -ENXIO;
> > >
> > > AFAIK it is specific to client specific parts,
>
> No this is not true, see c8939848f7e4 where it says I1 power is exposed as
> power1_crit for client products and as curr1_crit for server. Also I know this is
> available on future client products. So it appears only DG1 and DG2 were an
> exception because of lack of HW support, this will available in the future for
> client. Therefore the patch looks correct to me.
>
> > > how about declaring a is_client intel_runtime_info flag to
> > > distinguish between client and server part. That *intel_device_info*
> > > will also cover any future platform as well.
Thanks for explanation.
Reviewed-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
>
> Thanks.
> --
> Ashutosh
