[PATCH] drm/omap: fix tiled buffer stride calculations

[Tomi Valkeinen]

On 18/05/17 13:59, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Thursday 18 May 2017 13:28:26 Tomi Valkeinen wrote:
>> omap_gem uses page alignment for buffer stride. The related calculations
>> are a bit off, though, as byte stride of 4096 gets aligned to 8192,
>> instead of 4096. This patch fixes those calculations.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/omap_gem.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
>> b/drivers/gpu/drm/omapdrm/omap_gem.c index 13abf221d153..4c41000ff4c4
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
>> @@ -182,7 +182,7 @@ static void evict_entry(struct drm_gem_object *obj,
>>  	size_t size = PAGE_SIZE * n;
>>  	loff_t off = mmap_offset(obj) +
>>  			(entry->obj_pgoff << PAGE_SHIFT);
>> -	const int m = 1 + ((omap_obj->width << fmt) / PAGE_SIZE);
>> +	const int m = 1 + (((omap_obj->width - 1) << fmt) / PAGE_SIZE);
> 
> How about 
> 
> 	int m = round_up(omap_obj->width << fmt, PAGE_SIZE);
> 
> instead of open-coding it ? I find that a bit easier to understand.

That should be:

round_up(omap_obj->width << fmt, PAGE_SIZE) / PAGE_SIZE;

Yes, I think that's more understandable.

> By the way, shifting left by fmt should be fine for TILFMT_8BIT, TILFMT_16BIT 
> and TILFMT_32BIT that evaluate to 0, 1 and 2 respectively, but how does it 
> work with TILFMT_PAGE ? fmt is computed by gem2fmt() in call cases, which 
> returns TILFMT_PAGE in the default case (no tiled flag set). Can this happen 
> in practice ?

These functions are only called for 2D buffers. I do find shifting by
'enum tiler_fmt' quite ugly, though...

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170518/ff5e2b06/attachment-0001.sig>