Re: [Intel-gfx] [PATCH] drm/i915/dp: Don't invalidate intel_dp_detect result

[Manasi Navare]

On Wed, Aug 23, 2017 at 03:40:28PM -0700, Puthikorn Voravootivat wrote:
> If the full detect is already done, we shouldn't need to do it
> again.
> 
> This fixes the screen blinking issue that happen when calling
> DRM_IOCTL_MODE_GETCONNECTOR while PSR is active. The blinking
> is caused by full dp detect in intel_dp_long_pulse().
>

Thanks for the patch. Yes I completely agree with this, the
main purpose of this flag was to avoid calling long pulse
handler multiple times when it was being called from multiple places.
Now we do not need this flag since it does a full detect every time
get connector IOCTL is called since it calls intel_dp_detect().

This patch needs to remove the detect_done from all the other places
where its being set and from intel_dp struct as well.
I had already submitted a patch for this a while ago:
https://patchwork.freedesktop.org/patch/137592/
Please take a look. However if you read the M-L archive, you will
find the comments and it never got accepted. Some of the arguments
were that since EDID might have changed, we need to always do
a full detect each time GET_CONNECTOR IOCTL is called.

Regards
Manasi
 
> Signed-off-by: Puthikorn Voravootivat 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d3e5fdf0d2fa..152e7016d5f2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4834,8 +4834,6 @@ intel_dp_detect(struct drm_connector *connector,
>   if (!intel_dp->detect_done)
>   status = intel_dp_long_pulse(intel_dp->attached_connector);
>  
> - intel_dp->detect_done = false;
> -
>   return status;
>  }
>  
> -- 
> 2.14.1.342.g6490525c54-goog
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Don't invalidate intel_dp_detect result

[Rodrigo Vivi]

On Wed, Aug 23, 2017 at 4:32 PM, Puthikorn Voravootivat
 wrote:
>> what about instead of removing this "detect_done=false" like we move
>> to another place like whenever conector is disconected?
>
> Look like that is already done in
> https://patchwork.freedesktop.org/patch/113363/

hmm indeed that seems to address my first thoughts...
but this also shows we need to cc ville as well here...

>
> On Wed, Aug 23, 2017 at 4:00 PM, Rodrigo Vivi  wrote:
>> On Wed, Aug 23, 2017 at 3:40 PM, Puthikorn Voravootivat
>>  wrote:
>>> If the full detect is already done, we shouldn't need to do it
>>> again.
>>>
>>> This fixes the screen blinking issue that happen when calling
>>> DRM_IOCTL_MODE_GETCONNECTOR while PSR is active. The blinking
>>> is caused by full dp detect in intel_dp_long_pulse().
>>>
>>> Signed-off-by: Puthikorn Voravootivat 
>>> ---
>>>  drivers/gpu/drm/i915/intel_dp.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>>> b/drivers/gpu/drm/i915/intel_dp.c
>>> index d3e5fdf0d2fa..152e7016d5f2 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -4834,8 +4834,6 @@ intel_dp_detect(struct drm_connector *connector,
>>> if (!intel_dp->detect_done)
>>> status = intel_dp_long_pulse(intel_dp->attached_connector);
>>>
>>> -   intel_dp->detect_done = false;
>>> -
>>
>> something tells me that this will break other existent cases...
>> one case I imagine is changing the DP panel... maybe the MST case as well..
>>
>> what about instead of removing this "detect_done=false" like we move
>> to another place like whenever conector is disconected?
>>
>> looking to original commit:
>> commit 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c
>> Author: Shubhangi Shrivastava 
>>
>> let's cc few people here to see if they have something to say...
>>
>>
>>
>>> return status;
>>>  }
>>>
>>> --
>>> 2.14.1.342.g6490525c54-goog
>>>
>>> ___
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Rodrigo Vivi
>> Blog: http://blog.vivi.eng.br



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Don't invalidate intel_dp_detect result

[Puthikorn Voravootivat]

> what about instead of removing this "detect_done=false" like we move
> to another place like whenever conector is disconected?

Look like that is already done in
https://patchwork.freedesktop.org/patch/113363/

On Wed, Aug 23, 2017 at 4:00 PM, Rodrigo Vivi  wrote:
> On Wed, Aug 23, 2017 at 3:40 PM, Puthikorn Voravootivat
>  wrote:
>> If the full detect is already done, we shouldn't need to do it
>> again.
>>
>> This fixes the screen blinking issue that happen when calling
>> DRM_IOCTL_MODE_GETCONNECTOR while PSR is active. The blinking
>> is caused by full dp detect in intel_dp_long_pulse().
>>
>> Signed-off-by: Puthikorn Voravootivat 
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index d3e5fdf0d2fa..152e7016d5f2 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4834,8 +4834,6 @@ intel_dp_detect(struct drm_connector *connector,
>> if (!intel_dp->detect_done)
>> status = intel_dp_long_pulse(intel_dp->attached_connector);
>>
>> -   intel_dp->detect_done = false;
>> -
>
> something tells me that this will break other existent cases...
> one case I imagine is changing the DP panel... maybe the MST case as well..
>
> what about instead of removing this "detect_done=false" like we move
> to another place like whenever conector is disconected?
>
> looking to original commit:
> commit 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c
> Author: Shubhangi Shrivastava 
>
> let's cc few people here to see if they have something to say...
>
>
>
>> return status;
>>  }
>>
>> --
>> 2.14.1.342.g6490525c54-goog
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Don't invalidate intel_dp_detect result

[Rodrigo Vivi]

On Wed, Aug 23, 2017 at 3:40 PM, Puthikorn Voravootivat
 wrote:
> If the full detect is already done, we shouldn't need to do it
> again.
>
> This fixes the screen blinking issue that happen when calling
> DRM_IOCTL_MODE_GETCONNECTOR while PSR is active. The blinking
> is caused by full dp detect in intel_dp_long_pulse().
>
> Signed-off-by: Puthikorn Voravootivat 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d3e5fdf0d2fa..152e7016d5f2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4834,8 +4834,6 @@ intel_dp_detect(struct drm_connector *connector,
> if (!intel_dp->detect_done)
> status = intel_dp_long_pulse(intel_dp->attached_connector);
>
> -   intel_dp->detect_done = false;
> -

something tells me that this will break other existent cases...
one case I imagine is changing the DP panel... maybe the MST case as well..

what about instead of removing this "detect_done=false" like we move
to another place like whenever conector is disconected?

looking to original commit:
commit 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c
Author: Shubhangi Shrivastava 

let's cc few people here to see if they have something to say...



> return status;
>  }
>
> --
> 2.14.1.342.g6490525c54-goog
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/dp: Don't invalidate intel_dp_detect result

[Puthikorn Voravootivat]

If the full detect is already done, we shouldn't need to do it
again.

This fixes the screen blinking issue that happen when calling
DRM_IOCTL_MODE_GETCONNECTOR while PSR is active. The blinking
is caused by full dp detect in intel_dp_long_pulse().

Signed-off-by: Puthikorn Voravootivat 
---
 drivers/gpu/drm/i915/intel_dp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d3e5fdf0d2fa..152e7016d5f2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4834,8 +4834,6 @@ intel_dp_detect(struct drm_connector *connector,
if (!intel_dp->detect_done)
status = intel_dp_long_pulse(intel_dp->attached_connector);
 
-   intel_dp->detect_done = false;
-
return status;
 }
 
-- 
2.14.1.342.g6490525c54-goog

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx