[Intel-gfx] [PATCH 3/5] drm/i915/kbl: Kabylake A0 is based on Skylake H0.

[Rodrigo Vivi]

cc'ing Ben to get his opinion...

On Tue, Oct 6, 2015 at 10:43 AM Vivi, Rodrigo <rodrigo.vivi at intel.com>
wrote:

> On Tue, 2015-10-06 at 12:24 +0300, Jani Nikula wrote:
> > On Tue, 06 Oct 2015, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> > > Kabylake is gen 9.5 derivated from Skylake H0 stepping.
> > >
> > > So we don't need pre-production Skylake workaround and also
> > > firmware loading will use SKL H0 offsets.
> > >
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 7374a0d..580c005 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2436,7 +2436,6 @@ struct drm_i915_cmd_table {
> > >  })
> > >  #define INTEL_INFO(p)      (&__I915__(p)->info)
> > >  #define INTEL_DEVID(p)     (INTEL_INFO(p)->device_id)
> > > -#define INTEL_REVID(p)     (__I915__(p)->dev->pdev->revision)
> > >
> > >  #define IS_I830(dev)               (INTEL_DEVID(dev) == 0x3577)
> > >  #define IS_845G(dev)               (INTEL_DEVID(dev) == 0x2562)
> > > @@ -2508,6 +2507,9 @@ struct drm_i915_cmd_table {
> > >
> > >  #define IS_PRELIMINARY_HW(intel_info) ((intel_info)
> > > ->is_preliminary)
> > >
> > > +#define INTEL_REVID(p)     (__I915__(p)->dev->pdev->revision +
> > > \
> > > +                    IS_KABYLAKE(p) ? 7 : 0)
> > > +
> >
> > I am not fond of this at all. It will be really confusing that
> > ->revision is different from INTEL_REVID when checking the
> > workarounds,
> > and that you'll be using SKL_REVID_* to match KBL revision
> > ids.
>
> this is exactly one of the reasons why I did this sum in this way so
> they never match...
>
> > Additionally, we'll probably want to start removing SKL workarounds
> > before KBL workarounds.
>
> I believe this is another discussion... On HSW BDW I remember I was
> removing old Wa as it was no longer needed, but on SKL I saw this REVID
> and I believed the idea was to let them there since some devs might be
> using preliminary platforms yet for other reasons... I don't see a
> problem of letting the old W/a there.
>
> >
> > Others may disagree, but I'd like KBL revid checks be different from
> > SKL.
> >
> > >  #define SKL_REVID_A0               (0x0)
> > >  #define SKL_REVID_B0               (0x1)
> > >  #define SKL_REVID_C0               (0x2)
> > > @@ -2515,6 +2517,9 @@ struct drm_i915_cmd_table {
> > >  #define SKL_REVID_E0               (0x4)
> > >  #define SKL_REVID_F0               (0x5)
> > >
> > > +/* KBL A0 is based on SKL H0 */
> > > +#define KBL_REVID_A0               (0x7)
> >
> > You can't compare this against INTEL_REVID() now can you...? Or is
> > this
> > not the one in the spec? Confused already.
>
> Yes, this is confusing indeed. It seems that we have many levels of
> steppings (according to platform guys) and this platform stepping
> returning 0 is our KBL A0, but this correspond to our internal gpu
> stepping H0 (same going to skl h0).
>
> Like dmc firmware loading for instance we need to load the firmware for
> stepping 7.
>
> So yes, this definition matches BSPec KBL A0.
>
> >
> > BR,
> > Jani.
> >
> > > +
> > >  #define BXT_REVID_A0               (0x0)
> > >  #define BXT_REVID_B0               (0x3)
> > >  #define BXT_REVID_C0               (0x9)
> > > --
> > > 2.4.3
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20151006/b02cd3fa/attachment-0001.html>