[PATCH] drm/amdgpu: add dce6 drm_panic support

[PATCH] drm/amdgpu: add dce6 drm_panic support

[Lu Yao]

Add support for the drm_panic module, which displays a pretty user
friendly message on the screen when a Linux kernel panic occurs.

Signed-off-by: Lu Yao <yaolu@kylinos.cn>
---
The patch can work properly on the TTY, but after start X, drawn
image is messy, it looks like the data isn't linearly arranged.
However at this time 'fb->modifier' is 'DRM_FORMAT_MOD_LINEAR'.

Another difference I found is:
  For TTY, the amdgpu_bo is created with flag
  'AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED|AMDGPU_GEM_CREATE_CPU_GTT_USWC|
  AMDGPU_GEM_CREATE_VRAM_CLEARED|AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS'.
  For X, the amdgpu_bo is created with flag
  'AMDGPU_GEM_CREATE_NO_CPU_ACCESS|AMDGPU_GEM_CREATE_CPU_GTT_USWC'
I try to use same flag for X, it looks like no difference.

Can someone provide some insight into this problem or where I am going
wrong. Thanks a lot.

Test environment: X86 arch + v6.6 kernel + R7340.
---
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 32 +++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 05c0df97f01d..12c3801c264a 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -28,6 +28,8 @@
 #include <drm/drm_modeset_helper.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_panic.h>
+#include "../../drm_internal.h"
 
 #include "amdgpu.h"
 #include "amdgpu_pm.h"
@@ -2600,6 +2602,35 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
 	.get_scanout_position = amdgpu_crtc_get_scanout_position,
 };
 
+static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
+							 struct drm_scanout_buffer *sb)
+{
+	struct drm_framebuffer *fb;
+	struct drm_gem_object *obj;
+	struct amdgpu_bo *abo;
+	int ret = 0;
+
+	if (!plane->fb || plane->fb->modifier != DRM_FORMAT_MOD_LINEAR)
+		return -ENODEV;
+
+	fb = plane->fb;
+	sb->width = fb->width;
+	sb->height = fb->height;
+	sb->format = fb->format;
+	sb->pitch[0] = fb->pitches[0];
+
+	obj = fb->obj[0];
+	abo = gem_to_amdgpu_bo(obj);
+	if (!abo || abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
+		return -EINVAL;
+
+	return drm_gem_vmap(obj, &sb->map[0]);
+}
+
+static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
+	.get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
+};
+
 static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
@@ -2627,6 +2658,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
 	amdgpu_crtc->encoder = NULL;
 	amdgpu_crtc->connector = NULL;
 	drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
+	drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH] drm/amdgpu: add dce6 drm_panic support

[Christian König]

Am 02.08.24 um 09:17 schrieb Lu Yao:
> Add support for the drm_panic module, which displays a pretty user
> friendly message on the screen when a Linux kernel panic occurs.
>
> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> ---
> The patch can work properly on the TTY, but after start X, drawn
> image is messy, it looks like the data isn't linearly arranged.
> However at this time 'fb->modifier' is 'DRM_FORMAT_MOD_LINEAR'.
>
> Another difference I found is:
>    For TTY, the amdgpu_bo is created with flag
>    'AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED|AMDGPU_GEM_CREATE_CPU_GTT_USWC|
>    AMDGPU_GEM_CREATE_VRAM_CLEARED|AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS'.
>    For X, the amdgpu_bo is created with flag
>    'AMDGPU_GEM_CREATE_NO_CPU_ACCESS|AMDGPU_GEM_CREATE_CPU_GTT_USWC'
> I try to use same flag for X, it looks like no difference.
>
> Can someone provide some insight into this problem or where I am going
> wrong. Thanks a lot.
>
> Test environment: X86 arch + v6.6 kernel + R7340.
> ---
>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 32 +++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 05c0df97f01d..12c3801c264a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -28,6 +28,8 @@
>   #include <drm/drm_modeset_helper.h>
>   #include <drm/drm_modeset_helper_vtables.h>
>   #include <drm/drm_vblank.h>
> +#include <drm/drm_panic.h>

> +#include "../../drm_internal.h"

Well that this file is named "internal" and not in a common include 
directory is a strong indicator that you should absolutely *not* include 
it in a driver.

>   
>   #include "amdgpu.h"
>   #include "amdgpu_pm.h"
> @@ -2600,6 +2602,35 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
>   	.get_scanout_position = amdgpu_crtc_get_scanout_position,
>   };
>   
> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
> +							 struct drm_scanout_buffer *sb)
> +{
> +	struct drm_framebuffer *fb;
> +	struct drm_gem_object *obj;
> +	struct amdgpu_bo *abo;
> +	int ret = 0;
> +
> +	if (!plane->fb || plane->fb->modifier != DRM_FORMAT_MOD_LINEAR)
> +		return -ENODEV;
> +
> +	fb = plane->fb;
> +	sb->width = fb->width;
> +	sb->height = fb->height;
> +	sb->format = fb->format;
> +	sb->pitch[0] = fb->pitches[0];
> +
> +	obj = fb->obj[0];
> +	abo = gem_to_amdgpu_bo(obj);
> +	if (!abo || abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
> +		return -EINVAL;
> +
> +	return drm_gem_vmap(obj, &sb->map[0]);

Yeah that will almost always not work. Most display buffers are tilled 
and not CPU accessible.

Regards,
Christian.

> +}
> +
> +static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
> +	.get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
> +};
> +
>   static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>   {
>   	struct amdgpu_crtc *amdgpu_crtc;
> @@ -2627,6 +2658,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>   	amdgpu_crtc->encoder = NULL;
>   	amdgpu_crtc->connector = NULL;
>   	drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
> +	drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
>   
>   	return 0;
>   }

Re: [PATCH] drm/amdgpu: add dce6 drm_panic support

[yaolu]

