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

[Jindal, Sonika]

I think I am confused here..
Since sprite plane handles this scaling and rotation in kernel, I thought it should be taken care in driver for primary as well.
>From the test, I don't really change the src sizes for primary as well as sprite to take care of 90/270 rotation.

-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com] 
Sent: Wednesday, January 14, 2015 5:14 PM
To: Jindal, Sonika
Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/2] drm: Adding rotation to drm_plane_helper_check_update

On Wed, Jan 14, 2015 at 09:49:36AM +0000, Jindal, Sonika wrote:
> Since we do drm_rect_rotate with 90 rotation, the src->w changes to src->h.
> Now, when we call drm_rect_calc_hscale, the hscale calculated is lesser than the min_hscale (which is no_scaling by default), so it returns -ERANGE.

If you want no scaling then with 90/270 rotation then your src->w should be equal to dst->h. Then the calculated vscale will be 1.0. If it's not, then your test is passing in bad coordinates.

> So, I think we _relaxed is the function which should be called to update the destination size appropriately.
> Please correct me if I am wrong.
> 
> -----Original Message-----
> From: Jindal, Sonika
> Sent: Wednesday, January 14, 2015 3:06 PM
> To: 'Ville Syrjälä'
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH 1/2] drm: Adding rotation to 
> drm_plane_helper_check_update
> 
> 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 at linux.intel.com]
> Sent: Wednesday, January 14, 2015 2:58 PM
> To: Jindal, Sonika
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
> 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 at intel.com>
> > >> ---
> > >>   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

--
Ville Syrjälä
Intel OTC