[Mesa-dev] [PATCH] Better explain why we are lowering the num_tile_pipes value for TAHITI (v2)

[Alexandre Demers]

v2: Clarify the relation between num_tiles_pipes and GB_TILE_MODE and the fix 
 needed for Tahiti as suggested by Marek.

Signed-off-by: Alexandre Demers 
---
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 49c310c..73ef051 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -405,8 +405,10 @@ static boolean do_winsys_init(struct radeon_drm_winsys *ws)
 radeon_get_drm_value(ws->fd, RADEON_INFO_NUM_TILE_PIPES, NULL,
  >info.num_tile_pipes);
 
-/* The kernel returns 12 for some cards for an unknown reason.
- * I thought this was supposed to be a power of two.
+/* "num_tiles_pipes" must be equal to the number of pipes (Px) in 
the
+/* pipe config field of the GB_TILE_MODE array. Only one card 
(Tahiti)
+/* reports a different value (12). Fix it by setting what's in the
+/* GB_TILE_MODE array (8).
  */
 if (ws->gen == DRV_SI && ws->info.num_tile_pipes == 12)
 ws->info.num_tile_pipes = 8;
-- 
2.7.1

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

Re: [Mesa-dev] [PATCH] Better explain why we are lowering the num_tile_pipes value for TAHITI

[Alexandre Demers]

On 2016-02-10 05:14, Marek Olšák wrote:

On Wed, Feb 10, 2016 at 2:11 AM, Alexandre Demers
 wrote:

Signed-off-by: Alexandre Demers 
---
  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 49c310c..aab81f9 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -405,8 +405,9 @@ static boolean do_winsys_init(struct radeon_drm_winsys *ws)
  radeon_get_drm_value(ws->fd, RADEON_INFO_NUM_TILE_PIPES, NULL,
   >info.num_tile_pipes);

-/* The kernel returns 12 for some cards for an unknown reason.
- * I thought this was supposed to be a power of two.
+/* Tahiti have a max_tile_pipes of 12 exceptionally. However, we
+ * work with power of two, so let's set it to the nearest power of
+ * two value.
   */

This is even worse. :) The proper explanation should be:

"num_tiles_pipes" must be equal to the number of pipes (Px) in the
pipe config field of the GB_TILE_MODE array. Only one card (Tahiti)
reports a different value (12). Fix it by setting what's in the
GB_TILE_MODE array (8).

Marek
I agree, sending a new version of the patch with exactly that comment. 
No confusion possible. ;)


Alexandre Demers

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

Re: [Mesa-dev] [PATCH] Better explain why we are lowering the num_tile_pipes value for TAHITI (v2)

[Marek Olšák]

Thanks, I will push this shortly.

Marek

On Wed, Feb 10, 2016 at 3:45 PM, Alexandre Demers
 wrote:
> v2: Clarify the relation between num_tiles_pipes and GB_TILE_MODE and the fix
>  needed for Tahiti as suggested by Marek.
>
> Signed-off-by: Alexandre Demers 
> ---
>  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c 
> b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> index 49c310c..73ef051 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -405,8 +405,10 @@ static boolean do_winsys_init(struct radeon_drm_winsys 
> *ws)
>  radeon_get_drm_value(ws->fd, RADEON_INFO_NUM_TILE_PIPES, NULL,
>   >info.num_tile_pipes);
>
> -/* The kernel returns 12 for some cards for an unknown reason.
> - * I thought this was supposed to be a power of two.
> +/* "num_tiles_pipes" must be equal to the number of pipes (Px) 
> in the
> +/* pipe config field of the GB_TILE_MODE array. Only one card 
> (Tahiti)
> +/* reports a different value (12). Fix it by setting what's in 
> the
> +/* GB_TILE_MODE array (8).
>   */
>  if (ws->gen == DRV_SI && ws->info.num_tile_pipes == 12)
>  ws->info.num_tile_pipes = 8;
> --
> 2.7.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Better explain why we are lowering the num_tile_pipes value for TAHITI

[Marek Olšák]

On Wed, Feb 10, 2016 at 2:11 AM, Alexandre Demers
 wrote:
> Signed-off-by: Alexandre Demers 
> ---
>  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c 
> b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> index 49c310c..aab81f9 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -405,8 +405,9 @@ static boolean do_winsys_init(struct radeon_drm_winsys 
> *ws)
>  radeon_get_drm_value(ws->fd, RADEON_INFO_NUM_TILE_PIPES, NULL,
>   >info.num_tile_pipes);
>
> -/* The kernel returns 12 for some cards for an unknown reason.
> - * I thought this was supposed to be a power of two.
> +/* Tahiti have a max_tile_pipes of 12 exceptionally. However, we
> + * work with power of two, so let's set it to the nearest power 
> of
> + * two value.
>   */

