[Intel-gfx] [PATCH] drm/i915: Inline SUPPORTS_DIGITAL_OUTPUTS

[Daniel Vetter]

On Tue, Jul 07, 2015 at 10:54:01AM +0100, Chris Wilson wrote:
> On Tue, Jul 07, 2015 at 11:44:32AM +0200, Daniel Vetter wrote:
> > After the register save/restore code is gone there's just one user
> > left and it just obfuscates that one. Remove it.
> > 
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Suggested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      | 1 -
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  2 files changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 0d9a60b9e6c5..ee956a165d46 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2511,7 +2511,6 @@ struct drm_i915_cmd_table {
> >   */
> >  #define HAS_128_BYTE_Y_TILING(dev) (!IS_GEN2(dev) && !(IS_I915G(dev) || \
> >  						      IS_I915GM(dev)))
> > -#define SUPPORTS_DIGITAL_OUTPUTS(dev)	(!IS_GEN2(dev) && !IS_PINEVIEW(dev))
> >  #define SUPPORTS_TV(dev)		(INTEL_INFO(dev)->supports_tv)
> >  #define I915_HAS_HOTPLUG(dev)		 (INTEL_INFO(dev)->has_hotplug)
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 87be3f307e40..0663e1d97979 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14316,7 +14316,7 @@ static void intel_setup_outputs(struct drm_device *dev)
> >  		}
> >  
> >  		intel_dsi_init(dev);
> > -	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
> > +	} else if (!IS_GEN2(dev) && !IS_PINEVIEW(dev)) {
> >  		bool found = false;
> >  
> >  		if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
> 
> That is one wierd if-tree. At least half the problem is that we cannot
> see the if-elif-else chain because each branch is over a page high! And
> the other half is that there is no apparent logic between say HAS_DDI or
> HAS_PCH_SPLIT so knowing which should be applying to your device of
> interest is difficult.

It is. I tried pushing some of the decisions into lower-level functions
(like the detection for dig ports) to clean it up a bit, but didn't
improve really. No other idea thus far what to do with it.

> Anyway,
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch