[PATCH] drm/i915: Correcting proper src & dst height - width comparision for 90/270 rotation.

[PATCH] drm/i915: Correcting proper src & dst height - width comparision for 90/270 rotation.

[Nabendu Maiti]

On 90/270 rotation case source width and height was not compared
properly with destination height and width check plane.Which added
erroneous check while doing scaling or normal 90/270 rotation.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 688d484..cd5bb28 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane *plane,
 	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
 	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
 	bool can_position = false;
+	struct drm_rect *src = &state->src;
+	struct drm_rect *dest = &state->dst;
+	int ret = -1;
 
 	/* use scaler when colorkey is not required */
 	if (INTEL_INFO(plane->dev)->gen >= 9 &&
@@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane *plane,
 		can_position = true;
 	}
 
-	return drm_plane_helper_check_update(plane, crtc, fb, &state->src,
-					     &state->dst, &state->clip,
+	/*
+	 * FIXME the following code does a bunch of fuzzy adjustments to the
+	 * coordinates and sizes for rotations. We probably need some way to
+	 * decide whether more strict checking should be done instead.
+	 */
+	if (fb)
+		drm_rect_rotate(src, fb->width << 16, fb->height << 16,
+				state->base.rotation);
+
+	ret = drm_plane_helper_check_update(plane, crtc, fb, src,
+					     dest, &state->clip,
 					     min_scale, max_scale,
 					     can_position, true,
 					     &state->visible);
+
+	/* Restore the originl unrotated co-ordinates */
+	if (fb)
+		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
+				    state->base.rotation);
+	return ret;
 }
 
 static void
-- 
1.9.1

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

Re: [PATCH] drm/i915: Correcting proper src & dst height - width comparision for 90/270 rotation.

[Ville Syrjälä]

On Wed, Nov 18, 2015 at 05:19:26PM +0530, Nabendu Maiti wrote:
> On 90/270 rotation case source width and height was not compared
> properly with destination height and width check plane.Which added
> erroneous check while doing scaling or normal 90/270 rotation.
> 
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 688d484..cd5bb28 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane *plane,
>  	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
>  	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
>  	bool can_position = false;
> +	struct drm_rect *src = &state->src;
> +	struct drm_rect *dest = &state->dst;
> +	int ret = -1;
>  
>  	/* use scaler when colorkey is not required */
>  	if (INTEL_INFO(plane->dev)->gen >= 9 &&
> @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane *plane,
>  		can_position = true;
>  	}
>  
> -	return drm_plane_helper_check_update(plane, crtc, fb, &state->src,
> -					     &state->dst, &state->clip,
> +	/*
> +	 * FIXME the following code does a bunch of fuzzy adjustments to the
> +	 * coordinates and sizes for rotations. We probably need some way to
> +	 * decide whether more strict checking should be done instead.
> +	 */
> +	if (fb)
> +		drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> +				state->base.rotation);
> +
> +	ret = drm_plane_helper_check_update(plane, crtc, fb, src,
> +					     dest, &state->clip,
>  					     min_scale, max_scale,
>  					     can_position, true,
>  					     &state->visible);
> +
> +	/* Restore the originl unrotated co-ordinates */
> +	if (fb)
> +		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
> +				    state->base.rotation);


We should put the rotation handling into helper. And someone should
really just move all the good code from intel_sprite into the helper
instead of having two totally different ways of doing things.

> +	return ret;
>  }
>  
>  static void
> -- 
> 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: Correcting proper src & dst height - width comparision for 90/270 rotation.

[Nabendu Maiti]