在 2024/8/2 17:39, Christian König 写道:
> Am 02.08.24 um 09:17 schrieb Lu Yao:
>> Add support for the drm_panic module, which displays a pretty user
>> friendly message on the screen when a Linux kernel panic occurs.
>>
>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
>> ---
>> The patch can work properly on the TTY, but after start X, drawn
>> image is messy, it looks like the data isn't linearly arranged.
>> However at this time 'fb->modifier' is 'DRM_FORMAT_MOD_LINEAR'.
>>
>> Another difference I found is:
>>    For TTY, the amdgpu_bo is created with flag
>> 'AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED|AMDGPU_GEM_CREATE_CPU_GTT_USWC|
>> AMDGPU_GEM_CREATE_VRAM_CLEARED|AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS'.
>>    For X, the amdgpu_bo is created with flag
>> 'AMDGPU_GEM_CREATE_NO_CPU_ACCESS|AMDGPU_GEM_CREATE_CPU_GTT_USWC'
>> I try to use same flag for X, it looks like no difference.
>>
>> Can someone provide some insight into this problem or where I am going
>> wrong. Thanks a lot.
>>
>> Test environment: X86 arch + v6.6 kernel + R7340.
>> ---
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 32 +++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> index 05c0df97f01d..12c3801c264a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> @@ -28,6 +28,8 @@
>>   #include <drm/drm_modeset_helper.h>
>>   #include <drm/drm_modeset_helper_vtables.h>
>>   #include <drm/drm_vblank.h>
>> +#include <drm/drm_panic.h>
>
>> +#include "../../drm_internal.h"
>
> Well that this file is named "internal" and not in a common include 
> directory is a strong indicator that you should absolutely *not* 
> include it in a driver.
>
Okay, I'll fix it.
>>     #include "amdgpu.h"
>>   #include "amdgpu_pm.h"
>> @@ -2600,6 +2602,35 @@ static const struct drm_crtc_helper_funcs 
>> dce_v6_0_crtc_helper_funcs = {
>>       .get_scanout_position = amdgpu_crtc_get_scanout_position,
>>   };
>>   +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct 
>> drm_plane *plane,
>> +                             struct drm_scanout_buffer *sb)
>> +{
>> +    struct drm_framebuffer *fb;
>> +    struct drm_gem_object *obj;
>> +    struct amdgpu_bo *abo;
>> +    int ret = 0;
>> +
>> +    if (!plane->fb || plane->fb->modifier != DRM_FORMAT_MOD_LINEAR)
>> +        return -ENODEV;
>> +
>> +    fb = plane->fb;
>> +    sb->width = fb->width;
>> +    sb->height = fb->height;
>> +    sb->format = fb->format;
>> +    sb->pitch[0] = fb->pitches[0];
>> +
>> +    obj = fb->obj[0];
>> +    abo = gem_to_amdgpu_bo(obj);
>> +    if (!abo || abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
>> +        return -EINVAL;
>> +
>> +    return drm_gem_vmap(obj, &sb->map[0]);
>
> Yeah that will almost always not work. Most display buffers are tilled 
> and not CPU accessible.
>
> Regards,
> Christian.
>
I did some more tests. After removing the 
'AMDGPU_GEM_CREATE_NO_CPU_ACCESS' judgment here, then starting X, it 
worked well at '1280x960' resolution, but others (e.g. 1920x1080, 
640x480) not.
So, for this problem, it doesn't seem to matter 'Tiled memory' or 'CPU 
can't access'. Or is it just a coincidence ?

Thanks,
Lu Yao
>> +}
>> +
>> +static const struct drm_plane_helper_funcs 
>> dce_v6_0_drm_primary_plane_helper_funcs = {
>> +    .get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
>> +};
>> +
>>   static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>>   {
>>       struct amdgpu_crtc *amdgpu_crtc;
>> @@ -2627,6 +2658,7 @@ static int dce_v6_0_crtc_init(struct 
>> amdgpu_device *adev, int index)
>>       amdgpu_crtc->encoder = NULL;
>>       amdgpu_crtc->connector = NULL;
>>       drm_crtc_helper_add(&amdgpu_crtc->base, 
>> &dce_v6_0_crtc_helper_funcs);
>> +    drm_plane_helper_add(amdgpu_crtc->base.primary, 
>> &dce_v6_0_drm_primary_plane_helper_funcs);
>>         return 0;
>>   }
>

Re: [PATCH] drm/amdgpu: add dce6 drm_panic support

[Jocelyn Falempe]

On 02/08/2024 11:39, Christian König wrote:
> Am 02.08.24 um 09:17 schrieb Lu Yao:
>> Add support for the drm_panic module, which displays a pretty user
>> friendly message on the screen when a Linux kernel panic occurs.
>>
>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
>> ---
>> The patch can work properly on the TTY, but after start X, drawn
>> image is messy, it looks like the data isn't linearly arranged.
>> However at this time 'fb->modifier' is 'DRM_FORMAT_MOD_LINEAR'.
>>
>> Another difference I found is:
>>    For TTY, the amdgpu_bo is created with flag
>>    'AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED|AMDGPU_GEM_CREATE_CPU_GTT_USWC|
>>    AMDGPU_GEM_CREATE_VRAM_CLEARED|AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS'.
>>    For X, the amdgpu_bo is created with flag
>>    'AMDGPU_GEM_CREATE_NO_CPU_ACCESS|AMDGPU_GEM_CREATE_CPU_GTT_USWC'
>> I try to use same flag for X, it looks like no difference.
>>
>> Can someone provide some insight into this problem or where I am going
>> wrong. Thanks a lot.
>>
>> Test environment: X86 arch + v6.6 kernel + R7340.
>> ---
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 32 +++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> index 05c0df97f01d..12c3801c264a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> @@ -28,6 +28,8 @@
>>   #include <drm/drm_modeset_helper.h>
>>   #include <drm/drm_modeset_helper_vtables.h>
>>   #include <drm/drm_vblank.h>
>> +#include <drm/drm_panic.h>
> 
>> +#include "../../drm_internal.h"
> 
> Well that this file is named "internal" and not in a common include 
> directory is a strong indicator that you should absolutely *not* include 
> it in a driver.
> 
>>   #include "amdgpu.h"
>>   #include "amdgpu_pm.h"
>> @@ -2600,6 +2602,35 @@ static const struct drm_crtc_helper_funcs 
>> dce_v6_0_crtc_helper_funcs = {
>>       .get_scanout_position = amdgpu_crtc_get_scanout_position,
>>   };
>> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct 
>> drm_plane *plane,
>> +                             struct drm_scanout_buffer *sb)
>> +{
>> +    struct drm_framebuffer *fb;
>> +    struct drm_gem_object *obj;
>> +    struct amdgpu_bo *abo;
>> +    int ret = 0;
>> +
>> +    if (!plane->fb || plane->fb->modifier != DRM_FORMAT_MOD_LINEAR)
>> +        return -ENODEV;
>> +
>> +    fb = plane->fb;
>> +    sb->width = fb->width;
>> +    sb->height = fb->height;
>> +    sb->format = fb->format;
>> +    sb->pitch[0] = fb->pitches[0];
>> +
>> +    obj = fb->obj[0];
>> +    abo = gem_to_amdgpu_bo(obj);
>> +    if (!abo || abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
>> +        return -EINVAL;
>> +
>> +    return drm_gem_vmap(obj, &sb->map[0]);
> 
> Yeah that will almost always not work. Most display buffers are tilled 
> and not CPU accessible.

For the CPU accessible issue, Christian mentioned there was a debug 
interface on AMD GPU that can be used, to work around this:

https://lore.kernel.org/dri-devel/0baabe1f-8924-2c9a-5cd4-59084a37dbb2@gmail.com/ 
and 
https://lore.kernel.org/dri-devel/d233c376-ed07-2127-6084-8292d313dac7@amd.com/

And you will need to use the scanout_buffer->set_pixel() callback to 
write the pixels one by one, similar to what I've tried for nouveau with
https://patchwork.freedesktop.org/series/133963/

For the tiling format, the problem is that it is internal to the GPU, 
and currently the driver don't know which tiling format is being used.

It might be possible to disable tiling and compression, but it requires 
some internal DC knowledge:
https://lore.kernel.org/dri-devel/f76a3297-7d63-8615-45c5-47f02b64a1d5@amd.com/

Best regards,

-- 

Jocelyn



> 
> Regards,
> Christian.
> 
>> +}
>> +
>> +static const struct drm_plane_helper_funcs 
>> dce_v6_0_drm_primary_plane_helper_funcs = {
>> +    .get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
>> +};
>> +
>>   static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>>   {
>>       struct amdgpu_crtc *amdgpu_crtc;
>> @@ -2627,6 +2658,7 @@ static int dce_v6_0_crtc_init(struct 
>> amdgpu_device *adev, int index)
>>       amdgpu_crtc->encoder = NULL;
>>       amdgpu_crtc->connector = NULL;
>>       drm_crtc_helper_add(&amdgpu_crtc->base, 
>> &dce_v6_0_crtc_helper_funcs);
>> +    drm_plane_helper_add(amdgpu_crtc->base.primary, 
>> &dce_v6_0_drm_primary_plane_helper_funcs);
>>       return 0;
>>   }
> 

Re: [PATCH] drm/amdgpu: add dce6 drm_panic support

[Lu Yao]

On 2024/8/5 17:25, Jocelyn Falempe wrote:
>
>
> On 02/08/2024 11:39, Christian König wrote:
>> Am 02.08.24 um 09:17 schrieb Lu Yao:
>>> Add support for the drm_panic module, which displays a pretty user
>>> friendly message on the screen when a Linux kernel panic occurs.
>>>
>>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 32 
>>> +++++++++++++++++++++++++++
>>>   1 file changed, 32 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>>> index 05c0df97f01d..12c3801c264a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>>> @@ -28,6 +28,8 @@
>>>   #include <drm/drm_modeset_helper.h>
>>>   #include <drm/drm_modeset_helper_vtables.h>
>>>   #include <drm/drm_vblank.h>
>>> +#include <drm/drm_panic.h>
>>
>>> +#include "../../drm_internal.h"
>>
>> Well that this file is named "internal" and not in a common include 
>> directory is a strong indicator that you should absolutely *not* 
>> include it in a driver.
>>
>>>   #include "amdgpu.h"
>>>   #include "amdgpu_pm.h"
>>> @@ -2600,6 +2602,35 @@ static const struct drm_crtc_helper_funcs 
>>> dce_v6_0_crtc_helper_funcs = {
>>>       .get_scanout_position = amdgpu_crtc_get_scanout_position,
>>>   };
>>> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct 
>>> drm_plane *plane,
>>> +                             struct drm_scanout_buffer *sb)
>>> +{
>>> +    struct drm_framebuffer *fb;
>>> +    struct drm_gem_object *obj;
>>> +    struct amdgpu_bo *abo;
>>> +    int ret = 0;
>>> +
>>> +    if (!plane->fb || plane->fb->modifier != DRM_FORMAT_MOD_LINEAR)
>>> +        return -ENODEV;
>>> +
>>> +    fb = plane->fb;
>>> +    sb->width = fb->width;
>>> +    sb->height = fb->height;
>>> +    sb->format = fb->format;
>>> +    sb->pitch[0] = fb->pitches[0];
>>> +
>>> +    obj = fb->obj[0];
>>> +    abo = gem_to_amdgpu_bo(obj);
>>> +    if (!abo || abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
>>> +        return -EINVAL;
>>> +
>>> +    return drm_gem_vmap(obj, &sb->map[0]);
>>
>> Yeah that will almost always not work. Most display buffers are 
>> tilled and not CPU accessible.
>
> For the CPU accessible issue, Christian mentioned there was a debug 
> interface on AMD GPU that can be used, to work around this:
>
> https://lore.kernel.org/dri-devel/0baabe1f-8924-2c9a-5cd4-59084a37dbb2@gmail.com/ 
> and 
> https://lore.kernel.org/dri-devel/d233c376-ed07-2127-6084-8292d313dac7@amd.com/
>
> And you will need to use the scanout_buffer->set_pixel() callback to 
> write the pixels one by one, similar to what I've tried for nouveau with
> https://patchwork.freedesktop.org/series/133963/
>
> For the tiling format, the problem is that it is internal to the GPU, 
> and currently the driver don't know which tiling format is being used.
>
> It might be possible to disable tiling and compression, but it 
> requires some internal DC knowledge:
> https://lore.kernel.org/dri-devel/f76a3297-7d63-8615-45c5-47f02b64a1d5@amd.com/ 
>
>
> Best regards,

From the discussion provided, it is difficult to implement this feature without the relevant data book and knowledge.(Whether how tiled memory storage, or how to disable tiling of DC)

It looks like I'll just have to wait for AMD engineers to implement this.

Thanks a lot,
Lu Yao

Re: [PATCH] drm/amdgpu: add dce6 drm_panic support

[Alex Deucher]

On Thu, Aug 8, 2024 at 2:35 AM Lu Yao <yaolu@kylinos.cn> wrote:
>
> On 2024/8/5 17:25, Jocelyn Falempe wrote:
> >
> >
> > On 02/08/2024 11:39, Christian König wrote:
> >> Am 02.08.24 um 09:17 schrieb Lu Yao:
> >>> Add support for the drm_panic module, which displays a pretty user
> >>> friendly message on the screen when a Linux kernel panic occurs.
> >>>
> >>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> >>> ---
> >>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 32
> >>> +++++++++++++++++++++++++++
> >>>   1 file changed, 32 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> >>> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> >>> index 05c0df97f01d..12c3801c264a 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> >>> @@ -28,6 +28,8 @@
> >>>   #include <drm/drm_modeset_helper.h>
> >>>   #include <drm/drm_modeset_helper_vtables.h>
> >>>   #include <drm/drm_vblank.h>
> >>> +#include <drm/drm_panic.h>
> >>
> >>> +#include "../../drm_internal.h"
> >>
> >> Well that this file is named "internal" and not in a common include
> >> directory is a strong indicator that you should absolutely *not*
> >> include it in a driver.
> >>
> >>>   #include "amdgpu.h"
> >>>   #include "amdgpu_pm.h"
> >>> @@ -2600,6 +2602,35 @@ static const struct drm_crtc_helper_funcs
> >>> dce_v6_0_crtc_helper_funcs = {
> >>>       .get_scanout_position = amdgpu_crtc_get_scanout_position,
> >>>   };
> >>> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct
> >>> drm_plane *plane,
> >>> +                             struct drm_scanout_buffer *sb)
> >>> +{
> >>> +    struct drm_framebuffer *fb;
> >>> +    struct drm_gem_object *obj;
> >>> +    struct amdgpu_bo *abo;
> >>> +    int ret = 0;
> >>> +
> >>> +    if (!plane->fb || plane->fb->modifier != DRM_FORMAT_MOD_LINEAR)
> >>> +        return -ENODEV;
> >>> +
> >>> +    fb = plane->fb;
> >>> +    sb->width = fb->width;
> >>> +    sb->height = fb->height;
> >>> +    sb->format = fb->format;
> >>> +    sb->pitch[0] = fb->pitches[0];
> >>> +
> >>> +    obj = fb->obj[0];
> >>> +    abo = gem_to_amdgpu_bo(obj);
> >>> +    if (!abo || abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
> >>> +        return -EINVAL;
> >>> +
> >>> +    return drm_gem_vmap(obj, &sb->map[0]);
> >>
> >> Yeah that will almost always not work. Most display buffers are
> >> tilled and not CPU accessible.
> >
> > For the CPU accessible issue, Christian mentioned there was a debug
> > interface on AMD GPU that can be used, to work around this:
> >
> > https://lore.kernel.org/dri-devel/0baabe1f-8924-2c9a-5cd4-59084a37dbb2@gmail.com/
> > and
> > https://lore.kernel.org/dri-devel/d233c376-ed07-2127-6084-8292d313dac7@amd.com/
> >
> > And you will need to use the scanout_buffer->set_pixel() callback to
> > write the pixels one by one, similar to what I've tried for nouveau with
> > https://patchwork.freedesktop.org/series/133963/
> >
> > For the tiling format, the problem is that it is internal to the GPU,
> > and currently the driver don't know which tiling format is being used.
> >
> > It might be possible to disable tiling and compression, but it
> > requires some internal DC knowledge:
> > https://lore.kernel.org/dri-devel/f76a3297-7d63-8615-45c5-47f02b64a1d5@amd.com/
> >
> >
> > Best regards,
>
> From the discussion provided, it is difficult to implement this feature without the relevant data book and knowledge.(Whether how tiled memory storage, or how to disable tiling of DC)

For DCE 6, the GRPH_ARRAY_MODE field in mmGRPH_CONTROL controls the
display tiling.  Set that field to GRPH_ARRAY_LINEAR_GENERAL (0) to
disable tiling.

Alex

Re: [PATCH] drm/amdgpu: add dce6 drm_panic support

[Alex Deucher]

On Thu, Aug 8, 2024 at 12:43 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Aug 8, 2024 at 2:35 AM Lu Yao <yaolu@kylinos.cn> wrote:
> >
> > On 2024/8/5 17:25, Jocelyn Falempe wrote:
> > >
> > >
> > > On 02/08/2024 11:39, Christian König wrote:
> > >> Am 02.08.24 um 09:17 schrieb Lu Yao:
> > >>> Add support for the drm_panic module, which displays a pretty user
> > >>> friendly message on the screen when a Linux kernel panic occurs.
> > >>>
> > >>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> > >>> ---
> > >>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 32
> > >>> +++++++++++++++++++++++++++
> > >>>   1 file changed, 32 insertions(+)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > >>> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > >>> index 05c0df97f01d..12c3801c264a 100644
> > >>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > >>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > >>> @@ -28,6 +28,8 @@
> > >>>   #include <drm/drm_modeset_helper.h>
> > >>>   #include <drm/drm_modeset_helper_vtables.h>
> > >>>   #include <drm/drm_vblank.h>
> > >>> +#include <drm/drm_panic.h>
> > >>
> > >>> +#include "../../drm_internal.h"
> > >>
> > >> Well that this file is named "internal" and not in a common include
> > >> directory is a strong indicator that you should absolutely *not*
> > >> include it in a driver.
> > >>
> > >>>   #include "amdgpu.h"
> > >>>   #include "amdgpu_pm.h"
> > >>> @@ -2600,6 +2602,35 @@ static const struct drm_crtc_helper_funcs
> > >>> dce_v6_0_crtc_helper_funcs = {
> > >>>       .get_scanout_position = amdgpu_crtc_get_scanout_position,
> > >>>   };
> > >>> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct
> > >>> drm_plane *plane,
> > >>> +                             struct drm_scanout_buffer *sb)
> > >>> +{
> > >>> +    struct drm_framebuffer *fb;
> > >>> +    struct drm_gem_object *obj;
> > >>> +    struct amdgpu_bo *abo;
> > >>> +    int ret = 0;
> > >>> +
> > >>> +    if (!plane->fb || plane->fb->modifier != DRM_FORMAT_MOD_LINEAR)
> > >>> +        return -ENODEV;
> > >>> +
> > >>> +    fb = plane->fb;
> > >>> +    sb->width = fb->width;
> > >>> +    sb->height = fb->height;
> > >>> +    sb->format = fb->format;
> > >>> +    sb->pitch[0] = fb->pitches[0];
> > >>> +
> > >>> +    obj = fb->obj[0];
> > >>> +    abo = gem_to_amdgpu_bo(obj);
> > >>> +    if (!abo || abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
> > >>> +        return -EINVAL;
> > >>> +
> > >>> +    return drm_gem_vmap(obj, &sb->map[0]);
> > >>
> > >> Yeah that will almost always not work. Most display buffers are
> > >> tilled and not CPU accessible.
> > >
> > > For the CPU accessible issue, Christian mentioned there was a debug
> > > interface on AMD GPU that can be used, to work around this:
> > >
> > > https://lore.kernel.org/dri-devel/0baabe1f-8924-2c9a-5cd4-59084a37dbb2@gmail.com/
> > > and
> > > https://lore.kernel.org/dri-devel/d233c376-ed07-2127-6084-8292d313dac7@amd.com/
> > >
> > > And you will need to use the scanout_buffer->set_pixel() callback to
> > > write the pixels one by one, similar to what I've tried for nouveau with
> > > https://patchwork.freedesktop.org/series/133963/
> > >
> > > For the tiling format, the problem is that it is internal to the GPU,
> > > and currently the driver don't know which tiling format is being used.
> > >
> > > It might be possible to disable tiling and compression, but it
> > > requires some internal DC knowledge:
> > > https://lore.kernel.org/dri-devel/f76a3297-7d63-8615-45c5-47f02b64a1d5@amd.com/
> > >
> > >
> > > Best regards,
> >
> > From the discussion provided, it is difficult to implement this feature without the relevant data book and knowledge.(Whether how tiled memory storage, or how to disable tiling of DC)
>
> For DCE 6, the GRPH_ARRAY_MODE field in mmGRPH_CONTROL controls the
> display tiling.  Set that field to GRPH_ARRAY_LINEAR_GENERAL (0) to
> disable tiling.

For clarity that register is instanced so use mmGRPH_CONTROL +
amdgpu_crtc->crtc_offset to get the right instance.

Alex

>
> Alex

Re: [PATCH] drm/amdgpu: add dce6 drm_panic support

[Lu Yao]

On Thu, Aug 8, 2024 at 13:24 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Aug 8, 2024 at 12:43 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > On Thu, Aug 8, 2024 at 2:35 AM Lu Yao <yaolu@kylinos.cn> wrote:
> > >
> > > On 2024/8/5 17:25, Jocelyn Falempe wrote:
> > > >
> > > >
> > > > On 02/08/2024 11:39, Christian König wrote:
> > > >> Am 02.08.24 um 09:17 schrieb Lu Yao:
> > > >>> Add support for the drm_panic module, which displays a pretty user
> > > >>> friendly message on the screen when a Linux kernel panic occurs.
> > > >>>
> > > >>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> > > >>> ---
> > > >>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 32
> > > >>> +++++++++++++++++++++++++++
> > > >>>   1 file changed, 32 insertions(+)
> > > >>>
> > > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > >>> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > >>> index 05c0df97f01d..12c3801c264a 100644
> > > >>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > >>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > >>> @@ -28,6 +28,8 @@
> > > >>>   #include <drm/drm_modeset_helper.h>
> > > >>>   #include <drm/drm_modeset_helper_vtables.h>
> > > >>>   #include <drm/drm_vblank.h>
> > > >>> +#include <drm/drm_panic.h>
> > > >>
> > > >>> +#include "../../drm_internal.h"
> > > >>
> > > >> Well that this file is named "internal" and not in a common include
> > > >> directory is a strong indicator that you should absolutely *not*
> > > >> include it in a driver.
> > > >>
> > > >>>   #include "amdgpu.h"
> > > >>>   #include "amdgpu_pm.h"
> > > >>> @@ -2600,6 +2602,35 @@ static const struct drm_crtc_helper_funcs
> > > >>> dce_v6_0_crtc_helper_funcs = {
> > > >>>       .get_scanout_position = amdgpu_crtc_get_scanout_position,
> > > >>>   };
> > > >>> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct
> > > >>> drm_plane *plane,
> > > >>> +                             struct drm_scanout_buffer *sb)
> > > >>> +{
> > > >>> +    struct drm_framebuffer *fb;
> > > >>> +    struct drm_gem_object *obj;
> > > >>> +    struct amdgpu_bo *abo;
> > > >>> +    int ret = 0;
> > > >>> +
> > > >>> +    if (!plane->fb || plane->fb->modifier != DRM_FORMAT_MOD_LINEAR)
> > > >>> +        return -ENODEV;
> > > >>> +
> > > >>> +    fb = plane->fb;
> > > >>> +    sb->width = fb->width;
> > > >>> +    sb->height = fb->height;
> > > >>> +    sb->format = fb->format;
> > > >>> +    sb->pitch[0] = fb->pitches[0];
> > > >>> +
> > > >>> +    obj = fb->obj[0];
> > > >>> +    abo = gem_to_amdgpu_bo(obj);
> > > >>> +    if (!abo || abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
> > > >>> +        return -EINVAL;
> > > >>> +
> > > >>> +    return drm_gem_vmap(obj, &sb->map[0]);
> > > >>
> > > >> Yeah that will almost always not work. Most display buffers are
> > > >> tilled and not CPU accessible.
> > > >
> > > > For the CPU accessible issue, Christian mentioned there was a debug
> > > > interface on AMD GPU that can be used, to work around this:
> > > >
> > > > https://lore.kernel.org/dri-devel/0baabe1f-8924-2c9a-5cd4-59084a37dbb2@gmail.com/
> > > > and
> > > > https://lore.kernel.org/dri-devel/d233c376-ed07-2127-6084-8292d313dac7@amd.com/
> > > >
> > > > And you will need to use the scanout_buffer->set_pixel() callback to
> > > > write the pixels one by one, similar to what I've tried for nouveau with
> > > > https://patchwork.freedesktop.org/series/133963/
> > > >
> > > > For the tiling format, the problem is that it is internal to the GPU,
> > > > and currently the driver don't know which tiling format is being used.
> > > >
> > > > It might be possible to disable tiling and compression, but it
> > > > requires some internal DC knowledge:
> > > > https://lore.kernel.org/dri-devel/f76a3297-7d63-8615-45c5-47f02b64a1d5@amd.com/
> > > >
> > > >
> > > > Best regards,
> > >
> > > From the discussion provided, it is difficult to implement this feature without the relevant data book and knowledge.(Whether how tiled memory storage, or how to disable tiling of DC)
> >
> > For DCE 6, the GRPH_ARRAY_MODE field in mmGRPH_CONTROL controls the
> > display tiling.  Set that field to GRPH_ARRAY_LINEAR_GENERAL (0) to
> > disable tiling.
> 
> For clarity that register is instanced so use mmGRPH_CONTROL +
> amdgpu_crtc->crtc_offset to get the right instance.
> 

It works, I'll submit a new patch soon.

Thanks,
Lu Yao

> Alex

[PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Lu Yao]

Add support for the drm_panic module, which displays a pretty user
friendly message on the screen when a Linux kernel panic occurs.

Signed-off-by: Lu Yao <yaolu@kylinos.cn>
---
Changes in v2: 
1. Drop include "drm_internal.h"
2. Add disabling DC tiling ops.
Per suggestion from previous thread:
https://patchwork.freedesktop.org/patch/606879/?series=136832&rev=1
---
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 48 +++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 05c0df97f01d..ba1b7a36caa3 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -28,6 +28,7 @@
 #include <drm/drm_modeset_helper.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_panic.h>
 
 #include "amdgpu.h"
 #include "amdgpu_pm.h"
@@ -2600,6 +2601,52 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
 	.get_scanout_position = amdgpu_crtc_get_scanout_position,
 };
 
+static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
+							 struct drm_scanout_buffer *sb)
+{
+	struct drm_framebuffer *fb;
+	struct amdgpu_bo *abo;
+	struct amdgpu_crtc *amdgpu_crtc;
+	struct amdgpu_device *adev;
+	uint32_t fb_format;
+
+	if (!plane->fb)
+		return -EINVAL;
+
+	fb = plane->fb;
+
+	abo = gem_to_amdgpu_bo(fb->obj[0]);
+	amdgpu_crtc = to_amdgpu_crtc(plane->crtc);
+	adev = drm_to_adev(fb->dev);
+
+	if (!abo->kmap.virtual &&
+	    ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size), &abo->kmap)) {
+		DRM_WARN("amdgpu bo map failed, panic won't be displayed\n");
+		return -ENOMEM;
+	}
+
+	if (abo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK)
+		iosys_map_set_vaddr_iomem(&sb->map[0], abo->kmap.virtual);
+	else
+		iosys_map_set_vaddr(&sb->map[0], abo->kmap.virtual);
+
+	sb->width = fb->width;
+	sb->height = fb->height;
+	sb->format = fb->format;
+	sb->pitch[0] = fb->pitches[0];
+
+	/* Disable DC tiling */
+	fb_format = RREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset);
+	fb_format &= ~GRPH_ARRAY_MODE(0x7);
+	WREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset, fb_format);
+
+	return 0;
+}
+
+static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
+	.get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
+};
+
 static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
 {
 	struct amdgpu_crtc *amdgpu_crtc;
@@ -2627,6 +2674,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
 	amdgpu_crtc->encoder = NULL;
 	amdgpu_crtc->connector = NULL;
 	drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
+	drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Jocelyn Falempe]

