Re: [PATCH] drm/i915: Remove src adjustment in intel_check_sprite_plane.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Apr 23, 2018 at 04:39:54PM +0200, Maarten Lankhorst wrote:
> Op 23-04-18 om 16:30 schreef Ville Syrjälä:
> > On Mon, Apr 23, 2018 at 12:49:22PM +0200, Maarten Lankhorst wrote:
> >> The rounding can cause a perfectly normal 16x16 src to full-screen
> >> dst to be rounded down, even without clipping involved. Because of
> >> this we should just remove the adjustment, as no other driver or plane
> >> does it.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_sprite.c | 5 -----
> >>  1 file changed, 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index a1d048af0261..203ca8b362a5 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -996,11 +996,6 @@ intel_check_sprite_plane(struct intel_plane *plane,
> >>  			return vscale;
> >>  		}
> >>  
> >> -		/* Make the source viewport size an exact multiple of the scaling factors. */
> >> -		drm_rect_adjust_size(src,
> >> -				     drm_rect_width(dst) * hscale - drm_rect_width(src),
> >> -				     drm_rect_height(dst) * vscale - drm_rect_height(src));
> >> -
> > This makes the scaling factor checks a slightly incorrect. Ie. we might
> > exceed the max h/vscale a bit without realizing it. It's a shame the
> > hardware doesn't let us actually program the scaling factors/increments
> > and starting phases anymore :( Also we don't actually know how the
> > hardware calculates that stuff (assuming it has such things internally),
> > so I'm not actually sure how we should be checking the max limit so
> > that we actually check what the hardware will use.
> >
> > Without a more detailed study of the hardware behaviour I'm thinking
> > we should perhaps just check the final src vs. dst coordinates like so:
> > if (src > dst*max)
> > 	fail;
> > as that would avoid the precision issues with the .16 scaling factors.
> >
> > Another option could be to round h/vscale up. That should guarantee
> > that we never exceed the max.
> Could we take a pessimistic view for both sides? Round up if scaling > 1, round down when scaling < 1?
> That way we should never be afraid of any limits..

That does sound like a decent idea to me. At least I can't immediately
think why it wouldn't work out.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx