Re: [PATCH 5/6] drm/i915: drm/i915: Process hpd only for hdmi inside hotplug_work_func

["Jindal, Sonika" <sonika.jindal@intel.com>]

On 9/4/2015 8:17 PM, Daniel Vetter wrote:
> On Fri, Sep 04, 2015 at 06:56:15PM +0530, Sonika Jindal wrote:
>> If the same port is enumerated as hdmi as well as DP, this will get
>> called for DP connector as well which is not required because
>> i915_hotplug_work_func is solely to handle hdmi HPD.
>>
>> 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..8e1c43e 100644
>> --- a/drivers/gpu/drm/i915/intel_hotplug.c
>> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
>> @@ -325,7 +325,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
>>
>>   	list_for_each_entry(connector, &mode_config->connector_list, head) {
>>   		intel_connector = to_intel_connector(connector);
>> -		if (!intel_connector->encoder)
>> +		if (!intel_connector->encoder
>> +			&& connector->connector_type != DRM_MODE_CONNECTOR_HDMIA)
>>   			continue;
>
> Pretty sure this breaks hotplug detection for everything but HDMI (since
> now nothing but hdmi gets called it's ->detect function, and only if that
> signals a status change will we send out an uevent to userspace). Does
> this really not break DP hotplug?
>
> Yes we probe both hdmi and DP always, and probably we could be more
> intelligent with the fallback between the too. But this here doesn't seem
> to be the right solution.
> -Daniel

Hmm :(
This doesn't allow detect to be called for dp. I missed the part that 
hpd_pulse only handles mst. From the name and comments it looks like it 
should handle hpd for dp completely. I think this this code needs some 
refactoring.

For now, I think I better move this check to intel_encoder->hot_plug 
function because for the connectors with dp and hdmi both, this hot_plug 
hook gets called twice.

>
>>   		intel_encoder = intel_connector->encoder;
>>   		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx