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@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxx/
> and
> https://lore.kernel.org/dri-devel/d233c376-ed07-2127-6084-8292d313dac7@xxxxxxx/
>
> 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@xxxxxxx/
>
>
> Best regards,
