Re: [Mesa3d-dev] gallium: let drivers fallback on framebuffer clearing
On Thu, 2009-12-10 at 01:52 -0800, Marek Olšák wrote: > Keith, > > I've taken your comment into consideration and started laying out a > new simple driver module which I call Blitter. The idea is to provide > acceleration for operations like clear, surface_copy, and > surface_fill. The module doesn't depend on a CSO context, instead, a > driver must call appropriate util_blitter_save* functions to save CSOs > and a blit operation takes care of their restoration once it's done. > > I attached a patch illustrating the idea with the clear implemented > and a working example of usage, but it's not ready to get pushed yet. > > Please tell me what you think about it. Marek, This looks good to me. It looks like this approach keeps the implementation entirely on the driver side of the interface, which is what I was hoping for. I had assumed that doing this type of operation in the driver would require assistance "from above" for saving and restoring state. But it seems like you've been able to do without that, which is nice. Let me know how it progresses. Keith -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: let drivers fallback on framebuffer clearing
Keith, I've taken your comment into consideration and started laying out a new simple driver module which I call Blitter. The idea is to provide acceleration for operations like clear, surface_copy, and surface_fill. The module doesn't depend on a CSO context, instead, a driver must call appropriate util_blitter_save* functions to save CSOs and a blit operation takes care of their restoration once it's done. I attached a patch illustrating the idea with the clear implemented and a working example of usage, but it's not ready to get pushed yet. Please tell me what you think about it. Marek On Tue, Dec 8, 2009 at 1:42 PM, Keith Whitwell wrote: > On Mon, 2009-12-07 at 05:01 -0800, Marek Olšák wrote: >> Hi, >> >> Because some hardware under some circumstances (e.g. disabled tiling) >> may not be able to use a hardware-specific clear path, it would be >> nice to let a state tracker do the clearing by drawing a >> screen-aligned quad. I am proposing to change the pipe_context->clear >> function to return TRUE if buffers were cleared by the driver, and >> FALSE if the state tracker should do the clearing by taking a generic >> path. >> >> As an example, r300g can use the fast clearing only if colorbuffers >> and zbuffers are micro-tiled, otherwise it must draw a screen-aligned >> quad and doing it in the driver is really messy. >> >> The attached patch implements this interface change and adapts all >> state trackers and drivers. >> >> I noticed that a lot of drivers still use slow util_clear, so they >> will greatly benefit from this patch. It's up to their maintainers >> whether they enable it, I suspect some unstable drivers may not be >> able to draw a quad without visual errors. >> >> Please review. > > Marek, > > I think I'd prefer a scheme where we make it easier for the drivers to > do whatever they need internally to get clears done. This is mainly for > consistency with the rest of Gallium, where once we ask a driver to do > something, the contract is that it will get done, and that the > state-tracker can have a good idea about what a driver's capabilities > are ahead of time. > > In terms of this, it seems what we're really missing to allow drivers to > call equivalents of util_draw_quad() and friends is a way to > save/restore the current bound state which will get clobbered running > the quad blitter. > > Providing a general facility to support this in the driver gives the > equivalent functionality of this fallback patch on every path which > could potentially do this type of fallback, as opposed to adding return > values and dealing with fallbacks in every state tracker. > > So, I think I'll be naking this patch, but I recognise the issue and > would be interested to see ways that we can enable the driver to call > some driver-side equivalent of util_draw_quad() or similar to achieve > the clear. > > Keith > > From 518c29f9c1860a7e58f4e2c793af1b41350922e5 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Thu, 10 Dec 2009 10:25:33 +0100 Subject: [PATCH 1/2] util: add blitter --- src/gallium/auxiliary/util/Makefile|1 + src/gallium/auxiliary/util/SConscript |1 + src/gallium/auxiliary/util/u_blitter.c | 271 src/gallium/auxiliary/util/u_blitter.h | 153 ++ 4 files changed, 426 insertions(+), 0 deletions(-) create mode 100644 src/gallium/auxiliary/util/u_blitter.c create mode 100644 src/gallium/auxiliary/util/u_blitter.h diff --git a/src/gallium/auxiliary/util/Makefile b/src/gallium/auxiliary/util/Makefile index 1d8bb55..8df7445 100644 --- a/src/gallium/auxiliary/util/Makefile +++ b/src/gallium/auxiliary/util/Makefile @@ -9,6 +9,7 @@ C_SOURCES = \ u_debug_symbol.c \ u_debug_stack.c \ u_blit.c \ + u_blitter.c \ u_cache.c \ u_cpu_detect.c \ u_draw_quad.c \ diff --git a/src/gallium/auxiliary/util/SConscript b/src/gallium/auxiliary/util/SConscript index 8d99106..45d89c0 100644 --- a/src/gallium/auxiliary/util/SConscript +++ b/src/gallium/auxiliary/util/SConscript @@ -23,6 +23,7 @@ util = env.ConvenienceLibrary( source = [ 'u_bitmask.c', 'u_blit.c', + 'u_blitter.c', 'u_cache.c', 'u_cpu_detect.c', 'u_debug.c', diff --git a/src/gallium/auxiliary/util/u_blitter.c b/src/gallium/auxiliary/util/u_blitter.c new file mode 100644 index 000..9433c1a --- /dev/null +++ b/src/gallium/auxiliary/util/u_blitter.c @@ -0,0 +1,271 @@ +/** + * + * Copyright 2009 Marek Olšák + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sub license, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following c
Re: [Mesa3d-dev] gallium: let drivers fallback on framebuffer clearing
On Mon, 2009-12-07 at 05:01 -0800, Marek Olšák wrote: > Hi, > > Because some hardware under some circumstances (e.g. disabled tiling) > may not be able to use a hardware-specific clear path, it would be > nice to let a state tracker do the clearing by drawing a > screen-aligned quad. I am proposing to change the pipe_context->clear > function to return TRUE if buffers were cleared by the driver, and > FALSE if the state tracker should do the clearing by taking a generic > path. > > As an example, r300g can use the fast clearing only if colorbuffers > and zbuffers are micro-tiled, otherwise it must draw a screen-aligned > quad and doing it in the driver is really messy. > > The attached patch implements this interface change and adapts all > state trackers and drivers. > > I noticed that a lot of drivers still use slow util_clear, so they > will greatly benefit from this patch. It's up to their maintainers > whether they enable it, I suspect some unstable drivers may not be > able to draw a quad without visual errors. > > Please review. Marek, I think I'd prefer a scheme where we make it easier for the drivers to do whatever they need internally to get clears done. This is mainly for consistency with the rest of Gallium, where once we ask a driver to do something, the contract is that it will get done, and that the state-tracker can have a good idea about what a driver's capabilities are ahead of time. In terms of this, it seems what we're really missing to allow drivers to call equivalents of util_draw_quad() and friends is a way to save/restore the current bound state which will get clobbered running the quad blitter. Providing a general facility to support this in the driver gives the equivalent functionality of this fallback patch on every path which could potentially do this type of fallback, as opposed to adding return values and dealing with fallbacks in every state tracker. So, I think I'll be naking this patch, but I recognise the issue and would be interested to see ways that we can enable the driver to call some driver-side equivalent of util_draw_quad() or similar to achieve the clear. Keith -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
[Mesa3d-dev] gallium: let drivers fallback on framebuffer clearing
Hi,
Because some hardware under some circumstances (e.g. disabled tiling)
may not be able to use a hardware-specific clear path, it would be
nice to let a state tracker do the clearing by drawing a
screen-aligned quad. I am proposing to change the pipe_context->clear
function to return TRUE if buffers were cleared by the driver, and
FALSE if the state tracker should do the clearing by taking a generic
path.
As an example, r300g can use the fast clearing only if colorbuffers
and zbuffers are micro-tiled, otherwise it must draw a screen-aligned
quad and doing it in the driver is really messy.
The attached patch implements this interface change and adapts all
state trackers and drivers.
I noticed that a lot of drivers still use slow util_clear, so they
will greatly benefit from this patch. It's up to their maintainers
whether they enable it, I suspect some unstable drivers may not be
able to draw a quad without visual errors.
Please review.
Best regards
Marek Olšák
From a6d16e7bce330900d496593054ec6aeb3ed51a2f Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Marek=20Ol=C5=A1=C3=A1k?=
Date: Mon, 7 Dec 2009 00:33:17 +0100
Subject: [PATCH] gallium: let drivers fallback on framebuffer clearing
A driver can either clear a framebuffer by itself or can tell the state tracker
to draw a screen-aligned quad if a hardware-specific clear path is not
available.
---
src/gallium/drivers/cell/ppu/cell_clear.c|4 +++-
src/gallium/drivers/cell/ppu/cell_clear.h|3 ++-
src/gallium/drivers/i915/i915_clear.c|3 ++-
src/gallium/drivers/i915/i915_context.h |4 ++--
src/gallium/drivers/identity/id_context.c| 12 ++--
src/gallium/drivers/llvmpipe/lp_clear.c |4 +++-
src/gallium/drivers/llvmpipe/lp_clear.h |2 +-
src/gallium/drivers/nv04/nv04_clear.c| 10 ++
src/gallium/drivers/nv04/nv04_context.h |4 ++--
src/gallium/drivers/nv10/nv10_clear.c|3 ++-
src/gallium/drivers/nv10/nv10_context.h |2 +-
src/gallium/drivers/nv20/nv20_clear.c|3 ++-
src/gallium/drivers/nv20/nv20_context.h |2 +-
src/gallium/drivers/nv30/nv30_clear.c|3 ++-
src/gallium/drivers/nv30/nv30_context.h |2 +-
src/gallium/drivers/nv40/nv40_clear.c|3 ++-
src/gallium/drivers/nv40/nv40_context.h |2 +-
src/gallium/drivers/nv50/nv50_clear.c|4 +++-
src/gallium/drivers/nv50/nv50_context.h |2 +-
src/gallium/drivers/r300/r300_clear.c| 11 ++-
src/gallium/drivers/r300/r300_clear.h| 12 +++-
src/gallium/drivers/softpipe/sp_clear.c |5 +++--
src/gallium/drivers/softpipe/sp_clear.h |2 +-
src/gallium/drivers/svga/svga_context.h | 10 +-
src/gallium/drivers/svga/svga_pipe_clear.c |3 ++-
src/gallium/drivers/trace/tr_context.c |7 +--
src/gallium/include/pipe/p_context.h |4 +++-
src/gallium/state_trackers/vega/api_masks.c | 11 +++
src/gallium/state_trackers/vega/vg_context.c |6 +-
src/gallium/state_trackers/vega/vg_tracker.c |8 ++--
src/gallium/state_trackers/xorg/xorg_exa.c |7 +--
src/mesa/state_tracker/st_cb_clear.c | 12 +---
32 files changed, 107 insertions(+), 63 deletions(-)
diff --git a/src/gallium/drivers/cell/ppu/cell_clear.c b/src/gallium/drivers/cell/ppu/cell_clear.c
index 79ad687..553d924 100644
--- a/src/gallium/drivers/cell/ppu/cell_clear.c
+++ b/src/gallium/drivers/cell/ppu/cell_clear.c
@@ -48,7 +48,7 @@
/**
* Called via pipe->clear()
*/
-void
+boolean
cell_clear(struct pipe_context *pipe, unsigned buffers, const float *rgba,
double depth, unsigned stencil)
{
@@ -89,4 +89,6 @@ cell_clear(struct pipe_context *pipe, unsigned buffers, const float *rgba,
clr->surface = surfIndex;
clr->value = clearValue;
}
+
+ return TRUE;
}
diff --git a/src/gallium/drivers/cell/ppu/cell_clear.h b/src/gallium/drivers/cell/ppu/cell_clear.h
index 08e091a..9fc3a01 100644
--- a/src/gallium/drivers/cell/ppu/cell_clear.h
+++ b/src/gallium/drivers/cell/ppu/cell_clear.h
@@ -29,11 +29,12 @@
#ifndef CELL_CLEAR_H
#define CELL_CLEAR_H
+#include "pipe/p_compiler.h"
struct pipe_context;
-extern void
+extern boolean
cell_clear(struct pipe_context *pipe, unsigned buffers, const float *rgba,
double depth, unsigned stencil);
diff --git a/src/gallium/drivers/i915/i915_clear.c b/src/gallium/drivers/i915/i915_clear.c
index 90530f2..21528e5 100644
--- a/src/gallium/drivers/i915/i915_clear.c
+++ b/src/gallium/drivers/i915/i915_clear.c
@@ -39,10 +39,11 @@
* Clear the given buffers to the specified values.
* No masking, no scissor (clear entire buffer).
*/
-void
+boolean
i915_clear(struct pipe_context *pipe, unsigned buffers, const float *rgba,
double depth, unsigned stencil)
{
util_clear(pipe, &i915_context(pipe)->framebuffer, buffers, rgba, depth,
stencil
