On Mon, 2020-08-24 at 12:24 -0700, Lucas De Marchi wrote:
> On Mon, Aug 03, 2020 at 04:24:17PM -0700, Jose Souza wrote:
> > On Fri, 2020-07-24 at 14:39 -0700, Lucas De Marchi wrote:
> > > From: Matt Roper <
> > > matthew.d.roper@xxxxxxxxx
> > >
> > >
> > > DG1 does some additional pcode/uncore handshaking at
> > > boot time; this handshaking must complete before various other pcode
> > > commands are effective and before general work is submitted to the GPU.
> > > We need to poll a new pcode mailbox during startup until it reports that
> > > this handshaking is complete.
> > >
> > > The bspec doesn't give guidance on how long we may need to wait for this
> > > handshaking to complete. For now, let's just set a really long timeout;
> > > if we still don't get a completion status by the end of that timeout,
> > > we'll just continue on and hope for the best.
> > >
> > > Bspec: 52065
> > > Cc: Clinton Taylor <
> > > Clinton.A.Taylor@xxxxxxxxx
> > >
> > >
> > > Cc: Ville Syrjälä <
> > > ville.syrjala@xxxxxxxxxxxxxxx
> > >
> > >
> > > Cc: Radhakrishna Sripada <
> > > radhakrishna.sripada@xxxxxxxxx
> > >
> > >
> > > Signed-off-by: Matt Roper <
> > > matthew.d.roper@xxxxxxxxx
> > >
> > >
> > > Signed-off-by: Lucas De Marchi <
> > > lucas.demarchi@xxxxxxxxx
> > >
> > >
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 3 +++
> > > drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > > drivers/gpu/drm/i915/intel_sideband.c | 15 +++++++++++++++
> > > drivers/gpu/drm/i915/intel_sideband.h | 2 ++
> > > 4 files changed, 23 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index 5fd5af4bc855..5473bfe9126c 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -85,6 +85,7 @@
> > > #include "intel_gvt.h"
> > > #include "intel_memory_region.h"
> > > #include "intel_pm.h"
> > > +#include "intel_sideband.h"
> > > #include "vlv_suspend.h"
> > >
> > > static struct drm_driver driver;
> > > @@ -737,6 +738,8 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
> > > */
> > > intel_dram_detect(dev_priv);
> > >
> > > + intel_pcode_init(dev_priv);
> > > +
> > > intel_bw_init_hw(dev_priv);
> > >
> > > return 0;
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index a0d31f3bf634..3767b32127da 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -9245,6 +9245,9 @@ enum {
> > > #define GEN9_SAGV_DISABLE 0x0
> > > #define GEN9_SAGV_IS_DISABLED 0x1
> > > #define GEN9_SAGV_ENABLE 0x3
> > > +#define DG1_PCODE_STATUS 0x7E
> > > +#define DG1_CHECK_UNCORE_INIT_STATUS 0x0
> > > +#define DG1_UNCORE_INIT_COMPLETE 0x1
> >
> > With s/DG1_CHECK_UNCORE_INIT_STATUS/DG1_CHECK_UNCORE_INIT_STATUS_COMPLETE or something similar that makes easy to understand that 0x1 is the response
> > of the DG1_CHECK_UNCORE_INIT_STATUS sub-command.
>
> checking all the other users of skl_pcode_request() I don't see a
> pattern there. Examples:
>
> ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> SKL_CDCLK_PREPARE_FOR_CHANGE,
> SKL_CDCLK_READY_FOR_CHANGE,
> SKL_CDCLK_READY_FOR_CHANGE, 3);
>
> ret = skl_pcode_request(dev_priv, GEN9_PCODE_SAGV_CONTROL,
> GEN9_SAGV_DISABLE,
> GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED,
> 1);
>
> Giveng the current uses, I'd rather rename like:
>
> +#define DG1_PCODE_STATUS 0x7E
> +#define DG1_UNCORE_GET_INIT_STATUS 0x0
> +#define DG1_UNCORE_INIT_STATUS_COMPLETE 0x1
>
>
> > Reviewed-by: José Roberto de Souza <
> > jose.souza@xxxxxxxxx
> > >
>
> does that still stands with the rename above?
LGTM, keep it please.
>
> thanks
> Lucas De Marchi
>
> >
> > > #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US 0x23
> > > #define GEN6_PCODE_DATA _MMIO(0x138128)
> > > #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8
> > > diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> > > index 916ccd1c0e96..8b093525240d 100644
> > > --- a/drivers/gpu/drm/i915/intel_sideband.c
> > > +++ b/drivers/gpu/drm/i915/intel_sideband.c
> > > @@ -543,3 +543,18 @@ int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> > > return ret ? ret : status;
> > > #undef COND
> > > }
> > > +
> > > +void intel_pcode_init(struct drm_i915_private *i915)
> > > +{
> > > + int ret;
> > > +
> > > + if (!IS_DGFX(i915))
> > > + return;
> > > +
> > > + ret = skl_pcode_request(i915, DG1_PCODE_STATUS,
> > > + DG1_CHECK_UNCORE_INIT_STATUS,
> > > + DG1_UNCORE_INIT_COMPLETE,
> > > + DG1_UNCORE_INIT_COMPLETE, 50);
> > > + if (ret)
> > > + drm_err(&i915->drm, "Pcode did not report uncore initialization completion!\n");
> > > +}
> > > diff --git a/drivers/gpu/drm/i915/intel_sideband.h b/drivers/gpu/drm/i915/intel_sideband.h
> > > index 7fb95745a444..094c7b19c5d4 100644
> > > --- a/drivers/gpu/drm/i915/intel_sideband.h
> > > +++ b/drivers/gpu/drm/i915/intel_sideband.h
> > > @@ -138,4 +138,6 @@ int sandybridge_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox,
> > > int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> > > u32 reply_mask, u32 reply, int timeout_base_ms);
> > >
> > > +void intel_pcode_init(struct drm_i915_private *i915);
> > > +
> > > #endif /* _INTEL_SIDEBAND_H */
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
