Re: [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 24-04-18 om 17:21 schreef Ville Syrjälä:
> On Tue, Apr 24, 2018 at 01:36:32PM +0200, Maarten Lankhorst wrote:
>> When calculating limits we want to be as pessimistic as possible,
>> so we have to explicitly say whether we want to round up or down
>> to accurately calculate whether we are below min_scale or above
>> max_scale.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic_helper.c |  4 +--
>>  drivers/gpu/drm/drm_rect.c          | 39 +++++++++++++++++++++--------
>>  drivers/gpu/drm/i915/intel_sprite.c |  8 +++---
>>  include/drm/drm_rect.h              |  8 +++---
>>  4 files changed, 39 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 9cb2209f6fc8..7643202bfcf7 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -754,8 +754,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
>>  
>>  	/* Check scaling */
>> -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>> -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
>> +	hscale = drm_rect_calc_hscale(src, dst, min_scale, 1<<16, max_scale);
>> +	vscale = drm_rect_calc_vscale(src, dst, min_scale, 1<<16, max_scale);
>>  	if (hscale < 0 || vscale < 0) {
>>  		DRM_DEBUG_KMS("Invalid scaling of plane\n");
>>  		drm_rect_debug_print("src: ", &plane_state->src, true);
>> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
>> index 9817c1445ba9..2093474c268c 100644
>> --- a/drivers/gpu/drm/drm_rect.c
>> +++ b/drivers/gpu/drm/drm_rect.c
>> @@ -96,7 +96,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>>  }
>>  EXPORT_SYMBOL(drm_rect_clip_scaled);
>>  
>> -static int drm_calc_scale(int src, int dst)
>> +static int drm_calc_scale(int src, int dst, int scale_center)
>>  {
>>  	int scale = 0;
>>  
>> @@ -106,7 +106,10 @@ static int drm_calc_scale(int src, int dst)
>>  	if (dst == 0)
>>  		return 0;
>>  
>> -	scale = src / dst;
>> +	if (DIV_ROUND_UP(dst, scale_center) > src)
> That doesn't look right to me.
>
> How about just 'if (src > dst << 16)' ?
>
> I guess if we want to keep this code independent of the number of
> fractional bits we could pass the 16 in from the caller(s). Not sure
> there's much point in that though.
pass a bool round_up to the calc_scale functions?
>> +		return DIV_ROUND_UP(src, dst);
>> +	else
>> +		scale = src / dst;
>>  
>>  	return scale;
>>  }
>> @@ -116,21 +119,25 @@ static int drm_calc_scale(int src, int dst)
>>   * @src: source window rectangle
>>   * @dst: destination window rectangle
>>   * @min_hscale: minimum allowed horizontal scaling factor
>> + * @mid_hscale: mid point, below this point round down scaling, above round up.
>>   * @max_hscale: maximum allowed horizontal scaling factor
>>   *
>>   * Calculate the horizontal scaling factor as
>>   * (@src width) / (@dst width).
>>   *
>> + * If the scale is below @mid_hscale we round down, if above up. This will
>> + * calculate the hscale with the most pessimistic limit calculation.
>> + *
>>   * RETURNS:
>>   * The horizontal scaling factor, or errno of out of limits.
>>   */
>>  int drm_rect_calc_hscale(const struct drm_rect *src,
>>  			 const struct drm_rect *dst,
>> -			 int min_hscale, int max_hscale)
>> +			 int min_hscale, int mid_hscale, int max_hscale)
>>  {
>>  	int src_w = drm_rect_width(src);
>>  	int dst_w = drm_rect_width(dst);
>> -	int hscale = drm_calc_scale(src_w, dst_w);
>> +	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
>>  
>>  	if (hscale < 0 || dst_w == 0)
>>  		return hscale;
>> @@ -147,21 +154,25 @@ EXPORT_SYMBOL(drm_rect_calc_hscale);
>>   * @src: source window rectangle
>>   * @dst: destination window rectangle
>>   * @min_vscale: minimum allowed vertical scaling factor
>> + * @mid_vscale: mid point, below this point round down scaling, above round up.
>>   * @max_vscale: maximum allowed vertical scaling factor
>>   *
>>   * Calculate the vertical scaling factor as
>>   * (@src height) / (@dst height).
>>   *
>> + * If the scale is below @mid_vscale we round down, if above up. This will
>> + * calculate the vscale with the most pessimistic limit calculation.
>> + *
>>   * RETURNS:
>>   * The vertical scaling factor, or errno of out of limits.
>>   */
>>  int drm_rect_calc_vscale(const struct drm_rect *src,
>>  			 const struct drm_rect *dst,
>> -			 int min_vscale, int max_vscale)
>> +			 int min_vscale, int mid_vscale, int max_vscale)
>>  {
>>  	int src_h = drm_rect_height(src);
>>  	int dst_h = drm_rect_height(dst);
>> -	int vscale = drm_calc_scale(src_h, dst_h);
>> +	int vscale = drm_calc_scale(src_h, dst_h, mid_vscale);
>>  
>>  	if (vscale < 0 || dst_h == 0)
>>  		return vscale;
>> @@ -178,6 +189,7 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
>>   * @src: source window rectangle
>>   * @dst: destination window rectangle
>>   * @min_hscale: minimum allowed horizontal scaling factor
>> + * @mid_hscale: mid point, below this point round down scaling, above round up.
>>   * @max_hscale: maximum allowed horizontal scaling factor
>>   *
>>   * Calculate the horizontal scaling factor as
>> @@ -189,16 +201,19 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
>>   * If the calculated scaling factor is above @max_vscale,
>>   * decrease the height of rectangle @src to compensate.
>>   *
>> + * If the scale is below @mid_hscale we round down, if above up. This will
>> + * calculate the hscale with the most pessimistic limit calculation.
>> + *
>>   * RETURNS:
>>   * The horizontal scaling factor.
>>   */
>>  int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
>>  				 struct drm_rect *dst,
>> -				 int min_hscale, int max_hscale)
>> +				 int min_hscale, int mid_hscale, int max_hscale)
>>  {
>>  	int src_w = drm_rect_width(src);
>>  	int dst_w = drm_rect_width(dst);
>> -	int hscale = drm_calc_scale(src_w, dst_w);
>> +	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
>>  
>>  	if (hscale < 0 || dst_w == 0)
>>  		return hscale;
>> @@ -228,6 +243,7 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
>>   * @src: source window rectangle
>>   * @dst: destination window rectangle
>>   * @min_vscale: minimum allowed vertical scaling factor
>> + * @mid_vscale: mid point, below this point round down scaling, above round up.
>>   * @max_vscale: maximum allowed vertical scaling factor
>>   *
>>   * Calculate the vertical scaling factor as
>> @@ -239,16 +255,19 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
>>   * If the calculated scaling factor is above @max_vscale,
>>   * decrease the height of rectangle @src to compensate.
>>   *
>> + * If the scale is below @mid_vscale we round down, if above up. This will
>> + * calculate the vscale with the most pessimistic limit calculation.
>> + *
>>   * RETURNS:
>>   * The vertical scaling factor.
>>   */
>>  int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
>>  				 struct drm_rect *dst,
>> -				 int min_vscale, int max_vscale)
>> +				 int min_vscale, int mid_vscale, int max_vscale)
>>  {
>>  	int src_h = drm_rect_height(src);
>>  	int dst_h = drm_rect_height(dst);
>> -	int vscale = drm_calc_scale(src_h, dst_h);
>> +	int vscale = drm_calc_scale(src_h, mid_vscale, dst_h);
>>  
>>  	if (vscale < 0 || dst_h == 0)
>>  		return vscale;
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index aa1dfaa692b9..f8f7c66fc3ac 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -998,10 +998,10 @@ intel_check_sprite_plane(struct intel_plane *plane,
>>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
>>  			state->base.rotation);
>>  
>> -	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
>> +	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
>>  	BUG_ON(hscale < 0);
>>  
>> -	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
>> +	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
>>  	BUG_ON(vscale < 0);
>>  
>>  	if (crtc_state->base.enable)
>> @@ -1017,7 +1017,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>>  
>>  	if (state->base.visible) {
>>  		/* check again in case clipping clamped the results */
>> -		hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>> +		hscale = drm_rect_calc_hscale(src, dst, min_scale, 1 << 16, max_scale);
>>  		if (hscale < 0) {
>>  			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
>>  			drm_rect_debug_print("src: ", src, true);
>> @@ -1026,7 +1026,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>>  			return hscale;
>>  		}
>>  
>> -		vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
>> +		vscale = drm_rect_calc_vscale(src, dst, min_scale, 1 << 16, max_scale);
>>  		if (vscale < 0) {
>>  			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
>>  			drm_rect_debug_print("src: ", src, true);
>> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
>> index 44bc122b9ee0..dbb8631a9288 100644
>> --- a/include/drm/drm_rect.h
>> +++ b/include/drm/drm_rect.h
>> @@ -179,16 +179,16 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>>  			  int hscale, int vscale);
>>  int drm_rect_calc_hscale(const struct drm_rect *src,
>>  			 const struct drm_rect *dst,
>> -			 int min_hscale, int max_hscale);
>> +			 int min_hscale, int mid_hscale, int max_hscale);
>>  int drm_rect_calc_vscale(const struct drm_rect *src,
>>  			 const struct drm_rect *dst,
>> -			 int min_vscale, int max_vscale);
>> +			 int min_vscale, int mid_vscale, int max_vscale);
>>  int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
>>  				 struct drm_rect *dst,
>> -				 int min_hscale, int max_hscale);
>> +				 int min_hscale, int mid_hscale, int max_hscale);
>>  int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
>>  				 struct drm_rect *dst,
>> -				 int min_vscale, int max_vscale);
>> +				 int min_vscale, int mid_vscale, int max_vscale);
>>  void drm_rect_debug_print(const char *prefix,
>>  			  const struct drm_rect *r, bool fixed_point);
>>  void drm_rect_rotate(struct drm_rect *r,
>> -- 
>> 2.17.0


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel