Re: [PATCH 05/11] drm/i915: get runtime PM while trying to detect CRT

Paulo Zanoni
2014-02-24T14:36:13Z

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

--
Paulo Zanoni