Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats

[Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>]

On 9/22/21 11:28 AM, Manna, Animesh wrote:
> 
> 
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of José
>> Roberto de Souza
>> Sent: Tuesday, September 21, 2021 6:11 AM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>; Souza, Jose
>> <jose.souza@intel.com>
>> Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when
>> handling biplanar formats
>>
>> From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>>
>> We are still missing the PSR2 selective fetch handling of biplanar formats but
>> until proper handle is added we can workaround it by doing full frames fetch
>> when state has biplanar formats.
>>
>> We need the second check because an update in a regular format could
>> intersect with a biplanar plane that was not initialy part of the atomic commit.
>>
>> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_psr.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>> b/drivers/gpu/drm/i915/display/intel_psr.c
>> index 8ceb22c5a1a6b..e6a4c27975d8c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> @@ -1601,9 +1601,13 @@ int intel_psr2_sel_fetch_update(struct
>> intel_atomic_state *state,
>>    * TODO: Not clear how to handle planes with negative
>> position,
>>    * also planes are not updated if they have a negative X
>>    * position so for now doing a full update in this cases
>> + *
>> + * TODO: We are missing biplanar formats handling, until it is
>> + * implemented it will send full frame updates.
>>    */
>>   if (new_plane_state->uapi.dst.y1 < 0 ||
>> -    new_plane_state->uapi.dst.x1 < 0) {
>> +    new_plane_state->uapi.dst.x1 < 0 ||
>> +    new_plane_state->hw.fb->format->is_yuv) {
>>   full_update = true;
>>   break;
>>   }
>> @@ -1682,6 +1686,11 @@ int intel_psr2_sel_fetch_update(struct
>> intel_atomic_state *state,
>>   if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
>>   continue;
>>
> 
> Code comment can be added here why we need this check again in same function.
> Enabling full frame update is fine for me for planar format but not sure we need the 2nd check.
> 
That's right, we don't need to set this code here because we set 
full_update above when " new_plane_state->hw.fb->format->is_yuv" is true.

I will update this in the next version.

Thanks, Animesh.

> Regards,
> Animesh
> 
>> +if (new_plane_state->hw.fb->format->is_yuv) {
>> +full_update = true;
>> +break;
>> +}
>> +
>>   sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
>>   sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
>>   sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
>> --
>> 2.33.0
>