* [PATCH] drm/amd/display: Initialize num_pkrs on VANGOGH.
@ 2020-10-20 22:31 Bas Nieuwenhuizen
2020-10-20 23:47 ` Marek Olšák
2020-10-21 16:09 ` [PATCH] drm/amd/display: Interpret log_2(0) as 0 Harry Wentland
0 siblings, 2 replies; 6+ messages in thread
From: Bas Nieuwenhuizen @ 2020-10-20 22:31 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Roman.Li, Bas Nieuwenhuizen
As far a I can tell uses a variant of DCN3xx which uses num_pkrs.
If we do not initialize the variable we will set the register field
to ilog2(0) = -1, though the mask will reduce that to 7. Pretty sure
7 is not the value we want here.
Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
Found while rebasing my modifiers series. Not tested on HW.
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6855aad7f312..2713caac4f2a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4074,7 +4074,8 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
#ifdef CONFIG_DRM_AMD_DC_DCN3_0
if (adev->asic_type == CHIP_SIENNA_CICHLID ||
adev->asic_type == CHIP_NAVY_FLOUNDER ||
- adev->asic_type == CHIP_DIMGREY_CAVEFISH)
+ adev->asic_type == CHIP_DIMGREY_CAVEFISH ||
+ adev->asic_type == CHIP_VANGOGH)
tiling_info->gfx9.num_pkrs = adev->gfx.config.gb_addr_config_fields.num_pkrs;
#endif
ret = fill_plane_dcc_attributes(adev, afb, format, rotation,
--
2.28.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amd/display: Initialize num_pkrs on VANGOGH.
2020-10-20 22:31 [PATCH] drm/amd/display: Initialize num_pkrs on VANGOGH Bas Nieuwenhuizen
@ 2020-10-20 23:47 ` Marek Olšák
2020-10-21 2:48 ` Huang Rui
2020-10-21 16:09 ` [PATCH] drm/amd/display: Interpret log_2(0) as 0 Harry Wentland
1 sibling, 1 reply; 6+ messages in thread
From: Marek Olšák @ 2020-10-20 23:47 UTC (permalink / raw)
To: Bas Nieuwenhuizen; +Cc: Deucher, Alexander, Roman.Li, amd-gfx mailing list
[-- Attachment #1.1: Type: text/plain, Size: 1804 bytes --]
FYI, this fixes tiling on VanGogh.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Marek
On Tue, Oct 20, 2020 at 6:31 PM Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
wrote:
> As far a I can tell uses a variant of DCN3xx which uses num_pkrs.
>
> If we do not initialize the variable we will set the register field
> to ilog2(0) = -1, though the mask will reduce that to 7. Pretty sure
> 7 is not the value we want here.
>
> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
>
> Found while rebasing my modifiers series. Not tested on HW.
>
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 6855aad7f312..2713caac4f2a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4074,7 +4074,8 @@ fill_plane_buffer_attributes(struct amdgpu_device
> *adev,
> #ifdef CONFIG_DRM_AMD_DC_DCN3_0
> if (adev->asic_type == CHIP_SIENNA_CICHLID ||
> adev->asic_type == CHIP_NAVY_FLOUNDER ||
> - adev->asic_type == CHIP_DIMGREY_CAVEFISH)
> + adev->asic_type == CHIP_DIMGREY_CAVEFISH ||
> + adev->asic_type == CHIP_VANGOGH)
> tiling_info->gfx9.num_pkrs =
> adev->gfx.config.gb_addr_config_fields.num_pkrs;
> #endif
> ret = fill_plane_dcc_attributes(adev, afb, format,
> rotation,
> --
> 2.28.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
[-- Attachment #1.2: Type: text/html, Size: 2583 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amd/display: Initialize num_pkrs on VANGOGH.
2020-10-20 23:47 ` Marek Olšák
@ 2020-10-21 2:48 ` Huang Rui
0 siblings, 0 replies; 6+ messages in thread
From: Huang Rui @ 2020-10-21 2:48 UTC (permalink / raw)
To: Marek Olšák, Bas Nieuwenhuizen
Cc: Deucher, Alexander, Roman.Li, amd-gfx mailing list
On Tue, Oct 20, 2020 at 07:47:42PM -0400, Marek Olšák wrote:
> FYI, this fixes tiling on VanGogh.
>
> Reviewed-by: Marek Olšák <marek.olsak@amd.com>
>
> Marek
>
> On Tue, Oct 20, 2020 at 6:31 PM Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> wrote:
>
> > As far a I can tell uses a variant of DCN3xx which uses num_pkrs.
> >
> > If we do not initialize the variable we will set the register field
> > to ilog2(0) = -1, though the mask will reduce that to 7. Pretty sure
> > 7 is not the value we want here.
> >
> > Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> > ---
> >
> > Found while rebasing my modifiers series. Not tested on HW.
Yes! Thank you so much, Bas!
I just verified the tiling issue got resovled with your patch! :-)
Applied.
Reviewed-and-Tested-by: Huang Rui <ray.huang@amd.com>
> >
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 6855aad7f312..2713caac4f2a 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -4074,7 +4074,8 @@ fill_plane_buffer_attributes(struct amdgpu_device
> > *adev,
> > #ifdef CONFIG_DRM_AMD_DC_DCN3_0
> > if (adev->asic_type == CHIP_SIENNA_CICHLID ||
> > adev->asic_type == CHIP_NAVY_FLOUNDER ||
> > - adev->asic_type == CHIP_DIMGREY_CAVEFISH)
> > + adev->asic_type == CHIP_DIMGREY_CAVEFISH ||
> > + adev->asic_type == CHIP_VANGOGH)
> > tiling_info->gfx9.num_pkrs =
> > adev->gfx.config.gb_addr_config_fields.num_pkrs;
> > #endif
> > ret = fill_plane_dcc_attributes(adev, afb, format,
> > rotation,
> > --
> > 2.28.0
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cray.huang%40amd.com%7C9afdb4e0a2e949655c9b08d87552a1bf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637388345070265382%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=SbojOTLaOXt85OSbYjyRPozLAsqBwXpvoYVBu7c0Ido%3D&reserved=0
> >
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cray.huang%40amd.com%7C9afdb4e0a2e949655c9b08d87552a1bf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637388345070305360%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uPui9S6bxwp%2B4J4uav4FekR1zOZ9VeHYcdrOSA9ZFKg%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/amd/display: Interpret log_2(0) as 0
2020-10-20 22:31 [PATCH] drm/amd/display: Initialize num_pkrs on VANGOGH Bas Nieuwenhuizen
2020-10-20 23:47 ` Marek Olšák
@ 2020-10-21 16:09 ` Harry Wentland
2020-10-21 16:39 ` Bas Nieuwenhuizen
1 sibling, 1 reply; 6+ messages in thread
From: Harry Wentland @ 2020-10-21 16:09 UTC (permalink / raw)
To: amd-gfx, alexander.deucher, Roman.Li, bas; +Cc: Harry Wentland
Even though log(0) is technically undefined our code assumes that
log_2(0) is 0. This mirrors the current behavior of our log_2
implementation on non-Linux platforms.
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
What's num_pkrs value is upstream Mesa providing for CHIP_VANGOGH?
I saw that problem at bringup with an internal Mesa and had this fix
for it.
Harry
drivers/gpu/drm/amd/display/dc/basics/conversion.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/basics/conversion.h b/drivers/gpu/drm/amd/display/dc/basics/conversion.h
index ade785c4fdc7..da9883ec7b1c 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/conversion.h
+++ b/drivers/gpu/drm/amd/display/dc/basics/conversion.h
@@ -40,7 +40,11 @@ void convert_float_matrix(
static inline unsigned int log_2(unsigned int num)
{
- return ilog2(num);
+ /*
+ * Technically log(0) is undefined, but our code is structured
+ * in a way that assumes log(0) = 0
+ */
+ return num ? ilog2(num) : num;
}
#endif
--
2.28.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/amd/display: Interpret log_2(0) as 0
2020-10-21 16:09 ` [PATCH] drm/amd/display: Interpret log_2(0) as 0 Harry Wentland
@ 2020-10-21 16:39 ` Bas Nieuwenhuizen
2020-10-21 20:10 ` Marek Olšák
0 siblings, 1 reply; 6+ messages in thread
From: Bas Nieuwenhuizen @ 2020-10-21 16:39 UTC (permalink / raw)
To: Harry Wentland; +Cc: Alex Deucher, Roman.Li, amd-gfx mailing list
On Wed, Oct 21, 2020 at 6:09 PM Harry Wentland <harry.wentland@amd.com> wrote:
>
> Even though log(0) is technically undefined our code assumes that
> log_2(0) is 0. This mirrors the current behavior of our log_2
> implementation on non-Linux platforms.
>
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> ---
>
> What's num_pkrs value is upstream Mesa providing for CHIP_VANGOGH?
So mesa isn't quite providing a num_pkrs value to the HW as the render
HW doesn't require you to set one, it assumes something.
In practice mesa still uses num_pkrs, which is provided by a readback
of the GB_ADDR_CFG(CONFIG?) register info provided by the kernel. This
can be used for e.g. determining texture coordinates (like we do for
the displayable DCC transform) but otherwise isn't really needed,
since the tile size for texture allocation is not dependent on
num_pkrs.
so outside of displayable DCC I think you'd be able to get far in mesa
with a wrong num_pkrs.
>
> I saw that problem at bringup with an internal Mesa and had this fix
> for it.
>
> Harry
>
> drivers/gpu/drm/amd/display/dc/basics/conversion.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/conversion.h b/drivers/gpu/drm/amd/display/dc/basics/conversion.h
> index ade785c4fdc7..da9883ec7b1c 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/conversion.h
> +++ b/drivers/gpu/drm/amd/display/dc/basics/conversion.h
> @@ -40,7 +40,11 @@ void convert_float_matrix(
>
> static inline unsigned int log_2(unsigned int num)
> {
> - return ilog2(num);
> + /*
> + * Technically log(0) is undefined, but our code is structured
> + * in a way that assumes log(0) = 0
> + */
> + return num ? ilog2(num) : num;
> }
>
> #endif
> --
> 2.28.0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/amd/display: Interpret log_2(0) as 0
2020-10-21 16:39 ` Bas Nieuwenhuizen
@ 2020-10-21 20:10 ` Marek Olšák
0 siblings, 0 replies; 6+ messages in thread
From: Marek Olšák @ 2020-10-21 20:10 UTC (permalink / raw)
To: Bas Nieuwenhuizen
Cc: Alex Deucher, Harry Wentland, Roman.Li, amd-gfx mailing list
[-- Attachment #1.1: Type: text/plain, Size: 2281 bytes --]
num_pkrs is a hardware config parameter like the number of shader engines.
It's constant for each chip.
Marek
On Wed, Oct 21, 2020 at 12:39 PM Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
wrote:
> On Wed, Oct 21, 2020 at 6:09 PM Harry Wentland <harry.wentland@amd.com>
> wrote:
> >
> > Even though log(0) is technically undefined our code assumes that
> > log_2(0) is 0. This mirrors the current behavior of our log_2
> > implementation on non-Linux platforms.
> >
> > Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> > ---
> >
> > What's num_pkrs value is upstream Mesa providing for CHIP_VANGOGH?
>
> So mesa isn't quite providing a num_pkrs value to the HW as the render
> HW doesn't require you to set one, it assumes something.
>
> In practice mesa still uses num_pkrs, which is provided by a readback
> of the GB_ADDR_CFG(CONFIG?) register info provided by the kernel. This
> can be used for e.g. determining texture coordinates (like we do for
> the displayable DCC transform) but otherwise isn't really needed,
> since the tile size for texture allocation is not dependent on
> num_pkrs.
>
> so outside of displayable DCC I think you'd be able to get far in mesa
> with a wrong num_pkrs.
>
> >
> > I saw that problem at bringup with an internal Mesa and had this fix
> > for it.
> >
> > Harry
> >
> > drivers/gpu/drm/amd/display/dc/basics/conversion.h | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/basics/conversion.h
> b/drivers/gpu/drm/amd/display/dc/basics/conversion.h
> > index ade785c4fdc7..da9883ec7b1c 100644
> > --- a/drivers/gpu/drm/amd/display/dc/basics/conversion.h
> > +++ b/drivers/gpu/drm/amd/display/dc/basics/conversion.h
> > @@ -40,7 +40,11 @@ void convert_float_matrix(
> >
> > static inline unsigned int log_2(unsigned int num)
> > {
> > - return ilog2(num);
> > + /*
> > + * Technically log(0) is undefined, but our code is structured
> > + * in a way that assumes log(0) = 0
> > + */
> > + return num ? ilog2(num) : num;
> > }
> >
> > #endif
> > --
> > 2.28.0
> >
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
[-- Attachment #1.2: Type: text/html, Size: 3245 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-21 20:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-20 22:31 [PATCH] drm/amd/display: Initialize num_pkrs on VANGOGH Bas Nieuwenhuizen
2020-10-20 23:47 ` Marek Olšák
2020-10-21 2:48 ` Huang Rui
2020-10-21 16:09 ` [PATCH] drm/amd/display: Interpret log_2(0) as 0 Harry Wentland
2020-10-21 16:39 ` Bas Nieuwenhuizen
2020-10-21 20:10 ` Marek Olšák
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox