All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Rahul Sharma <r.sh.open@gmail.com>
Cc: Rahul Sharma <rahul.sharma@samsung.com>,
	"joshi@samsung.com" <joshi@samsung.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm: modify pages_to_sg prime helper to create optimized SG table
Date: Thu, 31 Jan 2013 13:17:35 +0100	[thread overview]
Message-ID: <CAKMK7uHrXxeJs0pm=msn-7YN6jLiJ79hg0nXQAimEmh3RSrjgA@mail.gmail.com> (raw)
In-Reply-To: <CAPdUM4MGQ6YnXqgasEBiT2VynR56wtK1_kkMcG9X2wo59bXEEw@mail.gmail.com>

On Thu, Jan 31, 2013 at 12:54 PM, Rahul Sharma <r.sh.open@gmail.com> wrote:
> 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.

Just chatted with Chris Wilson on our team, and at last i915.ko _has_
code paths which rely on this. See the reloc handling offset
computations in i915_gem_execbuf.c. I haven't checked
ttm/radeon/nouveau/vmwgfx simply because I don't have the time right
now, and I know that doing this carefully will blow through a few
days. So this patch will break prime buffer sharing on the desktop.

The other thing is that only doing sg entry coalescing for
prime/dma_buf will lead to a QA problem, since all the normal sg
tables still have a 1:1 relationship. So imo the right way to move
forward with this is to convert the various sg table constructors in
i915/ttm/radeon/nouveau/... over to coalesce pages. This allows us to
fix any fallout step-by-step.

Then, once each driver is ready for this, we can merge your patch.

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

See sg_alloc_table_from_pages in lib/scatterlist.c introduced with:

commit efc42bc98058a36d761b16a114823db1a902ed05
Author: Tomasz Stanislawski <t.stanislaws@samsung.com>
Date:   Mon Jun 18 09:25:01 2012 +0200

    scatterlist: add sg_alloc_table_from_pages function

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

  reply	other threads:[~2013-01-31 12:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-28 13:38 [PATCH] drm: modify pages_to_sg prime helper to create optimized SG table Rahul Sharma
2013-01-29 17:10 ` Aaron Plattner
2013-01-31  8:38   ` Rahul Sharma
2013-01-31  8:53     ` Daniel Vetter
2013-01-31 11:54       ` Rahul Sharma
2013-01-31 12:17         ` Daniel Vetter [this message]
2013-01-31 13:26           ` Rahul Sharma
2013-01-31 14:24             ` Daniel Vetter
2013-03-19  9:09   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKMK7uHrXxeJs0pm=msn-7YN6jLiJ79hg0nXQAimEmh3RSrjgA@mail.gmail.com' \
    --to=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=joshi@samsung.com \
    --cc=r.sh.open@gmail.com \
    --cc=rahul.sharma@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.