[PATCH 0/5] i965/skl: Add YF/YS tiling support

[PATCH 0/5] i965/skl: Add YF/YS tiling support

[Anuj Phogat]

Series is available at:
https://github.com/aphogat/drm.git, branch: tiling-yf-ys

Anuj Phogat (5):
  i965/skl: Add macros for Yf/Ys tiling formats
  i965/skl: Move tile_width computations out of
    drm_intel_gem_bo_tile_pitch
  i965/skl: Set tile width and height for YF/YS tiling
  Align YS tile base address to 64KB
  build: Bump the version to 2.4.61

 configure.ac             |  2 +-
 include/drm/i915_drm.h   |  2 ++
 intel/intel_bufmgr_gem.c | 77 +++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 66 insertions(+), 15 deletions(-)

-- 
2.3.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/5] i965/skl: Add macros for Yf/Ys tiling formats

[Anuj Phogat]

Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
---
 include/drm/i915_drm.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index ded43b1..a6c167c 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -842,6 +842,8 @@ struct drm_i915_gem_caching {
 #define I915_TILING_NONE	0
 #define I915_TILING_X		1
 #define I915_TILING_Y		2
+#define I915_TILING_YF          3
+#define I915_TILING_YS          4
 
 #define I915_BIT_6_SWIZZLE_NONE		0
 #define I915_BIT_6_SWIZZLE_9		1
-- 
2.3.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] i965/skl: Add macros for Yf/Ys tiling formats

[Daniel Vetter]

On Mon, Mar 30, 2015 at 02:00:04PM -0700, Anuj Phogat wrote:
> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
> ---
>  include/drm/i915_drm.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index ded43b1..a6c167c 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -842,6 +842,8 @@ struct drm_i915_gem_caching {
>  #define I915_TILING_NONE	0
>  #define I915_TILING_X		1
>  #define I915_TILING_Y		2
> +#define I915_TILING_YF          3
> +#define I915_TILING_YS          4

This is based on an old version of the Yf/Ys tiling patches which have not
been merged, so nack.

When you update the kernel headers in libdrm, _always_ use

$ make headers_install

from kernel sources and then copy over the headers unchanged to libdrm.
Never handedit i915_drm.h.
-Daniel

>  
>  #define I915_BIT_6_SWIZZLE_NONE		0
>  #define I915_BIT_6_SWIZZLE_9		1
> -- 
> 2.3.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] i965/skl: Add macros for Yf/Ys tiling formats

[Anuj Phogat]

On Tue, Mar 31, 2015 at 6:17 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Mar 30, 2015 at 02:00:04PM -0700, Anuj Phogat wrote:
>> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
>> ---
>>  include/drm/i915_drm.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
>> index ded43b1..a6c167c 100644
>> --- a/include/drm/i915_drm.h
>> +++ b/include/drm/i915_drm.h
>> @@ -842,6 +842,8 @@ struct drm_i915_gem_caching {
>>  #define I915_TILING_NONE     0
>>  #define I915_TILING_X                1
>>  #define I915_TILING_Y                2
>> +#define I915_TILING_YF          3
>> +#define I915_TILING_YS          4
>
> This is based on an old version of the Yf/Ys tiling patches which have not
> been merged, so nack.
>
Following your IRC chat with Kristian, and my offline discussion with
him, I will move the tile size computation (using width, height, cpp) for
Yf/Ys in to mesa. Then these definitions are not required in here.

> When you update the kernel headers in libdrm, _always_ use
>
> $ make headers_install
>
> from kernel sources and then copy over the headers unchanged to libdrm.
> Never handedit i915_drm.h.
OK. Thanks for the information.

> -Daniel
>
>>
>>  #define I915_BIT_6_SWIZZLE_NONE              0
>>  #define I915_BIT_6_SWIZZLE_9         1
>> --
>> 2.3.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/5] i965/skl: Move tile_width computations out of drm_intel_gem_bo_tile_pitch

[Anuj Phogat]

This will be utilized by next patch in this series.

Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
---
 intel/intel_bufmgr_gem.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 5a67f53..af44ba5 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -324,9 +324,9 @@ drm_intel_gem_bo_tile_size(drm_intel_bufmgr_gem *bufmgr_gem, unsigned long size,
  */
 static unsigned long
 drm_intel_gem_bo_tile_pitch(drm_intel_bufmgr_gem *bufmgr_gem,
-			    unsigned long pitch, uint32_t *tiling_mode)
+			    unsigned long pitch, unsigned long tile_width,
+			    uint32_t *tiling_mode)
 {
-	unsigned long tile_width;
 	unsigned long i;
 
 	/* If untiled, then just align it so that we can do rendering
@@ -335,13 +335,6 @@ drm_intel_gem_bo_tile_pitch(drm_intel_bufmgr_gem *bufmgr_gem,
 	if (*tiling_mode == I915_TILING_NONE)
 		return ALIGN(pitch, 64);
 
-	if (*tiling_mode == I915_TILING_X
-			|| (IS_915(bufmgr_gem->pci_device)
-			    && *tiling_mode == I915_TILING_Y))
-		tile_width = 512;
-	else
-		tile_width = 128;
-
 	/* 965 is flexible */
 	if (bufmgr_gem->gen >= 4)
 		return ROUND_UP_TO(pitch, tile_width);
@@ -816,7 +809,7 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name,
 			     unsigned long *pitch, unsigned long flags)
 {
 	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
-	unsigned long size, stride;
+	unsigned long size, stride, tile_width;
 	uint32_t tiling;
 
 	do {
@@ -842,14 +835,18 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name,
 			height_alignment = 16;
 		else if (tiling == I915_TILING_X
 			|| (IS_915(bufmgr_gem->pci_device)
-			    && tiling == I915_TILING_Y))
+			    && tiling == I915_TILING_Y)) {
 			height_alignment = 8;
-		else if (tiling == I915_TILING_Y)
+			tile_width = 512;
+		} else if (tiling == I915_TILING_Y)
 			height_alignment = 32;
+			tile_width = 128;
+		}
 		aligned_y = ALIGN(y, height_alignment);
 
 		stride = x * cpp;
-		stride = drm_intel_gem_bo_tile_pitch(bufmgr_gem, stride, tiling_mode);
+		stride = drm_intel_gem_bo_tile_pitch(bufmgr_gem, stride,
+						     tile_width, tiling_mode);
 		size = stride * aligned_y;
 		size = drm_intel_gem_bo_tile_size(bufmgr_gem, size, tiling_mode);
 	} while (*tiling_mode != tiling);
-- 
2.3.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/5] i965/skl: Set tile width and height for YF/YS tiling

[Anuj Phogat]

I'm still passing tiling=I915_TILING_Y in drm_intel_gem_bo_alloc_internal()
in case of YF/YS tiling. Passing tiling=I915_TILING_{YF,YS} causes bo
allocation failure. Any advice what's the right thing to do here?

Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
---
 intel/intel_bufmgr_gem.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index af44ba5..7c50e26 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -803,6 +803,39 @@ drm_intel_gem_bo_alloc(drm_intel_bufmgr *bufmgr,
 					       I915_TILING_NONE, 0);
 }
 
+/* This function does tile height computations valid only for Yf/Ys tiled
+ * surfaces.
+ */
+static unsigned
+drm_intel_gem_tile_height(unsigned bpp, uint32_t tiling)
+{
+	unsigned tile_height;
+	assert(tiling == I915_TILING_YF || tiling == I915_TILING_YS);
+
+	switch (bpp) {
+	case 8:
+		tile_height = 64;
+		break;
+	case 16:
+	case 32:
+		tile_height = 32;
+		break;
+	case 64:
+	case 128:
+		tile_height = 16;
+		break;
+	default:
+		printf("Invalid bits per pixel in %s: bpp = %d\n",
+		       __FUNCTION__, bpp);
+		return 0;
+	}
+
+	if (tiling == I915_TILING_YS)
+		tile_height *= 4;
+
+	return tile_height;
+}
+
 static drm_intel_bo *
 drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name,
 			     int x, int y, int cpp, uint32_t *tiling_mode,