On 12/08/2024 08:09, Lu Yao wrote:
> Add support for the drm_panic module, which displays a pretty user
> friendly message on the screen when a Linux kernel panic occurs.

 From a drm_panic point of view, it looks good to me.
I don't have the required hardware, so unfortunately I wasn't able to 
test it.

Best regards,

-- 

Jocelyn


> 
> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> ---
> Changes in v2:
> 1. Drop include "drm_internal.h"
> 2. Add disabling DC tiling ops.
> Per suggestion from previous thread:
> https://patchwork.freedesktop.org/patch/606879/?series=136832&rev=1
> ---
>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 48 +++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 05c0df97f01d..ba1b7a36caa3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -28,6 +28,7 @@
>   #include <drm/drm_modeset_helper.h>
>   #include <drm/drm_modeset_helper_vtables.h>
>   #include <drm/drm_vblank.h>
> +#include <drm/drm_panic.h>
>   
>   #include "amdgpu.h"
>   #include "amdgpu_pm.h"
> @@ -2600,6 +2601,52 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
>   	.get_scanout_position = amdgpu_crtc_get_scanout_position,
>   };
>   
> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
> +							 struct drm_scanout_buffer *sb)
> +{
> +	struct drm_framebuffer *fb;
> +	struct amdgpu_bo *abo;
> +	struct amdgpu_crtc *amdgpu_crtc;
> +	struct amdgpu_device *adev;
> +	uint32_t fb_format;
> +
> +	if (!plane->fb)
> +		return -EINVAL;
> +
> +	fb = plane->fb;
> +
> +	abo = gem_to_amdgpu_bo(fb->obj[0]);
> +	amdgpu_crtc = to_amdgpu_crtc(plane->crtc);
> +	adev = drm_to_adev(fb->dev);
> +
> +	if (!abo->kmap.virtual &&
> +	    ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size), &abo->kmap)) {
> +		DRM_WARN("amdgpu bo map failed, panic won't be displayed\n");
> +		return -ENOMEM;
> +	}
> +
> +	if (abo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK)
> +		iosys_map_set_vaddr_iomem(&sb->map[0], abo->kmap.virtual);
> +	else
> +		iosys_map_set_vaddr(&sb->map[0], abo->kmap.virtual);
> +
> +	sb->width = fb->width;
> +	sb->height = fb->height;
> +	sb->format = fb->format;
> +	sb->pitch[0] = fb->pitches[0];
> +
> +	/* Disable DC tiling */
> +	fb_format = RREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset);
> +	fb_format &= ~GRPH_ARRAY_MODE(0x7);
> +	WREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset, fb_format);
> +
> +	return 0;
> +}
> +
> +static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
> +	.get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
> +};
> +
>   static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>   {
>   	struct amdgpu_crtc *amdgpu_crtc;
> @@ -2627,6 +2674,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>   	amdgpu_crtc->encoder = NULL;
>   	amdgpu_crtc->connector = NULL;
>   	drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
> +	drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
>   
>   	return 0;
>   }

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Alex Deucher]

