| Message ID | 1393001548-2883-6-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 21 Feb 2014 13:52:22 -0300 Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Otherwise we'll read registers that return 0xffffffff, trigger some > WARNs, think CRT is actually connected (because certain bits are 1), > and fail the drm-resources-equal testcase! > > Tested on a SNB machine with runtime PM support (which is not upstream > yet, but is already on my public tree at freedesktop.org, and will > hopefully eventually become upstream). > > Testcase: igt/pm_pc8/drm-resources-equal > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_crt.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c > index 9864aa1..4c1230c 100644 > --- a/drivers/gpu/drm/i915/intel_crt.c > +++ b/drivers/gpu/drm/i915/intel_crt.c > @@ -630,10 +630,13 @@ static enum drm_connector_status > intel_crt_detect(struct drm_connector *connector, bool force) > { > struct drm_device *dev = connector->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crt *crt = intel_attached_crt(connector); > enum drm_connector_status status; > struct intel_load_detect_pipe tmp; > > + intel_runtime_pm_get(dev_priv); > + > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n", > connector->base.id, drm_get_connector_name(connector), > force); > @@ -645,23 +648,30 @@ intel_crt_detect(struct drm_connector *connector, bool force) > */ > if (intel_crt_detect_hotplug(connector)) { > DRM_DEBUG_KMS("CRT detected via hotplug\n"); > - return connector_status_connected; > + status = connector_status_connected; > + goto out; > } else > DRM_DEBUG_KMS("CRT not detected via hotplug\n"); > } > > - if (intel_crt_detect_ddc(connector)) > - return connector_status_connected; > + if (intel_crt_detect_ddc(connector)) { > + status = connector_status_connected; > + goto out; > + } > > /* Load detection is broken on HPD capable machines. Whoever wants a > * broken monitor (without edid) to work behind a broken kvm (that fails > * to have the right resistors for HP detection) needs to fix this up. > * For now just bail out. */ > - if (I915_HAS_HOTPLUG(dev)) > - return connector_status_disconnected; > + if (I915_HAS_HOTPLUG(dev)) { > + status = connector_status_disconnected; > + goto out; > + } > > - if (!force) > - return connector->status; > + if (!force) { > + status = connector->status; > + goto out; > + } > > /* for pre-945g platforms use load detect */ > if (intel_get_load_detect_pipe(connector, NULL, &tmp)) { > @@ -673,6 +683,8 @@ intel_crt_detect(struct drm_connector *connector, bool force) > } else > status = connector_status_unknown; > > +out: > + intel_runtime_pm_put(dev_priv); > return status; > } > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
On Fri, 2014-02-21 at 13:52 -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Otherwise we'll read registers that return 0xffffffff, trigger some > WARNs, think CRT is actually connected (because certain bits are 1), > and fail the drm-resources-equal testcase! > > Tested on a SNB machine with runtime PM support (which is not upstream > yet, but is already on my public tree at freedesktop.org, and will > hopefully eventually become upstream). > > Testcase: igt/pm_pc8/drm-resources-equal > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_crt.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c > index 9864aa1..4c1230c 100644 > --- a/drivers/gpu/drm/i915/intel_crt.c > +++ b/drivers/gpu/drm/i915/intel_crt.c > @@ -630,10 +630,13 @@ static enum drm_connector_status > intel_crt_detect(struct drm_connector *connector, bool force) > { > struct drm_device *dev = connector->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crt *crt = intel_attached_crt(connector); > enum drm_connector_status status; > struct intel_load_detect_pipe tmp; > > + intel_runtime_pm_get(dev_priv); > + > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n", > connector->base.id, drm_get_connector_name(connector), > force); > @@ -645,23 +648,30 @@ intel_crt_detect(struct drm_connector *connector, bool force) > */ > if (intel_crt_detect_hotplug(connector)) { > DRM_DEBUG_KMS("CRT detected via hotplug\n"); > - return connector_status_connected; > + status = connector_status_connected; > + goto out; > } else > DRM_DEBUG_KMS("CRT not detected via hotplug\n"); > } > > - if (intel_crt_detect_ddc(connector)) > - return connector_status_connected; > + if (intel_crt_detect_ddc(connector)) { > + status = connector_status_connected; > + goto out; > + } > > /* Load detection is broken on HPD capable machines. Whoever wants a > * broken monitor (without edid) to work behind a broken kvm (that fails > * to have the right resistors for HP detection) needs to fix this up. > * For now just bail out. */ > - if (I915_HAS_HOTPLUG(dev)) > - return connector_status_disconnected; > + if (I915_HAS_HOTPLUG(dev)) { > + status = connector_status_disconnected; > + goto out; > + } > > - if (!force) > - return connector->status; > + if (!force) { > + status = connector->status; > + goto out; > + } > > /* for pre-945g platforms use load detect */ > if (intel_get_load_detect_pipe(connector, NULL, &tmp)) { > @@ -673,6 +683,8 @@ intel_crt_detect(struct drm_connector *connector, bool force) > } else > status = connector_status_unknown; > > +out: > + intel_runtime_pm_put(dev_priv); > return status; > } Similarly to 04/11, this should be part of a power domain get/put for a new CRT power domain I added in the VLV power domain support patches. On top of those to solve this issue we'd also need to add an enable/disable handler for the HSW always-on power well, which would only do a hsw_disable_package_c8()/hsw_enable_package_c8(). --Imre
2014-02-24 8:33 GMT-03:00 Imre Deak <imre.deak@intel.com>: > On Fri, 2014-02-21 at 13:52 -0300, Paulo Zanoni wrote: >> From: Paulo Zanoni <paulo.r.zanoni@intel.com> >> >> Otherwise we'll read registers that return 0xffffffff, trigger some >> WARNs, think CRT is actually connected (because certain bits are 1), >> and fail the drm-resources-equal testcase! >> >> Tested on a SNB machine with runtime PM support (which is not upstream >> yet, but is already on my public tree at freedesktop.org, and will >> hopefully eventually become upstream). >> >> Testcase: igt/pm_pc8/drm-resources-equal >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> --- >> drivers/gpu/drm/i915/intel_crt.c | 26 +++++++++++++++++++------- >> 1 file changed, 19 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c >> index 9864aa1..4c1230c 100644 >> --- a/drivers/gpu/drm/i915/intel_crt.c >> +++ b/drivers/gpu/drm/i915/intel_crt.c >> @@ -630,10 +630,13 @@ static enum drm_connector_status >> intel_crt_detect(struct drm_connector *connector, bool force) >> { >> struct drm_device *dev = connector->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_crt *crt = intel_attached_crt(connector); >> enum drm_connector_status status; >> struct intel_load_detect_pipe tmp; >> >> + intel_runtime_pm_get(dev_priv); >> + >> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n", >> connector->base.id, drm_get_connector_name(connector), >> force); >> @@ -645,23 +648,30 @@ intel_crt_detect(struct drm_connector *connector, bool force) >> */ >> if (intel_crt_detect_hotplug(connector)) { >> DRM_DEBUG_KMS("CRT detected via hotplug\n"); >> - return connector_status_connected; >> + status = connector_status_connected; >> + goto out; >> } else >> DRM_DEBUG_KMS("CRT not detected via hotplug\n"); >> } >> >> - if (intel_crt_detect_ddc(connector)) >> - return connector_status_connected; >> + if (intel_crt_detect_ddc(connector)) { >> + status = connector_status_connected; >> + goto out; >> + } >> >> /* Load detection is broken on HPD capable machines. Whoever wants a >> * broken monitor (without edid) to work behind a broken kvm (that fails >> * to have the right resistors for HP detection) needs to fix this up. >> * For now just bail out. */ >> - if (I915_HAS_HOTPLUG(dev)) >> - return connector_status_disconnected; >> + if (I915_HAS_HOTPLUG(dev)) { >> + status = connector_status_disconnected; >> + goto out; >> + } >> >> - if (!force) >> - return connector->status; >> + if (!force) { >> + status = connector->status; >> + goto out; >> + } >> >> /* for pre-945g platforms use load detect */ >> if (intel_get_load_detect_pipe(connector, NULL, &tmp)) { >> @@ -673,6 +683,8 @@ intel_crt_detect(struct drm_connector *connector, bool force) >> } else >> status = connector_status_unknown; >> >> +out: >> + intel_runtime_pm_put(dev_priv); >> return status; >> } > > Similarly to 04/11, this should be part of a power domain get/put for a > new CRT power domain I added in the VLV power domain support patches. On > top of those to solve this issue we'd also need to add an enable/disable > handler for the HSW always-on power well, which would only do a > hsw_disable_package_c8()/hsw_enable_package_c8(). Yes, I guess your series solves the same problem. But then, again, this is a bug fix, so the same discussion of patch 4/11 applies here. > > --Imre
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 9864aa1..4c1230c 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -630,10 +630,13 @@ static enum drm_connector_status intel_crt_detect(struct drm_connector *connector, bool force) { struct drm_device *dev = connector->dev; + struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crt *crt = intel_attached_crt(connector); enum drm_connector_status status; struct intel_load_detect_pipe tmp; + intel_runtime_pm_get(dev_priv); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n", connector->base.id, drm_get_connector_name(connector), force); @@ -645,23 +648,30 @@ intel_crt_detect(struct drm_connector *connector, bool force) */ if (intel_crt_detect_hotplug(connector)) { DRM_DEBUG_KMS("CRT detected via hotplug\n"); - return connector_status_connected; + status = connector_status_connected; + goto out; } else DRM_DEBUG_KMS("CRT not detected via hotplug\n"); } - if (intel_crt_detect_ddc(connector)) - return connector_status_connected; + if (intel_crt_detect_ddc(connector)) { + status = connector_status_connected; + goto out; + } /* Load detection is broken on HPD capable machines. Whoever wants a * broken monitor (without edid) to work behind a broken kvm (that fails * to have the right resistors for HP detection) needs to fix this up. * For now just bail out. */ - if (I915_HAS_HOTPLUG(dev)) - return connector_status_disconnected; + if (I915_HAS_HOTPLUG(dev)) { + status = connector_status_disconnected; + goto out; + } - if (!force) - return connector->status; + if (!force) { + status = connector->status; + goto out; + } /* for pre-945g platforms use load detect */ if (intel_get_load_detect_pipe(connector, NULL, &tmp)) { @@ -673,6 +683,8 @@ intel_crt_detect(struct drm_connector *connector, bool force) } else status = connector_status_unknown; +out: + intel_runtime_pm_put(dev_priv); return status; }