@@ -838,7 +871,15 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name,
 			    && tiling == I915_TILING_Y)) {
 			height_alignment = 8;
 			tile_width = 512;
-		} else if (tiling == I915_TILING_Y)
+		} else if (tiling == I915_TILING_YF ||
+			   tiling == I915_TILING_YS) {
+			unsigned bpp = cpp * 8;
+			unsigned aspect_ratio =
+				(bpp == 16 || bpp == 64) ? 2 : 1;
+			height_alignment =
+				drm_intel_gem_tile_height(bpp, tiling);
+			tile_width = height_alignment * cpp * aspect_ratio;
+		} else if (tiling == I915_TILING_Y){
 			height_alignment = 32;
 			tile_width = 128;
 		}
@@ -855,6 +896,12 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name,
 	if (tiling == I915_TILING_NONE)
 		stride = 0;
 
+	/* Use I915_TILING_Y in drm_intel_gem_bo_alloc_internal() in case of
+	 * YF/YS tiling.
+	 */
+	tiling = (tiling == I915_TILING_YF || tiling == I915_TILING_YS) ?
+		  I915_TILING_Y : tiling;
+
 	return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, flags,
 					       tiling, stride);
 }
-- 
2.3.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] i965/skl: Set tile width and height for YF/YS tiling

[Tvrtko Ursulin]

Hi,

On 03/30/2015 10:00 PM, Anuj Phogat wrote:
> I'm still passing tiling=I915_TILING_Y in drm_intel_gem_bo_alloc_internal()
> in case of YF/YS tiling. Passing tiling=I915_TILING_{YF,YS} causes bo
> allocation failure. Any advice what's the right thing to do here?

Officially approved idea is to pass I915_TILING_NONE to the kernel since 
it doesn't need to know. (It is not possible to map Yf/Ys into GTT for 
de-tiling.)

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/5] Align YS tile base address to 64KB

[Anuj Phogat]

Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
---
 intel/intel_bufmgr_gem.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 7c50e26..775a9f9 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -289,8 +289,13 @@ drm_intel_gem_bo_tile_size(drm_intel_bufmgr_gem *bufmgr_gem, unsigned long size,
 	if (*tiling_mode == I915_TILING_NONE)
 		return size;
 
+	/* Tiled surface base addresses must be tile aligned (64KB aligned
+	 * for TileYS, 4KB aligned for all other tile modes).
+	 */
+	if (*tiling_mode == I915_TILING_YS)
+		return ROUND_UP_TO(size, 64 * 1024);
 	/* 965+ just need multiples of page size for tiling */
-	if (bufmgr_gem->gen >= 4)
+	else if (bufmgr_gem->gen >= 4)
 		return ROUND_UP_TO(size, 4096);
 
 	/* Older chips need powers of two, of at least 512k or 1M */
-- 
2.3.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] Align YS tile base address to 64KB

[Damien Lespiau]

On Mon, Mar 30, 2015 at 02:00:07PM -0700, Anuj Phogat wrote:
> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
> ---
>  intel/intel_bufmgr_gem.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index 7c50e26..775a9f9 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -289,8 +289,13 @@ drm_intel_gem_bo_tile_size(drm_intel_bufmgr_gem *bufmgr_gem, unsigned long size,
>  	if (*tiling_mode == I915_TILING_NONE)
>  		return size;
>  
> +	/* Tiled surface base addresses must be tile aligned (64KB aligned
> +	 * for TileYS, 4KB aligned for all other tile modes).
> +	 */
> +	if (*tiling_mode == I915_TILING_YS)
> +		return ROUND_UP_TO(size, 64 * 1024);
>  	/* 965+ just need multiples of page size for tiling */
> -	if (bufmgr_gem->gen >= 4)
> +	else if (bufmgr_gem->gen >= 4)
>  		return ROUND_UP_TO(size, 4096);

I'm confused. You're saying you want to align the address of those
buffers to 64k, but here we're talking about the object size. At the
moment, the kernel places buffers in the address space and it was chosen
that the kernel didn't need to know about those tiling formats. So we
need something else if that constraint is indeed true (could you tell
us the source for this assertion? privately if needed).

Thanks,

-- 
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] Align YS tile base address to 64KB

[Anuj Phogat]

On Tue, Mar 31, 2015 at 7:26 AM, Damien Lespiau
<damien.lespiau@intel.com> wrote:
> On Mon, Mar 30, 2015 at 02:00:07PM -0700, Anuj Phogat wrote:
>> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
>> ---
>>  intel/intel_bufmgr_gem.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
>> index 7c50e26..775a9f9 100644
>> --- a/intel/intel_bufmgr_gem.c
>> +++ b/intel/intel_bufmgr_gem.c
>> @@ -289,8 +289,13 @@ drm_intel_gem_bo_tile_size(drm_intel_bufmgr_gem *bufmgr_gem, unsigned long size,
>>       if (*tiling_mode == I915_TILING_NONE)
>>               return size;
>>
>> +     /* Tiled surface base addresses must be tile aligned (64KB aligned
>> +      * for TileYS, 4KB aligned for all other tile modes).
>> +      */
>> +     if (*tiling_mode == I915_TILING_YS)
>> +             return ROUND_UP_TO(size, 64 * 1024);
>>       /* 965+ just need multiples of page size for tiling */
>> -     if (bufmgr_gem->gen >= 4)
>> +     else if (bufmgr_gem->gen >= 4)
>>               return ROUND_UP_TO(size, 4096);
>
> I'm confused. You're saying you want to align the address of those
> buffers to 64k, but here we're talking about the object size. At the
> moment, the kernel places buffers in the address space and it was chosen
> that the kernel didn't need to know about those tiling formats. So we
> need something else if that constraint is indeed true (could you tell
> us the source for this assertion? privately if needed).
>
This comment is invalid here. It was meant for surface state in Mesa.
I'll remove it.

> Thanks,
>
> --
> Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] Align YS tile base address to 64KB

[Damien Lespiau]

On Tue, Mar 31, 2015 at 10:49:22AM -0700, Anuj Phogat wrote:
> On Tue, Mar 31, 2015 at 7:26 AM, Damien Lespiau
> <damien.lespiau@intel.com> wrote:
> > On Mon, Mar 30, 2015 at 02:00:07PM -0700, Anuj Phogat wrote:
> >> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
> >> ---
> >>  intel/intel_bufmgr_gem.c | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> >> index 7c50e26..775a9f9 100644
> >> --- a/intel/intel_bufmgr_gem.c
> >> +++ b/intel/intel_bufmgr_gem.c
> >> @@ -289,8 +289,13 @@ drm_intel_gem_bo_tile_size(drm_intel_bufmgr_gem *bufmgr_gem, unsigned long size,
> >>       if (*tiling_mode == I915_TILING_NONE)
> >>               return size;
> >>
> >> +     /* Tiled surface base addresses must be tile aligned (64KB aligned
> >> +      * for TileYS, 4KB aligned for all other tile modes).
> >> +      */
> >> +     if (*tiling_mode == I915_TILING_YS)
> >> +             return ROUND_UP_TO(size, 64 * 1024);
> >>       /* 965+ just need multiples of page size for tiling */
> >> -     if (bufmgr_gem->gen >= 4)
> >> +     else if (bufmgr_gem->gen >= 4)
> >>               return ROUND_UP_TO(size, 4096);
> >
> > I'm confused. You're saying you want to align the address of those
> > buffers to 64k, but here we're talking about the object size. At the
> > moment, the kernel places buffers in the address space and it was chosen
> > that the kernel didn't need to know about those tiling formats. So we
> > need something else if that constraint is indeed true (could you tell
> > us the source for this assertion? privately if needed).
> >
> This comment is invalid here. It was meant for surface state in Mesa.
> I'll remove it.

Don't you have the exact same problem? how does the kernel know about
this alignment constraint when resolving the relocation?

-- 
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] Align YS tile base address to 64KB

[Daniel Vetter]

On Tue, Mar 31, 2015 at 06:57:07PM +0100, Damien Lespiau wrote:
> On Tue, Mar 31, 2015 at 10:49:22AM -0700, Anuj Phogat wrote:
> > On Tue, Mar 31, 2015 at 7:26 AM, Damien Lespiau
> > <damien.lespiau@intel.com> wrote:
> > > On Mon, Mar 30, 2015 at 02:00:07PM -0700, Anuj Phogat wrote:
> > >> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
> > >> ---
> > >>  intel/intel_bufmgr_gem.c | 7 ++++++-
> > >>  1 file changed, 6 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > >> index 7c50e26..775a9f9 100644
> > >> --- a/intel/intel_bufmgr_gem.c
> > >> +++ b/intel/intel_bufmgr_gem.c
> > >> @@ -289,8 +289,13 @@ drm_intel_gem_bo_tile_size(drm_intel_bufmgr_gem *bufmgr_gem, unsigned long size,
> > >>       if (*tiling_mode == I915_TILING_NONE)
> > >>               return size;
> > >>
> > >> +     /* Tiled surface base addresses must be tile aligned (64KB aligned
> > >> +      * for TileYS, 4KB aligned for all other tile modes).
> > >> +      */
> > >> +     if (*tiling_mode == I915_TILING_YS)
> > >> +             return ROUND_UP_TO(size, 64 * 1024);
> > >>       /* 965+ just need multiples of page size for tiling */
> > >> -     if (bufmgr_gem->gen >= 4)
> > >> +     else if (bufmgr_gem->gen >= 4)
> > >>               return ROUND_UP_TO(size, 4096);
> > >
> > > I'm confused. You're saying you want to align the address of those
> > > buffers to 64k, but here we're talking about the object size. At the
> > > moment, the kernel places buffers in the address space and it was chosen
> > > that the kernel didn't need to know about those tiling formats. So we
> > > need something else if that constraint is indeed true (could you tell
> > > us the source for this assertion? privately if needed).
> > >
> > This comment is invalid here. It was meant for surface state in Mesa.
> > I'll remove it.
> 
> Don't you have the exact same problem? how does the kernel know about
> this alignment constraint when resolving the relocation?

struct drm_i915_gem_exec_object2.alignment goes back to some gen2/3 design
ideas that (afaik at least) have never been used really but survived until
now. Fancy how we can reuse that 7 generations later ;-)

And at least on a quick readthrough the code is all still functional too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] Align YS tile base address to 64KB

[Anuj Phogat]

On Tue, Mar 31, 2015 at 11:11 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Mar 31, 2015 at 06:57:07PM +0100, Damien Lespiau wrote:
>> On Tue, Mar 31, 2015 at 10:49:22AM -0700, Anuj Phogat wrote:
>> > On Tue, Mar 31, 2015 at 7:26 AM, Damien Lespiau
>> > <damien.lespiau@intel.com> wrote:
>> > > On Mon, Mar 30, 2015 at 02:00:07PM -0700, Anuj Phogat wrote:
>> > >> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
>> > >> ---
>> > >>  intel/intel_bufmgr_gem.c | 7 ++++++-
>> > >>  1 file changed, 6 insertions(+), 1 deletion(-)
>> > >>
>> > >> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
>> > >> index 7c50e26..775a9f9 100644
>> > >> --- a/intel/intel_bufmgr_gem.c
>> > >> +++ b/intel/intel_bufmgr_gem.c
>> > >> @@ -289,8 +289,13 @@ drm_intel_gem_bo_tile_size(drm_intel_bufmgr_gem *bufmgr_gem, unsigned long size,
>> > >>       if (*tiling_mode == I915_TILING_NONE)
>> > >>               return size;
>> > >>
>> > >> +     /* Tiled surface base addresses must be tile aligned (64KB aligned
>> > >> +      * for TileYS, 4KB aligned for all other tile modes).
>> > >> +      */
>> > >> +     if (*tiling_mode == I915_TILING_YS)
>> > >> +             return ROUND_UP_TO(size, 64 * 1024);
>> > >>       /* 965+ just need multiples of page size for tiling */
>> > >> -     if (bufmgr_gem->gen >= 4)
>> > >> +     else if (bufmgr_gem->gen >= 4)
>> > >>               return ROUND_UP_TO(size, 4096);
>> > >
>> > > I'm confused. You're saying you want to align the address of those
>> > > buffers to 64k, but here we're talking about the object size. At the
>> > > moment, the kernel places buffers in the address space and it was chosen
>> > > that the kernel didn't need to know about those tiling formats. So we
>> > > need something else if that constraint is indeed true (could you tell
>> > > us the source for this assertion? privately if needed).
>> > >
>> > This comment is invalid here. It was meant for surface state in Mesa.
>> > I'll remove it.
>>
>> Don't you have the exact same problem? how does the kernel know about
>> this alignment constraint when resolving the relocation?
>
> struct drm_i915_gem_exec_object2.alignment goes back to some gen2/3 design
> ideas that (afaik at least) have never been used really but survived until
> now. Fancy how we can reuse that 7 generations later ;-)
>
> And at least on a quick readthrough the code is all still functional too.
Yes, I'm planning to make use of it and pass alignment value for Yf/Ys
from Mesa. Thanks.

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/5] build: Bump the version to 2.4.61

