RE: [Intel-gfx] [PATCH 1/2] drm: Adding rotation to drm_plane_helper_check_update

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

 



We do exactly like this for sprite plane (ie, rotate the rect, then check scaling and adjust the size accordingly from drm_rect_calc_hscale_relaxed)
That's why I saw the need of this for primary plane as well. 
For sprite plane 90 rotation, intel_check_sprite_plane does the adjustments and the rotated sizes are fine. But since we don't do any of those stuff
for primary the destination size doesn't come right, and I get a little corrupted output after rotation.
With this change, the rotated plane is properly adjusted in the viewport.
So, I don't think it is a bug in test.

-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] 
Sent: Wednesday, January 14, 2015 2:58 PM
To: Jindal, Sonika
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [Intel-gfx] [PATCH 1/2] drm: Adding rotation to drm_plane_helper_check_update

On Wed, Jan 14, 2015 at 10:05:53AM +0530, sonika wrote:
> 
> On Tuesday 13 January 2015 07:02 PM, Ville Syrjälä wrote:
> > On Tue, Jan 13, 2015 at 06:03:39PM +0530, Sonika Jindal wrote:
> >> Taking rotation into account while checking the plane and adjusting 
> >> the sizes accordingly.
> >>
> >> Signed-off-by: Sonika Jindal <sonika.jindal@xxxxxxxxx>
> >> ---
> >>   drivers/gpu/drm/drm_plane_helper.c |   79 ++++++++++++++++++++++++++++++++++--
> >>   include/drm/drm_plane_helper.h     |    3 +-
> >>   2 files changed, 77 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> >> b/drivers/gpu/drm/drm_plane_helper.c
> >> index f24c4cf..4badd69 100644
> >> --- a/drivers/gpu/drm/drm_plane_helper.c
> >> +++ b/drivers/gpu/drm/drm_plane_helper.c
> >> @@ -138,9 +138,13 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> >>   				    int max_scale,
> >>   				    bool can_position,
> >>   				    bool can_update_disabled,
> >> -				    bool *visible)
> >> +				    bool *visible,
> >> +				    unsigned int rotation)
> >>   {
> >>   	int hscale, vscale;
> >> +	int crtc_x, crtc_y;
> >> +	unsigned int crtc_w, crtc_h;
> >> +	uint32_t src_x, src_y, src_w, src_h;
> >>   
> >>   	if (!fb) {
> >>   		*visible = false;
> >> @@ -158,9 +162,13 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> >>   		return -EINVAL;
> >>   	}
> >>   
> >> +	if (fb)
> >> +		drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> >> +				rotation);
> >> +
> >>   	/* Check scaling */
> >> -	hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale);
> >> -	vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale);
> >> +	hscale = drm_rect_calc_hscale_relaxed(src, dest, min_scale, max_scale);
> >> +	vscale = drm_rect_calc_vscale_relaxed(src, dest, min_scale, 
> >> +max_scale);
> > This is an unrelated change. Relaxed scaling allows the the src/dest 
> > rectangles to be reduced in size in order to keep the scaling ration 
> > within the min/max range. I suppose we should switch to using it to 
> > make the behaviour uniform across drivers, but definitely should be 
> > done with a separate patch.
> Since, I added drm_rect_rotate before this, it changes the src sizes 
> and it was giving me Invalid scaling if we don't let the sizes to be 
> changed using _relaxed functions. I am trying this for 90/270 
> rotation.

That would indicate a bug somewhere. Pontetially the bug could be in whatever test you're using as well.

> I can move it to a separate patch if required.

We nee to figure out why you got the error first.

--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux