Re: [PATCH] drm/i915: Add haswell_pcode_write function

O'Rourke, Tom
2014-11-06T17:40:06Z

On Thu, Nov 06, 2014 at 10:28:53AM +0200, Ville Syrjälä wrote:
> On Wed, Nov 05, 2014 at 05:32:44PM -0800, Tom.O'Rourke@intel.com wrote:
> > From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >
> > Based on sandybridge_pcode_write, haswell_pcode_write has an
> > additional field for address control.
>
> It's already there in snb.
If the address control field is already there for SNB,
then I would prefer to modify sandybridge_pcode_write
instead of adding a haswell_pcode_write.

I based this patch on the Haswell documentation for
PCU_CR_GTDRIVER_MAILBOX_INTERFACE_0_2_0_GTTMMADR on
page 101 of
https://01.org/linuxgraphics/sites/default/files/documentation/intel-gfx-prm-osrc-hsw-pcie-config-registers.pdf

Can you point me to the SNB documentation for the
address control field? I do not have documentation
for address control for earlier than Haswell.

>
> Do you have an actual use case for this? If so I wonder if we should
> just change the mbox parameter to u32 and allow the caller to specify
> it all?
>
No, I do not have an actual use case for this yet.
I am working on something that will use it but that
is not ready yet.

Yes, we could change mbox to u32 and shift the work
to the caller. The caller would need to combine the
u8 mailbox command with the address control field.
I do not think that would be better (or worse) but
I can make that change if others want it.

> >
> > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 9 +++++++--
> > 3 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 0f00e58..fd8b550 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2950,6 +2950,7 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine);
> > void assert_force_wake_inactive(struct drm_i915_private *dev_priv);
> >
> > int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val);
> > +int haswell_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val, u32 control);
> > int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
> >
> > /* intel_sideband.c */
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 6fbfdec..b674050 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6011,6 +6011,7 @@ enum punit_power_well {
> > #define GEN6_DECODE_RC6_VID(vids) (((vids) * 5) + 245)
> > #define DISPLAY_IPS_CONTROL 0x19
> > #define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A
> > +#define HSW_PCODE_ADDR_CNTL(cntl) ((cntl << 8) & 0x1fffff00)
> > #define GEN6_PCODE_DATA 0x138128
> > #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8
> > #define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 1244ff8..9c47bc8 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -7277,7 +7277,7 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
> > return 0;
> > }
> >
> > -int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val)
> > +int haswell_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val, u32 control)
> > {
> > WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> >
> > @@ -7287,7 +7287,7 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val)
> > }
> >
> > I915_WRITE(GEN6_PCODE_DATA, val);
> > - I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
> > + I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox | HSW_PCODE_ADDR_CNTL(control));
> >
> > if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> > 500)) {
> > @@ -7300,6 +7300,11 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val)
> > return 0;
> > }
> >
> > +int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val)
> > +{
> > + return haswell_pcode_write(dev_priv, mbox, val, 0);
> > +}
> > +
> > static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
> > {
> > int div;
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC