| Message ID | 20160929105245.18617-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On to, 2016-09-29 at 11:52 +0100, Chris Wilson wrote: > Other than reducing the number of EDID reads required for probing the > modes on a connector, this refactor has the importance of centralising > the logic for deciding when we need to probe for an analogue output via > DVI-I and then consistently using that information between detection and > mode computation. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=97971 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> kfree(detect_edid) during connector destroy. Also shouldn't connector forcing also force rereading the EDID? Otherwise looks ok to me. > --- > drivers/gpu/drm/i915/intel_crt.c | 98 +++++++++++++++------------------------- > 1 file changed, 36 insertions(+), 62 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c > index 88ebbdde185a..b9b88b0b73e7 100644 > --- a/drivers/gpu/drm/i915/intel_crt.c > +++ b/drivers/gpu/drm/i915/intel_crt.c > @@ -459,8 +459,8 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector) > return ret; > } > > -static struct edid *intel_crt_get_edid(struct drm_connector *connector, > - struct i2c_adapter *i2c) > +static struct edid *__intel_crt_get_edid(struct drm_connector *connector, > + struct i2c_adapter *i2c) > { > struct edid *edid; > > @@ -473,59 +473,50 @@ static struct edid *intel_crt_get_edid(struct drm_connector *connector, > intel_gmbus_force_bit(i2c, false); > } > > - return edid; > -} > - > -/* local version of intel_ddc_get_modes() to use intel_crt_get_edid() */ > -static int intel_crt_ddc_get_modes(struct drm_connector *connector, > - struct i2c_adapter *adapter) > -{ > - struct edid *edid; > - int ret; > - > - edid = intel_crt_get_edid(connector, adapter); > - if (!edid) > - return 0; > - > - ret = intel_connector_update_modes(connector, edid); > - kfree(edid); > + if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { > + DRM_DEBUG_KMS("EDID reports a digital output, ignoring for this VGA connector\n"); > + kfree(edid); > + edid = NULL; > + } > > - return ret; > + return edid; > } > > -static bool intel_crt_detect_ddc(struct drm_connector *connector) > +static struct edid *intel_crt_get_edid(struct drm_connector *connector) > { > - struct intel_crt *crt = intel_attached_crt(connector); > - struct drm_i915_private *dev_priv = to_i915(crt->base.base.dev); > - struct edid *edid; > +#define HAS_DVI_I(p) 1 /* XXX anything with HDMI, i.e. g33+ */ > + struct drm_i915_private *dev_priv = to_i915(connector->dev); > struct i2c_adapter *i2c; > - > - BUG_ON(crt->base.type != INTEL_OUTPUT_ANALOG); > + struct edid *edid; > > i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin); > - edid = intel_crt_get_edid(connector, i2c); > - > - if (edid) { > - bool is_digital = edid->input & DRM_EDID_INPUT_DIGITAL; > - > + edid = __intel_crt_get_edid(connector, i2c); > + if (!edid && HAS_DVI_I(dev_priv)) { > /* > * This may be a DVI-I connector with a shared DDC > * link between analog and digital outputs, so we > * have to check the EDID input spec of the attached device. > */ > - if (!is_digital) { > - DRM_DEBUG_KMS("CRT detected via DDC:0x50 [EDID]\n"); > - return true; > - } > - > - DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [EDID reports a digital panel]\n"); > - } else { > - DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [no valid EDID found]\n"); > + i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PIN_DPB); > + edid = __intel_crt_get_edid(connector, i2c); > } > > - kfree(edid); > + return edid; > +} > + > +static bool intel_crt_detect_ddc(struct drm_connector *connector) > +{ > + struct edid *edid; > + > + kfree(to_intel_connector(connector)->detect_edid); > + to_intel_connector(connector)->detect_edid = NULL; > > - return false; > + edid = intel_crt_get_edid(connector); > + if (!edid) > + return false; > + > + to_intel_connector(connector)->detect_edid = edid; > + return true; > } > > static enum drm_connector_status > @@ -727,30 +718,13 @@ static void intel_crt_destroy(struct drm_connector *connector) > > static int intel_crt_get_modes(struct drm_connector *connector) > { > - struct drm_device *dev = connector->dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_crt *crt = intel_attached_crt(connector); > - struct intel_encoder *intel_encoder = &crt->base; > - enum intel_display_power_domain power_domain; > - int ret; > - struct i2c_adapter *i2c; > - > - power_domain = intel_display_port_power_domain(intel_encoder); > - intel_display_power_get(dev_priv, power_domain); > - > - i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin); > - ret = intel_crt_ddc_get_modes(connector, i2c); > - if (ret || !IS_G4X(dev)) > - goto out; > - > - /* Try to probe digital port for output in DVI-I -> VGA mode. */ > - i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PIN_DPB); > - ret = intel_crt_ddc_get_modes(connector, i2c); > + struct edid *edid; > > -out: > - intel_display_power_put(dev_priv, power_domain); > + edid = to_intel_connector(connector)->detect_edid; > + if (!edid) > + return 0; > > - return ret; > + return intel_connector_update_modes(connector, edid); > } > > static int intel_crt_set_property(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 88ebbdde185a..b9b88b0b73e7 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -459,8 +459,8 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector) return ret; } -static struct edid *intel_crt_get_edid(struct drm_connector *connector, - struct i2c_adapter *i2c) +static struct edid *__intel_crt_get_edid(struct drm_connector *connector, + struct i2c_adapter *i2c) { struct edid *edid; @@ -473,59 +473,50 @@ static struct edid *intel_crt_get_edid(struct drm_connector *connector, intel_gmbus_force_bit(i2c, false); } - return edid; -} - -/* local version of intel_ddc_get_modes() to use intel_crt_get_edid() */ -static int intel_crt_ddc_get_modes(struct drm_connector *connector, - struct i2c_adapter *adapter) -{ - struct edid *edid; - int ret; - - edid = intel_crt_get_edid(connector, adapter); - if (!edid) - return 0; - - ret = intel_connector_update_modes(connector, edid); - kfree(edid); + if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { + DRM_DEBUG_KMS("EDID reports a digital output, ignoring for this VGA connector\n"); + kfree(edid); + edid = NULL; + } - return ret; + return edid; } -static bool intel_crt_detect_ddc(struct drm_connector *connector) +static struct edid *intel_crt_get_edid(struct drm_connector *connector) { - struct intel_crt *crt = intel_attached_crt(connector); - struct drm_i915_private *dev_priv = to_i915(crt->base.base.dev); - struct edid *edid; +#define HAS_DVI_I(p) 1 /* XXX anything with HDMI, i.e. g33+ */ + struct drm_i915_private *dev_priv = to_i915(connector->dev); struct i2c_adapter *i2c; - - BUG_ON(crt->base.type != INTEL_OUTPUT_ANALOG); + struct edid *edid; i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin); - edid = intel_crt_get_edid(connector, i2c); - - if (edid) { - bool is_digital = edid->input & DRM_EDID_INPUT_DIGITAL; - + edid = __intel_crt_get_edid(connector, i2c); + if (!edid && HAS_DVI_I(dev_priv)) { /* * This may be a DVI-I connector with a shared DDC * link between analog and digital outputs, so we * have to check the EDID input spec of the attached device. */ - if (!is_digital) { - DRM_DEBUG_KMS("CRT detected via DDC:0x50 [EDID]\n"); - return true; - } - - DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [EDID reports a digital panel]\n"); - } else { - DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [no valid EDID found]\n"); + i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PIN_DPB); + edid = __intel_crt_get_edid(connector, i2c); } - kfree(edid); + return edid; +} + +static bool intel_crt_detect_ddc(struct drm_connector *connector) +{ + struct edid *edid; + + kfree(to_intel_connector(connector)->detect_edid); + to_intel_connector(connector)->detect_edid = NULL; - return false; + edid = intel_crt_get_edid(connector); + if (!edid) + return false; + + to_intel_connector(connector)->detect_edid = edid; + return true; } static enum drm_connector_status @@ -727,30 +718,13 @@ static void intel_crt_destroy(struct drm_connector *connector) static int intel_crt_get_modes(struct drm_connector *connector) { - struct drm_device *dev = connector->dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_crt *crt = intel_attached_crt(connector); - struct intel_encoder *intel_encoder = &crt->base; - enum intel_display_power_domain power_domain; - int ret; - struct i2c_adapter *i2c; - - power_domain = intel_display_port_power_domain(intel_encoder); - intel_display_power_get(dev_priv, power_domain); - - i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin); - ret = intel_crt_ddc_get_modes(connector, i2c); - if (ret || !IS_G4X(dev)) - goto out; - - /* Try to probe digital port for output in DVI-I -> VGA mode. */ - i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PIN_DPB); - ret = intel_crt_ddc_get_modes(connector, i2c); + struct edid *edid; -out: - intel_display_power_put(dev_priv, power_domain); + edid = to_intel_connector(connector)->detect_edid; + if (!edid) + return 0; - return ret; + return intel_connector_update_modes(connector, edid); } static int intel_crt_set_property(struct drm_connector *connector,
Other than reducing the number of EDID reads required for probing the modes on a connector, this refactor has the importance of centralising the logic for deciding when we need to probe for an analogue output via DVI-I and then consistently using that information between detection and mode computation. References: https://bugs.freedesktop.org/show_bug.cgi?id=97971 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Imre Deak <imre.deak@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_crt.c | 98 +++++++++++++++------------------------- 1 file changed, 36 insertions(+), 62 deletions(-)