Re: [PATCH] drm/i915: Add BXT Sprite plane fractional scalings

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

On Thu, Nov 26, 2015 at 11:49:43PM +0530, Nabendu Maiti wrote:
> 
> 
> On 11/18/2015 06:49 PM, Ville Syrjälä wrote:
> > On Wed, Nov 18, 2015 at 06:37:17PM +0530, Maiti, Nabendu Bikash wrote:
> >>
> >> On 11/18/2015 5:41 PM, Ville Syrjälä wrote:
> >>> On Wed, Nov 18, 2015 at 05:13:01PM +0530, Nabendu Maiti wrote:
> >>>> On older platforms scalers/cliping used to provide destination size an
> >>>> exact multiple of src size.
> >>>> Gen-9 and above support fractional ratio of dst/src so that source is
> >>>> not manipulated to meet the exact multiple factor.
> >>>>
> >>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> >>>> ---
> >>>>    drivers/gpu/drm/i915/intel_sprite.c | 48 +++++++++++++++++++++----------------
> >>>>    1 file changed, 28 insertions(+), 20 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >>>> index 2b96f33..e8c17ae 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>> @@ -813,29 +813,37 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>>>    	crtc_h = drm_rect_height(dst);
> >>>>    
> >>>>    	if (state->visible) {
> >>>> -		/* check again in case clipping clamped the results */
> >>>> -		hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >>>> -		if (hscale < 0) {
> >>>> -			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
> >>>> -			drm_rect_debug_print("src: ", src, true);
> >>>> -			drm_rect_debug_print("dst: ", dst, false);
> >>>> -
> >>>> -			return hscale;
> >>>> -		}
> >>>>    
> >>>> -		vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >>>> -		if (vscale < 0) {
> >>>> -			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
> >>>> -			drm_rect_debug_print("src: ", src, true);
> >>>> -			drm_rect_debug_print("dst: ", dst, false);
> >>>> +		/* Gen 9 and above has fractional scaling support */
> >>>> +		if (INTEL_INFO(plane->dev)->gen < 9) {
> >>>>    
> >>>> -			return vscale;
> >>>> -		}
> >>>> +			/* check again in case clipping clamped the results */
> >>>> +			hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >>>> +			if (hscale < 0) {
> >>>> +				DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
> >>>> +				drm_rect_debug_print("src: ", src, true);
> >>>> +				drm_rect_debug_print("dst: ", dst, false);
> >>>> +
> >>>> +				return hscale;
> >>>> +			}
> >>>> +
> >>>> +			vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >>>> +			if (vscale < 0) {
> >>>> +				DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
> >>>> +				drm_rect_debug_print("src: ", src, true);
> >>>> +				drm_rect_debug_print("dst: ", dst, false);
> >>>>    
> >>>> -		/* 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));
> >>>> +				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));
> >>>> +		}
> >>> NAK. This code just makes sure the actual scaling ratio matches what we
> >>> calculated. As in there may have been some amount of rounding involved
> >>> etc.
> >>>
> >>> The part you actually want to change is what comes after this where we
> >>> throw away the fractional part of the coordinates. And then you need to
> >>> change the actual hw programming so that we actually feed the sub-pixel
> >>> coordinates to the hardware.
> >> In line
> >>
> >> drm_rect_adjust_size(src,
> >> +					     drm_rect_width(dst) * hscale - drm_rect_width(src),
> >> +					     drm_rect_height(dst) * vscale - drm_rect_height(src));
> >>
> >> I think we throw away the fractional part,
> > No, we don't.
> It get changed for example src size 400x596   to destination 1055x700.
> then  i.e. u32/int hscale = (src * 0x10000)/dst = 0x1900000/0x41f = 
> 26214400/1055= 24847 (actually 24847.772511848)
> so in next line it get modified .
> 
> drm_rect_adjust_size(src,
> +					     drm_rect_width(dst) * hscale - drm_rect_width(src),
> +					     drm_rect_height(dst) * vscale - drm_rect_height(src)); ,
> 
> Src width become 399, which is used for further calculation & commiting.

No, it becomes
hscale = floor(400 * 2^16 / 1055) = 24847
src width = 24847 * 1055 = 26213585
src width / 2^16 ~= 399.9876

The fractional bits are thrown away later when somone does src width >>16.

> 
> following logs also shows same result
> 
> 
> [ 144.749628] [drm:intel_check_sprite_plane] NBM T00: src- x1:y1-> 0:0 
> x2:y2->26214400:39059456
> [ 144.749687] [drm:intel_check_sprite_plane] NBM T01: dst- x1:y1-> 0:0 
> x2:y2->1055:700
> [ 144.749747] [drm:intel_check_sprite_plane] NBM T02: Clip- x1:y1-> 0:0 
> x2:y2->1080:1920
> [ 144.749862] [drm:intel_check_sprite_plane] NBM T02a: src- x1:y1-> 0:0 
> x2:y2->26214400:39059456
> [ 144.749924] [drm:intel_check_sprite_plane] NBM T02b: dst- x1:y1-> 0:0 
> x2:y2->1055:700
> [ 144.749983] [drm:intel_check_sprite_plane] NBM T03: hscale: 24847 
> vscale: 55799
> [ 144.750040] [drm:intel_check_sprite_plane] NBM T04: src- x1:y1-> 0:0 
> x2:y2->26214400:39059456
> [ 144.750143] [drm:intel_check_sprite_plane] NBM T05: dst- x1:y1-> 0:0 
> x2:y2->1055:700
> [ 144.750199] [drm:intel_check_sprite_plane] NBM T05a: Clip- x1:y1-> 0:0 
> x2:y2->1080:1920
> [ 144.750258] [drm:drm_rect_clip_scaled] NBM K00: src- x1:y1-> 0:0 
> h->26214400:39059456
> 
> [ 144.750312] [drm:drm_rect_clip_scaled] NBM K01: dst- x1:y1-> 0:0 
> h->1055:700
> [ 144.750312] [ 144.750423] [drm:intel_check_sprite_plane] NBM T05b: 
> src- x1:y1-> 0:0 x2:y2->26214400:39059456
> [ 144.750480] [drm:intel_check_sprite_plane] NBM T05c: dst- x1:y1-> 0:0 
> x2:y2->1055:700
> [ 144.750535] [drm:intel_check_sprite_plane] NBM T05d: Clip- x1:y1-> 0:0 
> x2:y2->1080:1920
> [ 144.750591] [drm:intel_check_sprite_plane] NBM T06-A: src- x1:y1-> 
> 408:78 x2:y2->26213993:39059378
> [ 144.750649] [drm:intel_check_sprite_plane] NBM T06-B: dst- x1:y1-> 0:0 
> x2:y2->1055:700
> [ 144.750706] [drm:intel_check_sprite_plane] NBM T06_0: src- x1:y1-> 
> 0:399 x2:y2->399:0
> [ 144.750761] [drm:intel_check_sprite_plane] NBM T07_0: dst- x1:y1-> 0:0 
> x2:y2->1055:700
> [ 144.750816] [drm:intel_check_sprite_plane] NBM T07a_0: vscale: d9f7 
> hscale:610f
> [ 144.750871] [drm:intel_check_sprite_plane] NBM T06: src- x1:y1-> 
> 408:78 x2:y2->26213993:39059378
> [ 144.750928] [drm:intel_check_sprite_plane] NBM T07: dst- x1:y1-> 0:0 
> x2:y2->1055:700
> [ 144.751001] [drm:intel_check_sprite_plane] NBM T07a: vscale: d9f7 
> hscale:610f
> [ 144.751097] [drm:intel_check_sprite_plane] NBM T08: src- x1:y1-> 0:0 
> x2:y2->26148864:38993920
> [ 144.751174] [drm:intel_check_sprite_plane] NBM T09: dst- x1:y1-> 0:0 
> x2:y2->1055:700
> [ 144.751230] [drm:intel_plane_atomic_check] NBM L02: src- x1:y1-> 0:0 
> h->26148864:38993920
> 
> [ 144.751291] [drm:intel_plane_atomic_calc_changes] NBM M0: src- x1:y1-> 
> 0:0 x2:y2->18f0000:2530000
> [ 144.751291] [drm:intel_plane_atomic_calc_changes] NBM M11: src- 
> x1:y1-> 0:0 x2:y2->18f0000:2530000
> 
> 
> >
> >> which modifies the src,rectangle. In commit we use the modified state->src size.
> >>
> >>>>    
> >>>>    		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
> >>>>    				    state->base.rotation);
> >>>> -- 
> >>>> 1.9.1
> >>>>
> >>>> _______________________________________________
> >>>> Intel-gfx mailing list
> >>>> Intel-gfx@lists.freedesktop.org
> >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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