On Mon, Aug 12, 2024 at 2:10 AM Lu Yao <yaolu@kylinos.cn> wrote:
>
> Add support for the drm_panic module, which displays a pretty user
> friendly message on the screen when a Linux kernel panic occurs.
>
> Signed-off-by: Lu Yao <yaolu@kylinos.cn>

Patch looks good to me.  Any chance you want to convert the other
non-DC dce files (dce_v8_0.c, dce_v10_0.c, dce_v11_0.c) while you are
at it?

Alex


> ---
> Changes in v2:
> 1. Drop include "drm_internal.h"
> 2. Add disabling DC tiling ops.
> Per suggestion from previous thread:
> https://patchwork.freedesktop.org/patch/606879/?series=136832&rev=1
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 48 +++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 05c0df97f01d..ba1b7a36caa3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -28,6 +28,7 @@
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_modeset_helper_vtables.h>
>  #include <drm/drm_vblank.h>
> +#include <drm/drm_panic.h>
>
>  #include "amdgpu.h"
>  #include "amdgpu_pm.h"
> @@ -2600,6 +2601,52 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
>         .get_scanout_position = amdgpu_crtc_get_scanout_position,
>  };
>
> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
> +                                                        struct drm_scanout_buffer *sb)
> +{
> +       struct drm_framebuffer *fb;
> +       struct amdgpu_bo *abo;
> +       struct amdgpu_crtc *amdgpu_crtc;
> +       struct amdgpu_device *adev;
> +       uint32_t fb_format;
> +
> +       if (!plane->fb)
> +               return -EINVAL;
> +
> +       fb = plane->fb;
> +
> +       abo = gem_to_amdgpu_bo(fb->obj[0]);
> +       amdgpu_crtc = to_amdgpu_crtc(plane->crtc);
> +       adev = drm_to_adev(fb->dev);
> +
> +       if (!abo->kmap.virtual &&
> +           ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size), &abo->kmap)) {
> +               DRM_WARN("amdgpu bo map failed, panic won't be displayed\n");
> +               return -ENOMEM;
> +       }
> +
> +       if (abo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK)
> +               iosys_map_set_vaddr_iomem(&sb->map[0], abo->kmap.virtual);
> +       else
> +               iosys_map_set_vaddr(&sb->map[0], abo->kmap.virtual);
> +
> +       sb->width = fb->width;
> +       sb->height = fb->height;
> +       sb->format = fb->format;
> +       sb->pitch[0] = fb->pitches[0];
> +
> +       /* Disable DC tiling */
> +       fb_format = RREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset);
> +       fb_format &= ~GRPH_ARRAY_MODE(0x7);
> +       WREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset, fb_format);
> +
> +       return 0;
> +}
> +
> +static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
> +       .get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
> +};
> +
>  static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> @@ -2627,6 +2674,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>         amdgpu_crtc->encoder = NULL;
>         amdgpu_crtc->connector = NULL;
>         drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
> +       drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
>
>         return 0;
>  }
> --
> 2.25.1
>

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Lu Yao]

On 2024/9/17 21:21, Alex Deucher wrote:
> On Mon, Aug 12, 2024 at 2:10 AM Lu Yao <yaolu@kylinos.cn> wrote:
>> Add support for the drm_panic module, which displays a pretty user
>> friendly message on the screen when a Linux kernel panic occurs.
>>
>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> Patch looks good to me.  Any chance you want to convert the other
> non-DC dce files (dce_v8_0.c, dce_v10_0.c, dce_v11_0.c) while you are
> at it?
>
> Alex
>
I don't have any hardware with those dc ip blocks.
Although the code logic looks consistent, but it still needs to be 
tested, right?  ; )

Lu Yao
> ---
> Changes in v2:
> 1. Drop include "drm_internal.h"
> 2. Add disabling DC tiling ops.
> Per suggestion from previous thread:
> https://patchwork.freedesktop.org/patch/606879/?series=136832&rev=1
> ---
>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 48 +++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
> --
> 2.25.1
>

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Jocelyn Falempe]

On 17/09/2024 15:21, Alex Deucher wrote:
> On Mon, Aug 12, 2024 at 2:10 AM Lu Yao <yaolu@kylinos.cn> wrote:
>>
>> Add support for the drm_panic module, which displays a pretty user
>> friendly message on the screen when a Linux kernel panic occurs.
>>
>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> 
> Patch looks good to me.  Any chance you want to convert the other
> non-DC dce files (dce_v8_0.c, dce_v10_0.c, dce_v11_0.c) while you are
> at it?

I've made a similar patch in amdgpu_dm_plane.c, and it works on a Radeon 
pro w6400.
But it only works when I'm in a VT terminal (so the framebuffer is 
linear and CPU accessible).
When under Gnome/Wayland, the flag AMDGPU_GEM_CREATE_NO_CPU_ACCESS is 
set, so that means I can't vmap it ?

Also I don't know if there is a similar way to disable 
tiling/compression on this hardware.

Best regards,

-- 

Jocelyn


> 
> Alex
> 
> 
>> ---
>> Changes in v2:
>> 1. Drop include "drm_internal.h"
>> 2. Add disabling DC tiling ops.
>> Per suggestion from previous thread:
>> https://patchwork.freedesktop.org/patch/606879/?series=136832&rev=1
>> ---
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 48 +++++++++++++++++++++++++++
>>   1 file changed, 48 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> index 05c0df97f01d..ba1b7a36caa3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> @@ -28,6 +28,7 @@
>>   #include <drm/drm_modeset_helper.h>
>>   #include <drm/drm_modeset_helper_vtables.h>
>>   #include <drm/drm_vblank.h>
>> +#include <drm/drm_panic.h>
>>
>>   #include "amdgpu.h"
>>   #include "amdgpu_pm.h"
>> @@ -2600,6 +2601,52 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
>>          .get_scanout_position = amdgpu_crtc_get_scanout_position,
>>   };
>>
>> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
>> +                                                        struct drm_scanout_buffer *sb)
>> +{
>> +       struct drm_framebuffer *fb;
>> +       struct amdgpu_bo *abo;
>> +       struct amdgpu_crtc *amdgpu_crtc;
>> +       struct amdgpu_device *adev;
>> +       uint32_t fb_format;
>> +
>> +       if (!plane->fb)
>> +               return -EINVAL;
>> +
>> +       fb = plane->fb;
>> +
>> +       abo = gem_to_amdgpu_bo(fb->obj[0]);
>> +       amdgpu_crtc = to_amdgpu_crtc(plane->crtc);
>> +       adev = drm_to_adev(fb->dev);
>> +
>> +       if (!abo->kmap.virtual &&
>> +           ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size), &abo->kmap)) {
>> +               DRM_WARN("amdgpu bo map failed, panic won't be displayed\n");
>> +               return -ENOMEM;
>> +       }
>> +
>> +       if (abo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK)
>> +               iosys_map_set_vaddr_iomem(&sb->map[0], abo->kmap.virtual);
>> +       else
>> +               iosys_map_set_vaddr(&sb->map[0], abo->kmap.virtual);
>> +
>> +       sb->width = fb->width;
>> +       sb->height = fb->height;
>> +       sb->format = fb->format;
>> +       sb->pitch[0] = fb->pitches[0];
>> +
>> +       /* Disable DC tiling */
>> +       fb_format = RREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset);
>> +       fb_format &= ~GRPH_ARRAY_MODE(0x7);
>> +       WREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset, fb_format);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
>> +       .get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
>> +};
>> +
>>   static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>>   {
>>          struct amdgpu_crtc *amdgpu_crtc;
>> @@ -2627,6 +2674,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>>          amdgpu_crtc->encoder = NULL;
>>          amdgpu_crtc->connector = NULL;
>>          drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
>> +       drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
>>
>>          return 0;
>>   }
>> --
>> 2.25.1
>>
> 

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Alex Deucher]

On Fri, Sep 20, 2024 at 11:36 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>
> On 17/09/2024 15:21, Alex Deucher wrote:
> > On Mon, Aug 12, 2024 at 2:10 AM Lu Yao <yaolu@kylinos.cn> wrote:
> >>
> >> Add support for the drm_panic module, which displays a pretty user
> >> friendly message on the screen when a Linux kernel panic occurs.
> >>
> >> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> >
> > Patch looks good to me.  Any chance you want to convert the other
> > non-DC dce files (dce_v8_0.c, dce_v10_0.c, dce_v11_0.c) while you are
> > at it?
>
> I've made a similar patch in amdgpu_dm_plane.c, and it works on a Radeon
> pro w6400.
> But it only works when I'm in a VT terminal (so the framebuffer is
> linear and CPU accessible).
> When under Gnome/Wayland, the flag AMDGPU_GEM_CREATE_NO_CPU_ACCESS is
> set, so that means I can't vmap it ?

It just means that the application does not need CPU access.  Whether
or not the CPU can access the buffer or not depends on the size of the
PCI BAR.  E.g., if the driver or bios has resized the PCI BAR, then
the CPU can access the entire BAR, but if not you are generally
limited to the first 256M of framebuffer.

>
> Also I don't know if there is a similar way to disable
> tiling/compression on this hardware.

UNP_GRPH_CONTROL on chips with DCE display hardware and
DCSURF_ADDR_CONFIG and DCSURF_TILING_CONFIG on DCN display hardware.

Alex

>
> Best regards,
>
> --
>
> Jocelyn
>
>
> >
> > Alex
> >
> >
> >> ---
> >> Changes in v2:
> >> 1. Drop include "drm_internal.h"
> >> 2. Add disabling DC tiling ops.
> >> Per suggestion from previous thread:
> >> https://patchwork.freedesktop.org/patch/606879/?series=136832&rev=1
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 48 +++++++++++++++++++++++++++
> >>   1 file changed, 48 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> >> index 05c0df97f01d..ba1b7a36caa3 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> >> @@ -28,6 +28,7 @@
> >>   #include <drm/drm_modeset_helper.h>
> >>   #include <drm/drm_modeset_helper_vtables.h>
> >>   #include <drm/drm_vblank.h>
> >> +#include <drm/drm_panic.h>
> >>
> >>   #include "amdgpu.h"
> >>   #include "amdgpu_pm.h"
> >> @@ -2600,6 +2601,52 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
> >>          .get_scanout_position = amdgpu_crtc_get_scanout_position,
> >>   };
> >>
> >> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
> >> +                                                        struct drm_scanout_buffer *sb)
> >> +{
> >> +       struct drm_framebuffer *fb;
> >> +       struct amdgpu_bo *abo;
> >> +       struct amdgpu_crtc *amdgpu_crtc;
> >> +       struct amdgpu_device *adev;
> >> +       uint32_t fb_format;
> >> +
> >> +       if (!plane->fb)
> >> +               return -EINVAL;
> >> +
> >> +       fb = plane->fb;
> >> +
> >> +       abo = gem_to_amdgpu_bo(fb->obj[0]);
> >> +       amdgpu_crtc = to_amdgpu_crtc(plane->crtc);
> >> +       adev = drm_to_adev(fb->dev);
> >> +
> >> +       if (!abo->kmap.virtual &&
> >> +           ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size), &abo->kmap)) {
> >> +               DRM_WARN("amdgpu bo map failed, panic won't be displayed\n");
> >> +               return -ENOMEM;
> >> +       }
> >> +
> >> +       if (abo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK)
> >> +               iosys_map_set_vaddr_iomem(&sb->map[0], abo->kmap.virtual);
> >> +       else
> >> +               iosys_map_set_vaddr(&sb->map[0], abo->kmap.virtual);
> >> +
> >> +       sb->width = fb->width;
> >> +       sb->height = fb->height;
> >> +       sb->format = fb->format;
> >> +       sb->pitch[0] = fb->pitches[0];
> >> +
> >> +       /* Disable DC tiling */
> >> +       fb_format = RREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset);
> >> +       fb_format &= ~GRPH_ARRAY_MODE(0x7);
> >> +       WREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset, fb_format);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
> >> +       .get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
> >> +};
> >> +
> >>   static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
> >>   {
> >>          struct amdgpu_crtc *amdgpu_crtc;
> >> @@ -2627,6 +2674,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
> >>          amdgpu_crtc->encoder = NULL;
> >>          amdgpu_crtc->connector = NULL;
> >>          drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
> >> +       drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
> >>
> >>          return 0;
> >>   }
> >> --
> >> 2.25.1
> >>
> >
>

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Michel Dänzer]

On 2024-09-24 16:02, Alex Deucher wrote:
> On Fri, Sep 20, 2024 at 11:36 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>> On 17/09/2024 15:21, Alex Deucher wrote:
>>> On Mon, Aug 12, 2024 at 2:10 AM Lu Yao <yaolu@kylinos.cn> wrote:
>>>>
>>>> Add support for the drm_panic module, which displays a pretty user
>>>> friendly message on the screen when a Linux kernel panic occurs.
>>>>
>>>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
>>>
>>> Patch looks good to me.  Any chance you want to convert the other
>>> non-DC dce files (dce_v8_0.c, dce_v10_0.c, dce_v11_0.c) while you are
>>> at it?
>>
>> I've made a similar patch in amdgpu_dm_plane.c, and it works on a Radeon
>> pro w6400.
>> But it only works when I'm in a VT terminal (so the framebuffer is
>> linear and CPU accessible).
>> When under Gnome/Wayland, the flag AMDGPU_GEM_CREATE_NO_CPU_ACCESS is
>> set, so that means I can't vmap it ?
> 
> It just means that the application does not need CPU access.  Whether
> or not the CPU can access the buffer or not depends on the size of the
> PCI BAR.  E.g., if the driver or bios has resized the PCI BAR, then
> the CPU can access the entire BAR, but if not you are generally
> limited to the first 256M of framebuffer.

FWIW, it's also possible to access all of VRAM via MMIO indirect registers. That'll be slower than a direct CPU map, it might be acceptable for drm_panic though, at least as a fallback.


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Jocelyn Falempe]

On 24/09/2024 16:02, Alex Deucher wrote:
> On Fri, Sep 20, 2024 at 11:36 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>
>> On 17/09/2024 15:21, Alex Deucher wrote:
>>> On Mon, Aug 12, 2024 at 2:10 AM Lu Yao <yaolu@kylinos.cn> wrote:
>>>>
>>>> Add support for the drm_panic module, which displays a pretty user
>>>> friendly message on the screen when a Linux kernel panic occurs.
>>>>
>>>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
>>>
>>> Patch looks good to me.  Any chance you want to convert the other
>>> non-DC dce files (dce_v8_0.c, dce_v10_0.c, dce_v11_0.c) while you are
>>> at it?
>>
>> I've made a similar patch in amdgpu_dm_plane.c, and it works on a Radeon
>> pro w6400.
>> But it only works when I'm in a VT terminal (so the framebuffer is
>> linear and CPU accessible).
>> When under Gnome/Wayland, the flag AMDGPU_GEM_CREATE_NO_CPU_ACCESS is
>> set, so that means I can't vmap it ?
> 
> It just means that the application does not need CPU access.  Whether
> or not the CPU can access the buffer or not depends on the size of the
> PCI BAR.  E.g., if the driver or bios has resized the PCI BAR, then
> the CPU can access the entire BAR, but if not you are generally
> limited to the first 256M of framebuffer.

I tried to use ttm_bo_kmap() anyway, it returns a valid virtual address, 
but writing to it has no effect on the display.
> 
>>
>> Also I don't know if there is a similar way to disable
>> tiling/compression on this hardware.
> 
> UNP_GRPH_CONTROL on chips with DCE display hardware and
> DCSURF_ADDR_CONFIG and DCSURF_TILING_CONFIG on DCN display hardware.

Thanks a lot, I will see if I can make this work.
For DCN, it depends on the HUBP version, and I need a pipe_ctx to access 
it. I didn't find how to get a pipe_ctx from the current primary plane.

Best regards,

-- 

Jocelyn

> 
> Alex
> 
>>
>> Best regards,
>>
>> --
>>
>> Jocelyn
>>
>>
>>>
>>> Alex
>>>
>>>
>>>> ---
>>>> Changes in v2:
>>>> 1. Drop include "drm_internal.h"
>>>> 2. Add disabling DC tiling ops.
>>>> Per suggestion from previous thread:
>>>> https://patchwork.freedesktop.org/patch/606879/?series=136832&rev=1
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 48 +++++++++++++++++++++++++++
>>>>    1 file changed, 48 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>>>> index 05c0df97f01d..ba1b7a36caa3 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>>>> @@ -28,6 +28,7 @@
>>>>    #include <drm/drm_modeset_helper.h>
>>>>    #include <drm/drm_modeset_helper_vtables.h>
>>>>    #include <drm/drm_vblank.h>
>>>> +#include <drm/drm_panic.h>
>>>>
>>>>    #include "amdgpu.h"
>>>>    #include "amdgpu_pm.h"
>>>> @@ -2600,6 +2601,52 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
>>>>           .get_scanout_position = amdgpu_crtc_get_scanout_position,
>>>>    };
>>>>
>>>> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
>>>> +                                                        struct drm_scanout_buffer *sb)
>>>> +{
>>>> +       struct drm_framebuffer *fb;
>>>> +       struct amdgpu_bo *abo;
>>>> +       struct amdgpu_crtc *amdgpu_crtc;
>>>> +       struct amdgpu_device *adev;
>>>> +       uint32_t fb_format;
>>>> +
>>>> +       if (!plane->fb)
>>>> +               return -EINVAL;
>>>> +
>>>> +       fb = plane->fb;
>>>> +
>>>> +       abo = gem_to_amdgpu_bo(fb->obj[0]);
>>>> +       amdgpu_crtc = to_amdgpu_crtc(plane->crtc);
>>>> +       adev = drm_to_adev(fb->dev);
>>>> +
>>>> +       if (!abo->kmap.virtual &&
>>>> +           ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size), &abo->kmap)) {
>>>> +               DRM_WARN("amdgpu bo map failed, panic won't be displayed\n");
>>>> +               return -ENOMEM;
>>>> +       }
>>>> +
>>>> +       if (abo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK)
>>>> +               iosys_map_set_vaddr_iomem(&sb->map[0], abo->kmap.virtual);
>>>> +       else
>>>> +               iosys_map_set_vaddr(&sb->map[0], abo->kmap.virtual);
>>>> +
>>>> +       sb->width = fb->width;
>>>> +       sb->height = fb->height;
>>>> +       sb->format = fb->format;
>>>> +       sb->pitch[0] = fb->pitches[0];
>>>> +
>>>> +       /* Disable DC tiling */
>>>> +       fb_format = RREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset);
>>>> +       fb_format &= ~GRPH_ARRAY_MODE(0x7);
>>>> +       WREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset, fb_format);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
>>>> +       .get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
>>>> +};
>>>> +
>>>>    static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>>>>    {
>>>>           struct amdgpu_crtc *amdgpu_crtc;
>>>> @@ -2627,6 +2674,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>>>>           amdgpu_crtc->encoder = NULL;
>>>>           amdgpu_crtc->connector = NULL;
>>>>           drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
>>>> +       drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
>>>>
>>>>           return 0;
>>>>    }
>>>> --
>>>> 2.25.1
>>>>
>>>
>>
> 

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Jocelyn Falempe]