On 11/18/2015 05:44 PM, Ville Syrjälä wrote:
> On Wed, Nov 18, 2015 at 05:19:26PM +0530, Nabendu Maiti wrote:
>> On 90/270 rotation case source width and height was not compared
>> properly with destination height and width check plane.Which added
>> erroneous check while doing scaling or normal 90/270 rotation.
>>
>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++--
>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 688d484..cd5bb28 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane *plane,
>>   	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
>>   	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
>>   	bool can_position = false;
>> +	struct drm_rect *src = &state->src;
>> +	struct drm_rect *dest = &state->dst;
>> +	int ret = -1;
>>   
>>   	/* use scaler when colorkey is not required */
>>   	if (INTEL_INFO(plane->dev)->gen >= 9 &&
>> @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane *plane,
>>   		can_position = true;
>>   	}
>>   
>> -	return drm_plane_helper_check_update(plane, crtc, fb, &state->src,
>> -					     &state->dst, &state->clip,
>> +	/*
>> +	 * FIXME the following code does a bunch of fuzzy adjustments to the
>> +	 * coordinates and sizes for rotations. We probably need some way to
>> +	 * decide whether more strict checking should be done instead.
>> +	 */
>> +	if (fb)
>> +		drm_rect_rotate(src, fb->width << 16, fb->height << 16,
>> +				state->base.rotation);
>> +
>> +	ret = drm_plane_helper_check_update(plane, crtc, fb, src,
>> +					     dest, &state->clip,
>>   					     min_scale, max_scale,
>>   					     can_position, true,
>>   					     &state->visible);
>> +
>> +	/* Restore the originl unrotated co-ordinates */
>> +	if (fb)
>> +		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
>> +				    state->base.rotation);
>
> We should put the rotation handling into helper. And someone should
> really just move all the good code from intel_sprite into the helper
> instead of having two totally different ways of doing things.
Yes right.I learned that there is an effort going on unification of 
primary and sprite plane. To put this in helper function is a bigger 
modifications .
In the meantime in android we need this fix. So if you suggest I will 
hold the patch until the unification completes anf float it once again 
if required.
>
>> +	return ret;
>>   }
>>   
>>   static void
>> -- 
>> 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: Correcting proper src & dst height - width comparision for 90/270 rotation.

[Nabendu Maiti]

On 11/27/2015 12:10 AM, Nabendu Maiti wrote:
>
>
> On 11/18/2015 05:44 PM, Ville Syrjälä wrote:
>> On Wed, Nov 18, 2015 at 05:19:26PM +0530, Nabendu Maiti wrote:
>>> On 90/270 rotation case source width and height was not compared
>>> properly with destination height and width check plane.Which added
>>> erroneous check while doing scaling or normal 90/270 rotation.
>>>
>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++--
>>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index 688d484..cd5bb28 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane 
>>> *plane,
>>>       int min_scale = DRM_PLANE_HELPER_NO_SCALING;
>>>       int max_scale = DRM_PLANE_HELPER_NO_SCALING;
>>>       bool can_position = false;
>>> +    struct drm_rect *src = &state->src;
>>> +    struct drm_rect *dest = &state->dst;
>>> +    int ret = -1;
>>>         /* use scaler when colorkey is not required */
>>>       if (INTEL_INFO(plane->dev)->gen >= 9 &&
>>> @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane 
>>> *plane,
>>>           can_position = true;
>>>       }
>>>   -    return drm_plane_helper_check_update(plane, crtc, fb, 
>>> &state->src,
>>> -                         &state->dst, &state->clip,
>>> +    /*
>>> +     * FIXME the following code does a bunch of fuzzy adjustments 
>>> to the
>>> +     * coordinates and sizes for rotations. We probably need some 
>>> way to
>>> +     * decide whether more strict checking should be done instead.
>>> +     */
>>> +    if (fb)
>>> +        drm_rect_rotate(src, fb->width << 16, fb->height << 16,
>>> +                state->base.rotation);
>>> +
>>> +    ret = drm_plane_helper_check_update(plane, crtc, fb, src,
>>> +                         dest, &state->clip,
>>>                            min_scale, max_scale,
>>>                            can_position, true,
>>>                            &state->visible);
>>> +
>>> +    /* Restore the originl unrotated co-ordinates */
>>> +    if (fb)
>>> +        drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
>>> +                    state->base.rotation);
>>
>> We should put the rotation handling into helper. And someone should
>> really just move all the good code from intel_sprite into the helper
>> instead of having two totally different ways of doing things.
> Yes right.I learned that there is an effort going on unification of 
> primary and sprite plane. To put this in helper function is a bigger 
> modifications .
> In the meantime in android we need this fix. So if you suggest I will 
> hold the patch until the unification completes anf float it once again 
> if required.
Any update on this patch or unification patch?

>>
>>> +    return ret;
>>>   }
>>>     static void
>>> -- 
>>> 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

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