On Thu, Aug 8, 2024 at 12:43 PM Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
>
> On Thu, Aug 8, 2024 at 2:35 AM Lu Yao <yaolu@xxxxxxxxxx> 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@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,
> >
> > 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
