Re: [PATCH] drm/i915: Call encoder hot_plug hook only for hdmi

[Daniel Vetter <daniel@ffwll.ch>]

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.jindal@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.
-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