[Intel-gfx] [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro

[Ville Syrjälä]

On Fri, May 06, 2016 at 03:44:05PM +0100, Tvrtko Ursulin wrote:
> 
> On 06/05/16 15:29, Ville Syrjälä wrote:
> > On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
> >>
> >> On 06/05/16 15:00, Ville Syrjälä wrote:
> >>> On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>>>
> >>>> If we allow it a dedicated flag in dev_priv we enable the
> >>>> compiler to nicely optimize conditions like IS_HASSWELL ||
> >>>> IS_BROADWELL.
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>>> ---
> >>>>    drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> >>>>    drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >>>>    2 files changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >>>> index 9fd221c97275..da6532da44e3 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.c
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >>>> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
> >>>>    static const struct intel_device_info intel_broadwell_d_info = {
> >>>>    	BDW_FEATURES,
> >>>>    	.gen = 8,
> >>>> +	.is_broadwell = 1,
> >>>>    };
> >>>>
> >>>>    static const struct intel_device_info intel_broadwell_m_info = {
> >>>>    	BDW_FEATURES,
> >>>>    	.gen = 8, .is_mobile = 1,
> >>>> +	.is_broadwell = 1,
> >>>>    };
> >>>>
> >>>>    static const struct intel_device_info intel_broadwell_gt3d_info = {
> >>>>    	BDW_FEATURES,
> >>>>    	.gen = 8,
> >>>> +	.is_broadwell = 1,
> >>>>    	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>>    };
> >>>>
> >>>>    static const struct intel_device_info intel_broadwell_gt3m_info = {
> >>>>    	BDW_FEATURES,
> >>>>    	.gen = 8, .is_mobile = 1,
> >>>> +	.is_broadwell = 1,
> >>>>    	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>>    };
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>> index c6351016eaf0..459561991081 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>> @@ -735,6 +735,7 @@ struct intel_csr {
> >>>>    	func(is_valleyview) sep \
> >>>>    	func(is_cherryview) sep \
> >>>>    	func(is_haswell) sep \
> >>>> +	func(is_broadwell) sep \
> >>>>    	func(is_skylake) sep \
> >>>>    	func(is_broxton) sep \
> >>>>    	func(is_kabylake) sep \
> >>>
> >>> We could just replace all of these with a single enum and save a bunch
> >>> of space.
> >>
> >> Bitfield is really nice here because compiler can then merge tests like
> >> IS_HASWELL || IS_BROADWELL into a single test instruction.
> >
> > Most if not all of those are in modeset code so I'm not entirely
> > convinced micro-optimizing that is all that useful.
> 
> Maybe not, but IS_BROADWELL was just tasteless and it is better to save 
> instructions across everything than a bit in dev_priv.
> 
> > Bunch of times I've just wished I could write 'whatever >= G4X' instead
> > of 'IS_G4X || gen >= 5' simply for readability.
> 
> I don't even know what G4X is so can't really follow what you mean. Is 
> it like Gen4.5, or Gen5-- ? So I can't figure out what kind of scheme 
> you propose.

It's still a GMCH platform, but the display engine north side bits are
very close to ILK already. Oh and I suppose a bunch of the south side
bits are also pretty close to ILK as well since G4X has native HDMI
and DP ports already. So the display engine in many ways is closer to
ILK than to gen4. The one major difference to ILK is the lack of FDI.

-- 
Ville Syrjälä
Intel OTC