cc'ing Ben to get his opinion... On Tue, Oct 6, 2015 at 10:43 AM Vivi, Rodrigo <[email protected]> wrote:
> On Tue, 2015-10-06 at 12:24 +0300, Jani Nikula wrote: > > On Tue, 06 Oct 2015, Rodrigo Vivi <[email protected]> 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 <[email protected]> > > > --- > > > 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 > > > [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
_______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
