[Intel-gfx] [PATCH] drm/i915: Call encoder hot_plug hook only for hdmi

[Daniel Vetter]

On Thu, Sep 10, 2015 at 01:07:18AM +0000, Jindal, Sonika wrote:
> 
> 
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, September 9, 2015 8:48 PM
> To: Jindal, Sonika
> Cc: Daniel Vetter; intel-gfx at 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.jindal at 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?

That would be my suggestion, but I didn't check the details.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch