On Thu, Sep 10, 2015 at 01:07:18AM +0000, Jindal, Sonika wrote: > > > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel Vetter > Sent: Wednesday, September 9, 2015 8:48 PM > To: Jindal, Sonika > Cc: Daniel Vetter; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Sharma, Shashank > Subject: Re: [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@xxxxxxxxx> > > >>>--- > > >>> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
