[PATCH] drm: modify pages_to_sg prime helper to create optimized SG table

[Rahul Sharma]

On Thu, Jan 31, 2013 at 2:23 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Thu, Jan 31, 2013 at 9:38 AM, Rahul Sharma <r.sh.open at gmail.com> wrote:
>> On Tue, Jan 29, 2013 at 10:40 PM, Aaron Plattner <aplattner at nvidia.com> wrote:
>>> On 01/28/2013 05:38 AM, Rahul Sharma wrote:
>>>>
>>>> It fixes the issue arises due to passing 'nr_pages' in place of 'nents' to
>>>> sg_alloc_table. When ARM_HAS_SG_CHAIN is disabled, it is causing failure
>>>> in
>>>> creating SG table for the buffers having more than 204 physical pages i.e.
>>>> equal to SG_MAX_SINGLE_ALLOC.
>>>>
>>>> When using sg_alloc_table_from_pages interface, in place of
>>>> sg_alloc_table,
>>>> page list will be passes to get each contiguous section which is
>>>> represented
>>>> by a single entry in the table. For a Contiguous Buffer, number of entries
>>>> should be equal to 1.
>>>>
>>>> Following check is causing the failure which is not applicable for
>>>> Non-Contig
>>>> buffers:
>>>>
>>>>         if (WARN_ON_ONCE(nents > max_ents))
>>>>                 return -EINVAL;
>>>>
>>>> Above patch is well tested for EXYNOS4 and EXYNOS5 for with/wihtout IOMMU
>>>> supprot. NOUVEAU and RADEON platforms also depends on
>>>> drm_prime_pages_to_sg
>>>> helper function.
>>>>
>>>> This set is base on "exynos-drm-fixes" branch at
>>>> http://git.kernel.org/?p=linux/kernel/git/daeinki/drm-exynos.git
>>>>
>>>> Signed-off-by: Rahul Sharma <rahul.sharma at samsung.com>
>>>
>>>
>>> Reviewed-by: Aaron Plattner <aplattner at nvidia.com>
>>>
>>> I also verified that this reduces my 2025-entry sg_table to 6 entries, so
>>>
>>> Tested-by: Aaron Plattner <aplattner at nvidia.com>
>>>
>>> --
>>> Aaron
>>>
>>
>> Thanks Aaron,
>>
>> I want to request stake-holders to review and test this patch for
>> other platforms.
>
> Iirc the old i915 dma_buf import code presumed that each sg entry
> points to exactly one page. Yeah, Dave cut a few corners in the
> initial implementation. Since Chris' rework to use sg_tables
> internally in i915.ko I think we should be safe, but would need to do
> a full audit of the code. No idea what the exact situation in ttm is.
>
> So I think step one is for you to crawl through the existing drm prime
> drivers and check that you don't break any hidden assumptions about
> this.

Thanks Daniel,

I have parsed the related code and it looks fine to me. I couldn't find
any code section, expecting sg-tables with single-page sgl entries. I
just want to ensure again that it doesn't cause any side effects on
various platforms.

>
> For the patch itself I think there's now a generic pages_to_sg helper
> in the dma core which does exactly what you want it to do. I can dig
> it out if you can't find it.
>

Sorry, I din't get this part. Please elaborate a bit.

regards,
Rahul Sharma.

> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch