| Message ID | 1436262272-8663-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@linux.intel.com> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@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. Anyway, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
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@linux.intel.com> > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@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@chris-wilson.co.uk> Queued for -next, thanks for the review. -Daniel
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) {
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@linux.intel.com> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@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(-)