On Thu, Aug 8, 2024 at 2:35 AM Lu Yao <ya...@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 <ya...@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-59084a37d...@gmail.com/
> > and
> > https://lore.kernel.org/dri-devel/d233c376-ed07-2127-6084-8292d313d...@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-47f02b64a...@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