This is even worse. :) The proper explanation should be:

"num_tiles_pipes" must be equal to the number of pipes (Px) in the
pipe config field of the GB_TILE_MODE array. Only one card (Tahiti)
reports a different value (12). Fix it by setting what's in the
GB_TILE_MODE array (8).

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

[Mesa-dev] [PATCH] Better explain why we are lowering the num_tile_pipes value for TAHITI

[Alexandre Demers]

Signed-off-by: Alexandre Demers 
---
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 49c310c..aab81f9 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -405,8 +405,9 @@ static boolean do_winsys_init(struct radeon_drm_winsys *ws)
 radeon_get_drm_value(ws->fd, RADEON_INFO_NUM_TILE_PIPES, NULL,
  >info.num_tile_pipes);
 
-/* The kernel returns 12 for some cards for an unknown reason.
- * I thought this was supposed to be a power of two.
+/* Tahiti have a max_tile_pipes of 12 exceptionally. However, we 
+ * work with power of two, so let's set it to the nearest power of 
+ * two value.
  */
 if (ws->gen == DRV_SI && ws->info.num_tile_pipes == 12)
 ws->info.num_tile_pipes = 8;
-- 
2.7.1

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

Re: [Mesa-dev] [PATCH] Better explain why we are lowering the num_tile_pipes value for TAHITI

[Alexandre Demers]

On Tue, 9 Feb 2016 at 22:38 Michel Dänzer  wrote:

> On 10.02.2016 10:11, Alexandre Demers wrote:
> > Signed-off-by: Alexandre Demers 
> > ---
> >  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> > index 49c310c..aab81f9 100644
> > --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> > +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> > @@ -405,8 +405,9 @@ static boolean do_winsys_init(struct
> radeon_drm_winsys *ws)
> >  radeon_get_drm_value(ws->fd, RADEON_INFO_NUM_TILE_PIPES,
> NULL,
> >   >info.num_tile_pipes);
> >
> > -/* The kernel returns 12 for some cards for an unknown
> reason.
> > - * I thought this was supposed to be a power of two.
> > +/* Tahiti have a max_tile_pipes of 12 exceptionally.
> However, we
> > + * work with power of two, so let's set it to the nearest
> power of
> > + * two value.
> >   */
>
> Not sure that's better I'm afraid. It doesn't look like 12 is actually a
> possible hardware configuration, it might simply be a kernel driver bug.
>
>
> --
> Earthling Michel Dänzer   |   http://www.amd.com
> Libre software enthusiast | Mesa and X developer
>

On the other hand, the current comment isn't true either (and probably
farther from the truth): it's not true that we don't know why the kernel
returns 12 or which cards/gpus are affected (only tahiti is concerned). If
someone has to investigate this portion of code later in time, it will be
even harder to understand why the comment was added: it gives no clue about
the reported bug it fixes nor any information we have about the specific
GPU identified and the fact that this value is known to be the one set in
the kernel driver.

While it could be true that this is a driver bug, I don't have any way to
verify it. Also, according to what is available under ci_gpu_init() @
si.c#n3254 (where max_tile_pipes is defined), the value "12" was already
flagged as problematic (there is a comment under tile_config for the
default case). This comment was inserted by Alex Deucher himself in the
initial commit (0ce635d67f8c65f9f804abd77b63a65c08107e79). And finally, if
I understand correctly what Alex Deucher summed up under the thread
"[PATCH] winsys/radeon: fix a wrong NUM_TILE_PIPES value from the kernel",
he doesn't remember clearly where and why the value was defined as 12, most
probably a hardware value, but it needed to be mapped to 8 for software
usage.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Better explain why we are lowering the num_tile_pipes value for TAHITI

[Michel Dänzer]

On 10.02.2016 10:11, Alexandre Demers wrote:
> Signed-off-by: Alexandre Demers 
> ---
>  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c 
> b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> index 49c310c..aab81f9 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -405,8 +405,9 @@ static boolean do_winsys_init(struct radeon_drm_winsys 
> *ws)
>  radeon_get_drm_value(ws->fd, RADEON_INFO_NUM_TILE_PIPES, NULL,
>   >info.num_tile_pipes);
>  
> -/* The kernel returns 12 for some cards for an unknown reason.
> - * I thought this was supposed to be a power of two.
> +/* Tahiti have a max_tile_pipes of 12 exceptionally. However, we 
> + * work with power of two, so let's set it to the nearest power 
> of 
> + * two value.
>   */

Not sure that's better I'm afraid. It doesn't look like 12 is actually a
possible hardware configuration, it might simply be a kernel driver bug.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev