Re: [Mesa-dev] [PATCH 14/17] Modified i915 intel_bufferobj_data to use _mesa_align_malloc instead of malloc.

2013-11-25 Thread Ian Romanick
On 11/24/2013 11:17 PM, Chris Forbes wrote:
 I'm not convinced that just using _mesa_align_malloc() is sufficient
 here. Issue 2 from the ARB_map_buffer_alignment spec says:
 
 2) What alignment should be used for MapBufferRange when offset is
 non-zero?
 
 RESOLVED:  The pointer returned should be consistent with the base of the
 buffer being aligned.  For example, if a buffer was mapped with an
 offset of 24 bytes on an implementation with a
 MIN_MAP_BUFFER_ALIGNMENT of 64, the value of the pointer returned
 would have a value of the form 64*N+24 for some integer N.
 
 Cc'ing Ian, since I assume this is based on his instructions.

Yeah... Ken told me that Eric mentioned this issue when Ken sent out
some patches to enable this extension just for i965.  There are a couple
ways to solve this, I think.

One way would be to modify MapBuffer and friends to only request aligned
offsets from the driver and do some pointer fidgeting.  For drivers that
advertise small alignments, that should be fine... for drivers that
advertise large alignments this could be a problem.

Another way would be some similar sorts of fudging in each driver.  The
MapBufferRange handler in the driver would have to do an aligned
allocation but only copy the necessary bits.

I don't have a strong opinion either way about the implementation...
though I would like to see some tests that could tickle this bug, if it
existed.  AFAIK, we tend to return a direct mapping, so an obvious test
of glMapBufferRange(target, 7, size, access) probably won't hit it.

 -- Chris
 
 
 On Mon, Nov 25, 2013 at 6:36 PM, Siavash Eliasi siavashser...@gmail.com 
 wrote:
 ---
  src/mesa/drivers/dri/i915/intel_buffer_objects.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/mesa/drivers/dri/i915/intel_buffer_objects.c 
 b/src/mesa/drivers/dri/i915/intel_buffer_objects.c
 index bc58c70..4fbf954 100644
 --- a/src/mesa/drivers/dri/i915/intel_buffer_objects.c
 +++ b/src/mesa/drivers/dri/i915/intel_buffer_objects.c
 @@ -137,7 +137,7 @@ intel_bufferobj_data(struct gl_context * ctx,
 * contents anyway.
 */
if (target == GL_ARRAY_BUFFER || target == GL_ELEMENT_ARRAY_BUFFER) {
 -intel_obj-sys_buffer = malloc(size);
 +intel_obj-sys_buffer = _mesa_align_malloc(size, 
 ctx-Const.MinMapBufferAlignment);
  if (intel_obj-sys_buffer != NULL) {
 if (data != NULL)
memcpy(intel_obj-sys_buffer, data, size);
 @@ -337,7 +337,7 @@ intel_bufferobj_map_range(struct gl_context * ctx,
 if ((access  GL_MAP_INVALIDATE_RANGE_BIT) 
 drm_intel_bo_busy(intel_obj-buffer)) {
if (access  GL_MAP_FLUSH_EXPLICIT_BIT) {
 -intel_obj-range_map_buffer = malloc(length);
 +intel_obj-range_map_buffer = _mesa_align_malloc(length, 
 ctx-Const.MinMapBufferAlignment);
  obj-Pointer = intel_obj-range_map_buffer;
} else {
  intel_obj-range_map_bo = drm_intel_bo_alloc(intel-bufmgr,
 --
 1.8.4.2

 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 14/17] Modified i915 intel_bufferobj_data to use _mesa_align_malloc instead of malloc.

2013-11-24 Thread Siavash Eliasi
---
 src/mesa/drivers/dri/i915/intel_buffer_objects.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i915/intel_buffer_objects.c 
b/src/mesa/drivers/dri/i915/intel_buffer_objects.c
index bc58c70..4fbf954 100644
--- a/src/mesa/drivers/dri/i915/intel_buffer_objects.c
+++ b/src/mesa/drivers/dri/i915/intel_buffer_objects.c
@@ -137,7 +137,7 @@ intel_bufferobj_data(struct gl_context * ctx,
* contents anyway.
*/
   if (target == GL_ARRAY_BUFFER || target == GL_ELEMENT_ARRAY_BUFFER) {
-intel_obj-sys_buffer = malloc(size);
+intel_obj-sys_buffer = _mesa_align_malloc(size, 
ctx-Const.MinMapBufferAlignment);
 if (intel_obj-sys_buffer != NULL) {
if (data != NULL)
   memcpy(intel_obj-sys_buffer, data, size);
@@ -337,7 +337,7 @@ intel_bufferobj_map_range(struct gl_context * ctx,
if ((access  GL_MAP_INVALIDATE_RANGE_BIT) 
drm_intel_bo_busy(intel_obj-buffer)) {
   if (access  GL_MAP_FLUSH_EXPLICIT_BIT) {
-intel_obj-range_map_buffer = malloc(length);
+intel_obj-range_map_buffer = _mesa_align_malloc(length, 
ctx-Const.MinMapBufferAlignment);
 obj-Pointer = intel_obj-range_map_buffer;
   } else {
 intel_obj-range_map_bo = drm_intel_bo_alloc(intel-bufmgr,
-- 
1.8.4.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 14/17] Modified i915 intel_bufferobj_data to use _mesa_align_malloc instead of malloc.

2013-11-24 Thread Chris Forbes
I'm not convinced that just using _mesa_align_malloc() is sufficient
here. Issue 2 from the ARB_map_buffer_alignment spec says:

2) What alignment should be used for MapBufferRange when offset is
non-zero?

RESOLVED:  The pointer returned should be consistent with the base of the
buffer being aligned.  For example, if a buffer was mapped with an
offset of 24 bytes on an implementation with a
MIN_MAP_BUFFER_ALIGNMENT of 64, the value of the pointer returned
would have a value of the form 64*N+24 for some integer N.

Cc'ing Ian, since I assume this is based on his instructions.

-- Chris


On Mon, Nov 25, 2013 at 6:36 PM, Siavash Eliasi siavashser...@gmail.com wrote:
 ---
  src/mesa/drivers/dri/i915/intel_buffer_objects.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/mesa/drivers/dri/i915/intel_buffer_objects.c 
 b/src/mesa/drivers/dri/i915/intel_buffer_objects.c
 index bc58c70..4fbf954 100644
 --- a/src/mesa/drivers/dri/i915/intel_buffer_objects.c
 +++ b/src/mesa/drivers/dri/i915/intel_buffer_objects.c
 @@ -137,7 +137,7 @@ intel_bufferobj_data(struct gl_context * ctx,
 * contents anyway.
 */
if (target == GL_ARRAY_BUFFER || target == GL_ELEMENT_ARRAY_BUFFER) {
 -intel_obj-sys_buffer = malloc(size);
 +intel_obj-sys_buffer = _mesa_align_malloc(size, 
 ctx-Const.MinMapBufferAlignment);
  if (intel_obj-sys_buffer != NULL) {
 if (data != NULL)
memcpy(intel_obj-sys_buffer, data, size);
 @@ -337,7 +337,7 @@ intel_bufferobj_map_range(struct gl_context * ctx,
 if ((access  GL_MAP_INVALIDATE_RANGE_BIT) 
 drm_intel_bo_busy(intel_obj-buffer)) {
if (access  GL_MAP_FLUSH_EXPLICIT_BIT) {
 -intel_obj-range_map_buffer = malloc(length);
 +intel_obj-range_map_buffer = _mesa_align_malloc(length, 
 ctx-Const.MinMapBufferAlignment);
  obj-Pointer = intel_obj-range_map_buffer;
} else {
  intel_obj-range_map_bo = drm_intel_bo_alloc(intel-bufmgr,
 --
 1.8.4.2

 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev