-----Original Message-----
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, September 9, 2015 8:48 PM
To: Jindal, Sonika
Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Sharma, Shashank
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Call encoder hot_plug hook only for 
hdmi

On Tue, Sep 08, 2015 at 05:08:02PM +0530, Jindal, Sonika wrote:
> 
> 
> On 9/8/2015 10:12 AM, Jindal, Sonika wrote:
> >
> >
> >On 9/7/2015 9:56 PM, Daniel Vetter wrote:
> >>On Mon, Sep 07, 2015 at 10:34:34AM +0530, Sonika Jindal wrote:
> >>>If the same port is enumerated as hdmi as well as DP, this will 
> >>>call hot_plug hook for DP as well which is not required here.
> >>>This splitting of edid read and detect is done only for HDMI with 
> >>>this series.
> >>>
> >>>v2: Avoid breaking DP hpd. Also corrected the commit message and 
> >>>description accordingly. (Daniel)
> >>>
> >>>Signed-off-by: Sonika Jindal <sonika.jin...@intel.com>
> >>>---
> >>>  drivers/gpu/drm/i915/intel_hotplug.c |    3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> >>>b/drivers/gpu/drm/i915/intel_hotplug.c
> >>>index 53c0173..ff4692a 100644
> >>>--- a/drivers/gpu/drm/i915/intel_hotplug.c
> >>>+++ b/drivers/gpu/drm/i915/intel_hotplug.c
> >>>@@ -331,7 +331,8 @@ static void i915_hotplug_work_func(struct 
> >>>work_struct *work)
> >>>          if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> >>>              DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug 
> >>>event.\n",
> >>>                        connector->name, intel_encoder->hpd_pin);
> >>>-            if (intel_encoder->hot_plug)
> >>>+            if (intel_encoder->hot_plug
> >>>+                && connector->connector_type ==
> >>>DRM_MODE_CONNECTOR_HDMIA)
> >>
> >>Please use something like grep to find all the other ->hot_plug 
> >>implementations and then please tell me why you don't break them all.
> >>-Daniel
> >>
> >Hmm, I only checked for hot_plug for DP/edp which is not there. 
> >Failed to notice that there is one in intel_sdvo.c.
> >My mistake. I will place it properly somewhere else.
> >
> >Regards,
> >Sonika
> 
> Is there any suggestion about how we can differentiate if it is actual 
> DP or HDMI hotplug at this point? intel_encoder's type gets updated 
> after detect call. So, not sure how to have this kind of check.
> 
> For now, I think we can abandon this patch from this series.

No, hpd is shared between hdmi and dp at the hw level so we can't 
differentiate. Long term my idea would be that we merge together all the hdmi 
_and_ dp hpd handling into one overall control-flow. Then we can make sure to 
not call anything twice and also have sensible high-level flow (like first 
checking for dp vs. hdmi and then taking relevant paths).

For dealing with ->hot_plug a quick fix might be to have a separate loop going 
over encoders (to make sure we only call it once per encoder if there's more 
than one connector for 1 encoder). That behaviour would also be ok for sdvo.

<Sonika> Hmm, so instead of relying on connector, we can check for the hpd_pin 
on encoder and remove the connector loop completely?

-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to