Re: [PATCH 1/3] drm/i915: Do CRT HPD force trigger only when needed

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Apr 18, 2016 at 07:03:23PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There doesn't seem to be any need to for the force trigger trick every
> single time on VLV CRT output. Let's do it the same way as on ILK+, and
> do the force trigger only on the first detect after reset.
> 
> This means we can actually just call the ILK+ code on VLV. The only
> difference between the two is the "turn DAC off" trick, but that's
> already conditional on HAS_PCH_SPLIT() so it won't be done on VLV.

Chris requested that I add a bit more details here, so let's amend this with:

The valleyview specific code was added
commit 7d2c24e8cd99 ("drm/i915: add ValleyView specific CRT detect function")
with no real details on why it always uses the force trigger, just a
vague comment about "bugs". It predates proper power well code, as well
as the "DPLL refclk needed for CRT hpd" fix from
commit e5cbfbfb2e0e ("drm/i915: vlv: W/a for hotplug/manual VGA detection")

> 
> Cc: Lyude <cpaul@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c | 39 +--------------------------------------
>  1 file changed, 1 insertion(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index a2a31fd01d1d..d10dea5bd08a 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -316,40 +316,6 @@ static bool intel_ironlake_crt_detect_hotplug(struct drm_connector *connector)
>  	return ret;
>  }
>  
> -static bool valleyview_crt_detect_hotplug(struct drm_connector *connector)
> -{
> -	struct drm_device *dev = connector->dev;
> -	struct intel_crt *crt = intel_attached_crt(connector);
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 adpa;
> -	bool ret;
> -	u32 save_adpa;
> -
> -	save_adpa = adpa = I915_READ(crt->adpa_reg);
> -	DRM_DEBUG_KMS("trigger hotplug detect cycle: adpa=0x%x\n", adpa);
> -
> -	adpa |= ADPA_CRT_HOTPLUG_FORCE_TRIGGER;
> -
> -	I915_WRITE(crt->adpa_reg, adpa);
> -
> -	if (wait_for((I915_READ(crt->adpa_reg) & ADPA_CRT_HOTPLUG_FORCE_TRIGGER) == 0,
> -		     1000)) {
> -		DRM_DEBUG_KMS("timed out waiting for FORCE_TRIGGER");
> -		I915_WRITE(crt->adpa_reg, save_adpa);
> -	}
> -
> -	/* Check the status to see if both blue and green are on now */
> -	adpa = I915_READ(crt->adpa_reg);
> -	if ((adpa & ADPA_CRT_HOTPLUG_MONITOR_MASK) != 0)
> -		ret = true;
> -	else
> -		ret = false;
> -
> -	DRM_DEBUG_KMS("valleyview hotplug adpa=0x%x, result %d\n", adpa, ret);
> -
> -	return ret;
> -}
> -
>  /**
>   * Uses CRT_HOTPLUG_EN and CRT_HOTPLUG_STAT to detect CRT presence.
>   *
> @@ -366,12 +332,9 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector)
>  	bool ret = false;
>  	int i, tries = 0;
>  
> -	if (HAS_PCH_SPLIT(dev))
> +	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
>  		return intel_ironlake_crt_detect_hotplug(connector);
>  
> -	if (IS_VALLEYVIEW(dev))
> -		return valleyview_crt_detect_hotplug(connector);
> -
>  	/*
>  	 * On 4 series desktop, CRT detect sequence need to be done twice
>  	 * to get a reliable result.
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx