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

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

[Nabendu Maiti]

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));
+		}
 
 		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

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

[Ville Syrjälä]

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.

>  
>  		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

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

[Maiti, Nabendu Bikash]

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, 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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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.

> 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

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

[Nabendu Maiti]

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.

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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

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

[Ville Syrjälä]

On Thu, Nov 26, 2015 at 08:33:42PM +0200, Ville Syrjälä wrote:
> 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.

Just to clarify a bit. I origianally wrote this stuff for nice hardware
which can handle sub-pixel coordinates and where you actually program
the scaling factor into the hardware. Doing the rect adjustment avoids
throwing away data from just the right/bottom edges of the image and
instead throws away a bit from each edge.

If the hardware doesn't operate like this (eg. doesn't do sub-pixel
coordinates at all) then I guess it might make sense to not do this. But
then we run into the question that what happens if the scaling ratio is
actually slightly over the max (since we truncate the result from
the src/dst computation it can happen). Without knowing the internals of
the hardware it's hard to say what it will do.

I guess what we could do is check the scaling ratio without the
truncation, and then we should be somewhat certain that things will
work. So eg,. something like this:

int drm_rect_check_hscale(const struct drm_rect *src,
			  const struct drm_rect *dst,
			  int min_hscale, int max_hscale)
{
	int src_w = drm_rect_width(src);
	int64_t dst_w = drm_rect_width(dst);

	if (src_w < min_hscale * dst_w ||
	    src_w > max_hscale * dst_w)
		return -ERANGE;

	return 0;
}

And then I guess we'd need to change the relaxed scaling calculations
too so that they do the scaling ratio comparison this same way.

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

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

[Nabendu Maiti]

On 11/27/2015 01:00 AM, Ville Syrjälä wrote:
> On Thu, Nov 26, 2015 at 08:33:42PM +0200, Ville Syrjälä wrote:
>> 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.
> Just to clarify a bit. I origianally wrote this stuff for nice hardware
> which can handle sub-pixel coordinates and where you actually program
> the scaling factor into the hardware. Doing the rect adjustment avoids
> throwing away data from just the right/bottom edges of the image and
> instead throws away a bit from each edge.
>
> If the hardware doesn't operate like this (eg. doesn't do sub-pixel
> coordinates at all) then I guess it might make sense to not do this. But
> then we run into the question that what happens if the scaling ratio is
> actually slightly over the max (since we truncate the result from
> the src/dst computation it can happen). Without knowing the internals of
> the hardware it's hard to say what it will do.
>
> I guess what we could do is check the scaling ratio without the
> truncation, and then we should be somewhat certain that things will
> work. So eg,. something like this:
>
> int drm_rect_check_hscale(const struct drm_rect *src,
> 			  const struct drm_rect *dst,
> 			  int min_hscale, int max_hscale)
> {
> 	int src_w = drm_rect_width(src);
> 	int64_t dst_w = drm_rect_width(dst);
>
> 	if (src_w < min_hscale * dst_w ||
> 	    src_w > max_hscale * dst_w)
> 		return -ERANGE;
>
> 	return 0;
> }
>
> And then I guess we'd need to change the relaxed scaling calculations
> too so that they do the scaling ratio comparison this same way.
>
Hi Ville,
Yes computation is done here , here after calculation 399.9876, here 400 
to 399.98 transform happen.
According to bspec our BXT scaler can handle fractional scaling. So we 
can feed 400 to scaler input.
Whatever you suggested then in primary case also need to handle that 
way. But in primary we receive 400 and feed it to scaler.Then please let 
me know what is your suggestion.
Regards,
Nabendu


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx