Re: [PATCH v2 1/6] drm/i915/display/psr: Calculate selective fetch plane registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
Tested-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>

On Mon, 2020-11-02 at 11:18 -0800, Souza, Jose wrote:
> On Thu, 2020-10-29 at 21:37 +0000, Mun, Gwan-gyeong wrote:
> > On Tue, 2020-10-27 at 16:45 -0700, José Roberto de Souza wrote:
> > > Add the calculations to set plane selective fetch registers
> > > depending
> > > in the value of the area damaged.
> > > It is still using the whole plane area as damaged but that will
> > > change
> > > in next patches.
> > > 
> > > v2:
> > > - fixed new_plane_state->uapi.dst.y2 typo in
> > > intel_psr2_sel_fetch_update()
> > > - do not shifthing new_plane_state->uapi.dst only src is in 16.16
> > > format
> > > 
> > > BSpec: 55229
> > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > > ---
> > >  .../drm/i915/display/intel_display_types.h    |  2 ++
> > >  drivers/gpu/drm/i915/display/intel_psr.c      | 22
> > > ++++++++++++++---
> > > --
> > >  2 files changed, 18 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index f6f0626649e0..3f2707d882cc 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -604,6 +604,8 @@ struct intel_plane_state {
> > >  	u32 planar_slave;
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >  	struct drm_intel_sprite_colorkey ckey;
> > > +
> > > +	struct drm_rect psr2_sel_fetch_area;
> > >  };
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >  struct intel_initial_plane_config {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index b2544102e7b1..6dead51d7a81 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1187,6 +1187,7 @@ void
> > > intel_psr2_program_plane_sel_fetch(struct
> > > intel_plane *plane,
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > >  	enum pipe pipe = plane->pipe;
> > > +	const struct drm_rect *clip;
> > >  	u32 val;
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >  	if (!crtc_state->enable_psr2_sel_fetch)
> > > @@ -1198,16 +1199,20 @@ void
> > > intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> > >  	if (!val || plane->id == PLANE_CURSOR)
> > >  		return;
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > -	val = plane_state->uapi.dst.y1 << 16 | plane_state-
> > > > uapi.dst.x1;
> > > +	clip = &plane_state->psr2_sel_fetch_area;
> > > +
> > > +	val = (clip->y1 + plane_state->uapi.dst.y1) << 16;
> > > +	val |= plane_state->uapi.dst.x1;
> > >  	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane-
> > > > id), val);
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > -	val = plane_state->color_plane[color_plane].y << 16;
> > > +	/* TODO: consider tiling and auxiliary surfaces */
> > > +	val = (clip->y1 + plane_state->color_plane[color_plane].y) <<
> > > 16;
> > >  	val |= plane_state->color_plane[color_plane].x;
> > >  	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane-
> > > > id),
> > >  			  val);
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >  	/* Sizes are 0 based */
> > > -	val = ((drm_rect_height(&plane_state->uapi.src) >> 16) - 1) <<
> > > 16;
> > > +	val = (drm_rect_height(clip) - 1) << 16;
> > >  	val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1;
> > >  	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane-
> > > > id), val);
> > >  }
> > > @@ -1281,7 +1286,7 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >  	for_each_oldnew_intel_plane_in_state(state, plane,
> > > old_plane_state,
> > >  					     new_plane_state, i) {
> > > -		struct drm_rect temp;
> > > +		struct drm_rect *sel_fetch_area, temp;
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >  		if (new_plane_state->uapi.crtc != crtc_state-
> > > > uapi.crtc)
> > >  			continue;
> > > @@ -1304,8 +1309,13 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > >  		 * For now doing a selective fetch in the whole plane
> > > area,
> > >  		 * optimizations will come in the future.
> > >  		 */
> > > -		temp.y1 = new_plane_state->uapi.dst.y1;
> > > -		temp.y2 = new_plane_state->uapi.dst.y2;
> > > +		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> > > +		sel_fetch_area->y1 = new_plane_state->uapi.src.y1 >>
> > > 16;
> > > +		sel_fetch_area->y2 = new_plane_state->uapi.src.y2 >>
> > > 16;
> > > +
> > > +		temp = *sel_fetch_area;
> > > +		temp.y1 += new_plane_state->uapi.dst.y1;
> > > +		temp.y2 += new_plane_state->uapi.dst.y2;
> > when the userspace call drmModeSetPlane(), src_y and crtc_y can be
> > differ.
> > (drm core uses drm_rect_translate_to() utility function for apply
> > coordinate traslate to dst from src. )
> 
> Checking drm_atomic_helper_update_plane() it sets userspace values to
> plane_state->crtc_ and plane_state->src_, then atomic code will set
> the right
> values back to uapi.dst and uapi.src.
> 
> > In my opinion, we can calculate like as,
> > int coord_trans = new_plane_state->uapi.dst.y1 - new_plane_state-
> > > uapi.src.y1 >> 16;
> > temp = *sel_fetch_area; // assunes that sel_fetch_area has damage
> > area.
> > temp.y1 += coord_trans;
> > temp.y2 += coord_trans;
> 
> uapi.dst.x1/y1 is the coordinates where plane should be placed in
> pipe to be combined with other planes(skl_program_plane()), that
> exactly what we
> need to compute the damaged areas of the whole pipe.
> 
> > >  		clip_area_update(&pipe_clip, &temp);
> > 
> > As we don't check and calculate plane rotation and scale for dst
> > coordinates here.
> > so can you add checking of plane scale or rotation? and if the
> > plane
> > has a scale factor or rotation, for now, we should not apply PSR
> > SU.
> > (if plane scale or rotation are used, dst might differ. if we don't
> > apply PSR SU, we don't need to call clip_area_update() here. )
> 
> Rotation is not supported by selective
> fetch(intel_psr2_sel_fetch_config_valid()) about scale, the scaler HW
> will take care of adjust the original
> size but I doubt that scale works with PSR panels.
> 
> > >  	}
> > >  
> > > 
> > > 
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux