diff mbox series

[v3,3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats

Message ID 20210921004113.261827-3-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled | expand

Commit Message

Souza, Jose Sept. 21, 2021, 12:41 a.m. UTC 
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(-)

Comments

Manna, Animesh Sept. 22, 2021, 8:28 a.m. UTC | #1 
> -----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.

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
Gwan-gyeong Mun Sept. 22, 2021, 12:08 p.m. UTC | #2 
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
>
Souza, Jose Sept. 22, 2021, 2:17 p.m. UTC | #3 
On Wed, 2021-09-22 at 15:08 +0300, Gwan-gyeong Mun wrote:
> 
> 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.

We need it, the reason is on the commit description.

> 
> 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
> >
diff mbox series

Patch

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;
 
+		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;