On 25/09/2024 10:33, Jocelyn Falempe wrote:
> On 24/09/2024 16:02, Alex Deucher wrote:
>> On Fri, Sep 20, 2024 at 11:36 AM Jocelyn Falempe <jfalempe@redhat.com> 
>> wrote:
>>>
>>> On 17/09/2024 15:21, Alex Deucher wrote:
>>>> On Mon, Aug 12, 2024 at 2:10 AM Lu Yao <yaolu@kylinos.cn> wrote:
>>>>>
>>>>> Add support for the drm_panic module, which displays a pretty user
>>>>> friendly message on the screen when a Linux kernel panic occurs.
>>>>>
>>>>> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
>>>>
>>>> Patch looks good to me.  Any chance you want to convert the other
>>>> non-DC dce files (dce_v8_0.c, dce_v10_0.c, dce_v11_0.c) while you are
>>>> at it?
>>>
>>> I've made a similar patch in amdgpu_dm_plane.c, and it works on a Radeon
>>> pro w6400.
>>> But it only works when I'm in a VT terminal (so the framebuffer is
>>> linear and CPU accessible).
>>> When under Gnome/Wayland, the flag AMDGPU_GEM_CREATE_NO_CPU_ACCESS is
>>> set, so that means I can't vmap it ?
>>
>> It just means that the application does not need CPU access.  Whether
>> or not the CPU can access the buffer or not depends on the size of the
>> PCI BAR.  E.g., if the driver or bios has resized the PCI BAR, then
>> the CPU can access the entire BAR, but if not you are generally
>> limited to the first 256M of framebuffer.
> 
> I tried to use ttm_bo_kmap() anyway, it returns a valid virtual address, 
> but writing to it has no effect on the display.
>>
>>>
>>> Also I don't know if there is a similar way to disable
>>> tiling/compression on this hardware.
>>
>> UNP_GRPH_CONTROL on chips with DCE display hardware and
>> DCSURF_ADDR_CONFIG and DCSURF_TILING_CONFIG on DCN display hardware.

I've now a working Prototype on a Radeon pro W6400.

Here is what I do to disable tiling:

REG_UPDATE(DCHUBP_REQ_SIZE_CONFIG, SWATH_HEIGHT, 0);
REG_UPDATE(DCSURF_TILING_CONFIG, SW_MODE, DC_SW_LINEAR);

REG_UPDATE_6(DCSURF_SURFACE_CONTROL,
		PRIMARY_SURFACE_DCC_EN, 0,
		PRIMARY_SURFACE_DCC_IND_BLK, 0,
		PRIMARY_SURFACE_DCC_IND_BLK_C, 0,
		SECONDARY_SURFACE_DCC_EN, 0,
		SECONDARY_SURFACE_DCC_IND_BLK, 0,
		SECONDARY_SURFACE_DCC_IND_BLK_C, 0);

I also need to call this, to refresh the display:

hubp->funcs->hubp_program_surface_flip_and_addr(hubp, 
&dc_plane_state->address, dc_plane_state->flip_immediate);

And I use a modified version of amdgpu_ttm_access_memory() to write to 
the framebuffer.

I will send a patch when I've time to make a clean version.

Best regards,

-- 

Jocelyn

Re: [PATCH v2] drm/amdgpu: add dce6 drm_panic support

[Alex Deucher]

Applied.  Thanks.

Alex

On Mon, Aug 12, 2024 at 2:10 AM Lu Yao <yaolu@kylinos.cn> wrote:
>
> Add support for the drm_panic module, which displays a pretty user
> friendly message on the screen when a Linux kernel panic occurs.
>
> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> ---
> Changes in v2:
> 1. Drop include "drm_internal.h"
> 2. Add disabling DC tiling ops.
> Per suggestion from previous thread:
> https://patchwork.freedesktop.org/patch/606879/?series=136832&rev=1
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 48 +++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 05c0df97f01d..ba1b7a36caa3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -28,6 +28,7 @@
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_modeset_helper_vtables.h>
>  #include <drm/drm_vblank.h>
> +#include <drm/drm_panic.h>
>
>  #include "amdgpu.h"
>  #include "amdgpu_pm.h"
> @@ -2600,6 +2601,52 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
>         .get_scanout_position = amdgpu_crtc_get_scanout_position,
>  };
>
> +static int dce_v6_0_drm_primary_plane_get_scanout_buffer(struct drm_plane *plane,
> +                                                        struct drm_scanout_buffer *sb)
> +{
> +       struct drm_framebuffer *fb;
> +       struct amdgpu_bo *abo;
> +       struct amdgpu_crtc *amdgpu_crtc;
> +       struct amdgpu_device *adev;
> +       uint32_t fb_format;
> +
> +       if (!plane->fb)
> +               return -EINVAL;
> +
> +       fb = plane->fb;
> +
> +       abo = gem_to_amdgpu_bo(fb->obj[0]);
> +       amdgpu_crtc = to_amdgpu_crtc(plane->crtc);
> +       adev = drm_to_adev(fb->dev);
> +
> +       if (!abo->kmap.virtual &&
> +           ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size), &abo->kmap)) {
> +               DRM_WARN("amdgpu bo map failed, panic won't be displayed\n");
> +               return -ENOMEM;
> +       }
> +
> +       if (abo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK)
> +               iosys_map_set_vaddr_iomem(&sb->map[0], abo->kmap.virtual);
> +       else
> +               iosys_map_set_vaddr(&sb->map[0], abo->kmap.virtual);
> +
> +       sb->width = fb->width;
> +       sb->height = fb->height;
> +       sb->format = fb->format;
> +       sb->pitch[0] = fb->pitches[0];
> +
> +       /* Disable DC tiling */
> +       fb_format = RREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset);
> +       fb_format &= ~GRPH_ARRAY_MODE(0x7);
> +       WREG32(mmGRPH_CONTROL + amdgpu_crtc->crtc_offset, fb_format);
> +
> +       return 0;
> +}
> +
> +static const struct drm_plane_helper_funcs dce_v6_0_drm_primary_plane_helper_funcs = {
> +       .get_scanout_buffer = dce_v6_0_drm_primary_plane_get_scanout_buffer
> +};
> +
>  static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>  {
>         struct amdgpu_crtc *amdgpu_crtc;
> @@ -2627,6 +2674,7 @@ static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index)
>         amdgpu_crtc->encoder = NULL;
>         amdgpu_crtc->connector = NULL;
>         drm_crtc_helper_add(&amdgpu_crtc->base, &dce_v6_0_crtc_helper_funcs);
> +       drm_plane_helper_add(amdgpu_crtc->base.primary, &dce_v6_0_drm_primary_plane_helper_funcs);
>
>         return 0;
>  }
> --
> 2.25.1
>