[Anuj Phogat]

This is required due to new macros added to i915_drm.h.
These macros are used by i965 driver.

Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 155d577..17c0e71 100644
--- a/configure.ac
+++ b/configure.ac
@@ -20,7 +20,7 @@
 
 AC_PREREQ([2.63])
 AC_INIT([libdrm],
-        [2.4.60],
+        [2.4.61],
         [https://bugs.freedesktop.org/enter_bug.cgi?product=DRI],
         [libdrm])
 
-- 
2.3.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] build: Bump the version to 2.4.61

[Damien Lespiau]

On Mon, Mar 30, 2015 at 02:00:08PM -0700, Anuj Phogat wrote:
> This is required due to new macros added to i915_drm.h.
> These macros are used by i965 driver.
> 
> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
> ---

Hi,

Bumping the libdrm number is done when releasing, so we can't quite take
patches like this unless we're doing a release just after applying them.
Usually the person doing the release bumps the version number at the
same time.

-- 
Damien

>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 155d577..17c0e71 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -20,7 +20,7 @@
>  
>  AC_PREREQ([2.63])
>  AC_INIT([libdrm],
> -        [2.4.60],
> +        [2.4.61],
>          [https://bugs.freedesktop.org/enter_bug.cgi?product=DRI],
>          [libdrm])
>  
> -- 
> 2.3.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] build: Bump the version to 2.4.61

[Anuj Phogat]

On Tue, Mar 31, 2015 at 7:28 AM, Damien Lespiau
<damien.lespiau@intel.com> wrote:
> On Mon, Mar 30, 2015 at 02:00:08PM -0700, Anuj Phogat wrote:
>> This is required due to new macros added to i915_drm.h.
>> These macros are used by i965 driver.
>>
>> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
>> ---
>
> Hi,
>
> Bumping the libdrm number is done when releasing, so we can't quite take
> patches like this unless we're doing a release just after applying them.
> Usually the person doing the release bumps the version number at the
> same time.
OK.
>
> --
> Damien
>
>>  configure.ac | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 155d577..17c0e71 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -20,7 +20,7 @@
>>
>>  AC_PREREQ([2.63])
>>  AC_INIT([libdrm],
>> -        [2.4.60],
>> +        [2.4.61],
>>          [https://bugs.freedesktop.org/enter_bug.cgi?product=DRI],
>>          [libdrm])
>>
>> --
>> 2.3.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] build: Bump the version to 2.4.61

[Daniel Vetter]

On Tue, Mar 31, 2015 at 10:51:12AM -0700, Anuj Phogat wrote:
> On Tue, Mar 31, 2015 at 7:28 AM, Damien Lespiau
> <damien.lespiau@intel.com> wrote:
> > On Mon, Mar 30, 2015 at 02:00:08PM -0700, Anuj Phogat wrote:
> >> This is required due to new macros added to i915_drm.h.
> >> These macros are used by i965 driver.
> >>
> >> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
> >> ---
> >
> > Hi,
> >
> > Bumping the libdrm number is done when releasing, so we can't quite take
> > patches like this unless we're doing a release just after applying them.
> > Usually the person doing the release bumps the version number at the
> > same time.
> OK.

Also this is all done by the release script to make sure it follows
procedure and nothing is forgotten (like building&uploading tarballs and
sending out the release announcement).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx