Re: [PATCH 1/5] drm/i915/kbl: Add Kabylake PCI ID

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Oct 07, 2015 at 03:55:31PM +0200, Daniel Vetter wrote:
> On Tue, Oct 06, 2015 at 05:31:30PM +0000, Vivi, Rodrigo wrote:
> > On Tue, 2015-10-06 at 12:09 +0300, Jani Nikula wrote:
> > > On Tue, 06 Oct 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > > From: Deepak S <deepak.s@intel.com>
> > > > 
> > > > v2: separate out device info into different GT (Damien)
> > > > v3: Add is_kabylake to the KBL gt3 structuer (Damien)
> > > >     Sort the platforms in older -> newer order (Damien)
> > > > 
> > > > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> > > > Signed-off-by: Deepak S <deepak.s@intel.com>
> > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.c | 33 
> > > > ++++++++++++++++++++++++++++++++-
> > > >  drivers/gpu/drm/i915/i915_drv.h |  2 ++
> > > >  include/drm/i915_pciids.h       | 29 +++++++++++++++++++++++++++++
> > > >  3 files changed, 63 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > index 1cb6b82..f42102d 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -394,6 +394,34 @@ static const struct intel_device_info 
> > > > intel_broxton_info = {
> > > >  	IVB_CURSOR_OFFSETS,
> > > >  };
> > > >  
> > > > +static const struct intel_device_info intel_kabylake_info = {
> > > > +	.is_preliminary = 1,
> > > > +	.is_skylake = 1,
> > > 
> > > Now's the time to call the shots, is this really a good idea or not? 
> > > See
> > > VLV vs. CHV, we (okay, the royal we) still confuse ourselves with
> > > IS_VALLEYVIEW.
> > > 
> > > Granted, 74 call sites for IS_SKYLAKE(), all of those would need to 
> > > be
> > > patched. We'd need something like "is skylake family" including SKL 
> > > and
> > > KBL. Some of them might be changed to be more like feature flags, 
> > > which
> > > is something we've decided we need to do more anyway.
> > 
> > To be honest I also don't like this approach as I didn't like the chv
> > is vlv one. I always get confused and I thought about changing
> > everything many times during last week, but as you pointed out it would
> > be a change in many entries and not sure what others opinion was since
> > I never saw any complain about this patch before neither the vlv-chv
> > ones... 
> > 
> > But if you and Daniel prefer I can do a
> > s/IS_SKYLAKE(dev)/IS_KABYLAKE(dev) || IS_SKYLAKE(dev)
> > Just let me know...
> > 
> > The IS_SKYLAKE_FAMILY I don't like much because it will still confuse
> > people because in ark they will probably be 2 different "families" if
> > they continue with the current scheme they are using so far.
> 
> IS_SKYLAKE_FAMILY == IS_GEN9 pretty much everywhere. The exceptions (where
> bxt is special) are probably worth listing explicitly anyway. So yeah
> IS_KABYLAKE.
> 
> btw the original reasons for putting chv into vlv was that what we really
> wanted is HAS_GMCH but getting there was a lot more work. At least that's
> how I remember that particular story ;-)

IIRC I left it like that mainly to minimize the diff between upstream
and internal when chv was still living there. And I could never come up
with any good IS_<whatever> name to cover both platforms, so didn't do
such a change upfront in upstream.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx