diff mbox

drm: micro optimise cache flushing

Message ID 1348017161-27475-1-git-send-email-airlied@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Airlie Sept. 19, 2012, 1:12 a.m. UTC
From: Dave Airlie <airlied@redhat.com>

We hit this a lot with i915 and although we'd like to engineer things to hit
it a lot less, this commit at least makes it consume a few less cycles.

from something containing
movzwl 0x0(%rip),%r10d
to
add    %r8,%rdx

I only noticed it while using perf to profile something else.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_cache.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Chris Wilson Sept. 19, 2012, 7:50 a.m. UTC | #1
On Wed, 19 Sep 2012 11:12:41 +1000, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> We hit this a lot with i915 and although we'd like to engineer things to hit
> it a lot less, this commit at least makes it consume a few less cycles.
> 
> from something containing
> movzwl 0x0(%rip),%r10d
> to
> add    %r8,%rdx
> 
> I only noticed it while using perf to profile something else.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I would have thought this is something the compiler should, with the
appropriate hints, be smart enough to decide for itself as boot_cpu_data
should be constant once defined. *shrug*
-Chris
Jani Nikula Sept. 20, 2012, 10:02 a.m. UTC | #2
On Wed, 19 Sep 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, 19 Sep 2012 11:12:41 +1000, Dave Airlie <airlied@gmail.com> wrote:
>> From: Dave Airlie <airlied@redhat.com>
>> 
>> We hit this a lot with i915 and although we'd like to engineer things to hit
>> it a lot less, this commit at least makes it consume a few less cycles.
>> 
>> from something containing
>> movzwl 0x0(%rip),%r10d
>> to
>> add    %r8,%rdx
>> 
>> I only noticed it while using perf to profile something else.
>> 
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> I would have thought this is something the compiler should, with the
> appropriate hints, be smart enough to decide for itself as boot_cpu_data
> should be constant once defined. *shrug*

Since it's not obvious the compiler isn't smart enough, should the code
carry a comment so we don't forget?

BR,
Jani.
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 08758e0..3dbc7f1 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -37,12 +37,13 @@  drm_clflush_page(struct page *page)
 {
 	uint8_t *page_virtual;
 	unsigned int i;
+	const int size = boot_cpu_data.x86_clflush_size;
 
 	if (unlikely(page == NULL))
 		return;
 
 	page_virtual = kmap_atomic(page);
-	for (i = 0; i < PAGE_SIZE; i += boot_cpu_data.x86_clflush_size)
+	for (i = 0; i < PAGE_SIZE; i += size)
 		clflush(page_virtual + i);
 	kunmap_atomic(page_virtual);
 }