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

From: Jani Nikula <jani.nikula@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 3/5] drm/i915/kbl: Kabylake A0 is based on	Skylake H0.
Date: Tue, 06 Oct 2015 12:24:20 +0300	[thread overview]
Message-ID: <8737xo9wor.fsf@intel.com> (raw)
In-Reply-To: <1444085055-27447-4-git-send-email-rodrigo.vivi@intel.com>

On Tue, 06 Oct 2015, Rodrigo Vivi <rodrigo.vivi@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@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. Additionally, we'll probably want to start removing SKL workarounds
before KBL workarounds.

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.

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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx