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@xxxxxxxxxxxxxxx] Sent: Wednesday, January 14, 2015 5:14 PM To: Jindal, Sonika Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx Subject: Re: [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@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx > Subject: RE: [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@xxxxxxxxxxxxxxx] > Sent: Wednesday, January 14, 2015 2:58 PM > To: Jindal, Sonika > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [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 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
