dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* Re:
  2012-05-18 12:27 (unknown), Sascha Hauer
@ 2012-05-22 14:06 ` Lars-Peter Clausen
  2012-05-23  8:12   ` Re: Sascha Hauer
  2012-05-24  6:31   ` Re: Sascha Hauer
  0 siblings, 2 replies; 62+ messages in thread
From: Lars-Peter Clausen @ 2012-05-22 14:06 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: linux-arm-kernel, dri-devel

On 05/18/2012 02:27 PM, Sascha Hauer wrote:
> Hi All,
> 
> The following adds a drm/kms driver for the Freescale i.MX LCDC
> controller. Most notable change to the last SDRM based version is that
> the SDRM layer has been removed and the driver now is purely i.MX
> specific. I hope that this is more acceptable now.
> 
> Another change is that the probe is now devicetree based. For now I
> took the easy way out and only put an edid blob into the devicetree.
> I haven't documented the binding yet, I would add that when the rest
> is considered ok.
> 
> Comments very welcome.
> 

Hi,

I really liked the sdrm layer. At least some bits of it. I've been working
on a "simple" DRM driver as well. The hardware has no fancy acceleration
features, just a simple buffer and some scanout logic. I'm basically using
the same gem buffer structure and the buffer is also allocated using
dma_alloc_writecombine, which means we can probably share all of the GEM
handling code and probably also most of the fbdev code. I also started with
the Exynos GEM code as a template, but reworked it later to be more like the
UDL code, which made it a bit more compact. I think it would be a good idea
to put at least the GEM handling in some common code as I expect that we'll
see more similar "simple" DRM drivers pop up.

The code in question can be found at
https://github.com/lclausen-adi/linux-2.6/commit/87a8fd6b98eeee317c7a486846cc8405d0bd68d8

Btw. the imx-drm.h is missing in your patch.

- Lars

> Thanks
>  Sascha
> 
> ----------------------------------------------------------------
> Sascha Hauer (2):
>       DRM: add Freescale i.MX LCDC driver
>       pcm038 lcdc support
> 
>  arch/arm/boot/dts/imx27-phytec-phycore.dts |   39 ++
>  arch/arm/boot/dts/imx27.dtsi               |    7 +
>  arch/arm/mach-imx/clock-imx27.c            |    1 +
>  drivers/gpu/drm/Kconfig                    |    2 +
>  drivers/gpu/drm/Makefile                   |    1 +
>  drivers/gpu/drm/imx/Kconfig                |   18 +
>  drivers/gpu/drm/imx/Makefile               |    8 +
>  drivers/gpu/drm/imx/imx-drm-core.c         |  745 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/imx/imx-fb.c               |  179 +++++++
>  drivers/gpu/drm/imx/imx-fbdev.c            |  275 ++++++++++
>  drivers/gpu/drm/imx/imx-gem.c              |  343 +++++++++++++
>  drivers/gpu/drm/imx/imx-lcdc-crtc.c        |  517 +++++++++++++++++++
>  drivers/gpu/drm/imx/imx-parallel-display.c |  228 +++++++++
>  13 files changed, 2363 insertions(+)
>  create mode 100644 drivers/gpu/drm/imx/Kconfig
>  create mode 100644 drivers/gpu/drm/imx/Makefile
>  create mode 100644 drivers/gpu/drm/imx/imx-drm-core.c
>  create mode 100644 drivers/gpu/drm/imx/imx-fb.c
>  create mode 100644 drivers/gpu/drm/imx/imx-fbdev.c
>  create mode 100644 drivers/gpu/drm/imx/imx-gem.c
>  create mode 100644 drivers/gpu/drm/imx/imx-lcdc-crtc.c
>  create mode 100644 drivers/gpu/drm/imx/imx-parallel-display.c
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2012-05-22 14:06 ` Lars-Peter Clausen
@ 2012-05-23  8:12   ` Sascha Hauer
  2012-05-24  6:31   ` Re: Sascha Hauer
  1 sibling, 0 replies; 62+ messages in thread
From: Sascha Hauer @ 2012-05-23  8:12 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: linux-arm-kernel, dri-devel

On Tue, May 22, 2012 at 04:06:41PM +0200, Lars-Peter Clausen wrote:
> On 05/18/2012 02:27 PM, Sascha Hauer wrote:
> > Hi All,
> > 
> > The following adds a drm/kms driver for the Freescale i.MX LCDC
> > controller. Most notable change to the last SDRM based version is that
> > the SDRM layer has been removed and the driver now is purely i.MX
> > specific. I hope that this is more acceptable now.
> > 
> > Another change is that the probe is now devicetree based. For now I
> > took the easy way out and only put an edid blob into the devicetree.
> > I haven't documented the binding yet, I would add that when the rest
> > is considered ok.
> > 
> > Comments very welcome.
> > 
> 
> Hi,
> 
> I really liked the sdrm layer. At least some bits of it. I've been working
> on a "simple" DRM driver as well. The hardware has no fancy acceleration
> features, just a simple buffer and some scanout logic. I'm basically using
> the same gem buffer structure and the buffer is also allocated using
> dma_alloc_writecombine, which means we can probably share all of the GEM
> handling code and probably also most of the fbdev code. I also started with
> the Exynos GEM code as a template, but reworked it later to be more like the
> UDL code, which made it a bit more compact. I think it would be a good idea
> to put at least the GEM handling in some common code as I expect that we'll
> see more similar "simple" DRM drivers pop up.

I totally agree. Having to track other drivers for bug fixes to apply
them on the own driver is not very convenient. As answered to Rob I do
not really have a clue how to accomplish this.

> 
> The code in question can be found at
> https://github.com/lclausen-adi/linux-2.6/commit/87a8fd6b98eeee317c7a486846cc8405d0bd68d8
> 
> Btw. the imx-drm.h is missing in your patch.

Oops, here it is for reference, will include it in the next round.


#ifndef _IMX_DRM_H_
#define _IMX_DRM_H_

/**
 * User-desired buffer creation information structure.
 *
 * @size: requested size for the object.
 *	- this size value would be page-aligned internally.
 * @flags: user request for setting memory type or cache attributes.
 * @handle: returned handle for the object.
 */
struct imx_drm_gem_create {
	unsigned int size;
	unsigned int flags;
	unsigned int handle;
};

struct imx_drm_device;
struct imx_drm_crtc;

struct imx_drm_crtc_helper_funcs {
	int (*enable_vblank)(struct drm_crtc *crtc);
	void (*disable_vblank)(struct drm_crtc *crtc);
};

int imx_drm_add_crtc(struct drm_crtc *crtc,
		struct imx_drm_crtc **new_crtc,
		const struct drm_crtc_funcs *crtc_funcs,
		const struct drm_crtc_helper_funcs *crtc_helper_funcs,
		const struct imx_drm_crtc_helper_funcs *ec_helper_funcs,
		struct module *owner);
int imx_drm_remove_crtc(struct imx_drm_crtc *);
int imx_drm_init_drm(struct platform_device *pdev,
		int preferred_bpp);
int imx_drm_exit_drm(void);

int imx_drm_crtc_vblank_get(struct imx_drm_crtc *imx_drm_crtc);
void imx_drm_crtc_vblank_put(struct imx_drm_crtc *imx_drm_crtc);
void imx_drm_handle_vblank(struct imx_drm_crtc *imx_drm_crtc);

/*
 * imx drm buffer entry structure.
 *
 * @paddr: physical address of allocated memory.
 * @vaddr: kernel virtual address of allocated memory.
 * @size: size of allocated memory.
 */
struct imx_drm_buf_entry {
	dma_addr_t paddr;
	void __iomem *vaddr;
	unsigned int size;
};

/* get physical memory information of a drm framebuffer. */
struct imx_drm_buf_entry *imx_drm_fb_get_buf(struct drm_framebuffer *fb);

struct imx_drm_encoder;
int imx_drm_add_encoder(struct drm_encoder *encoder,
		struct imx_drm_encoder **new_enc,
		struct module *owner);
int imx_drm_remove_encoder(struct imx_drm_encoder *);

struct imx_drm_connector;
int imx_drm_add_connector(struct drm_connector *connector,
		struct imx_drm_connector **new_con,
		struct module *owner);
int imx_drm_remove_connector(struct imx_drm_connector *);

void imx_drm_mode_config_init(struct drm_device *drm);

#define to_imx_drm_gem_obj(x)	container_of(x,\
			struct imx_drm_gem_obj, base)

struct imx_drm_gem_obj {
	struct drm_gem_object base;
	struct imx_drm_buf_entry *entry;
};

/* unmap a buffer from user space. */
int imx_drm_gem_munmap_ioctl(struct drm_device *drm, void *data,
		struct drm_file *file_priv);

/* initialize gem object. */
int imx_drm_gem_init_object(struct drm_gem_object *obj);

/* free gem object. */
void imx_drm_gem_free_object(struct drm_gem_object *gem_obj);

/* create memory region for drm framebuffer. */
int imx_drm_gem_dumb_create(struct drm_file *file_priv,
		struct drm_device *drm, struct drm_mode_create_dumb *args);

/* map memory region for drm framebuffer to user space. */
int imx_drm_gem_dumb_map_offset(struct drm_file *file_priv,
		struct drm_device *drm, uint32_t handle, uint64_t *offset);

/* page fault handler and mmap fault address(virtual) to physical memory. */
int imx_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);

/* set vm_flags and we can change the vm attribute to other one at here. */
int imx_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);

/*
 * destroy memory region allocated.
 *	- a gem handle and physical memory region pointed by a gem object
 *	would be released by drm_gem_handle_delete().
 */
int imx_drm_gem_dumb_destroy(struct drm_file *file_priv,
		struct drm_device *drm, unsigned int handle);

/* allocate physical memory. */
struct imx_drm_buf_entry *imx_drm_buf_create(struct drm_device *drm,
		unsigned int size);

/* remove allocated physical memory. */
void imx_drm_buf_destroy(struct drm_device *drm, struct imx_drm_buf_entry *entry);

struct drm_device *imx_drm_device_get(void);
void imx_drm_device_put(void);

#endif /* _IMX_DRM_H_ */
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2012-05-22 14:06 ` Lars-Peter Clausen
  2012-05-23  8:12   ` Re: Sascha Hauer
@ 2012-05-24  6:31   ` Sascha Hauer
  1 sibling, 0 replies; 62+ messages in thread
From: Sascha Hauer @ 2012-05-24  6:31 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: linux-arm-kernel, dri-devel

On Tue, May 22, 2012 at 04:06:41PM +0200, Lars-Peter Clausen wrote:
> On 05/18/2012 02:27 PM, Sascha Hauer wrote:
> > Hi All,
> > 
> > The following adds a drm/kms driver for the Freescale i.MX LCDC
> > controller. Most notable change to the last SDRM based version is that
> > the SDRM layer has been removed and the driver now is purely i.MX
> > specific. I hope that this is more acceptable now.
> > 
> > Another change is that the probe is now devicetree based. For now I
> > took the easy way out and only put an edid blob into the devicetree.
> > I haven't documented the binding yet, I would add that when the rest
> > is considered ok.
> > 
> > Comments very welcome.
> > 
> 
> Hi,
> 
> I really liked the sdrm layer. At least some bits of it. I've been working
> on a "simple" DRM driver as well. The hardware has no fancy acceleration
> features, just a simple buffer and some scanout logic. I'm basically using
> the same gem buffer structure and the buffer is also allocated using
> dma_alloc_writecombine, which means we can probably share all of the GEM
> handling code and probably also most of the fbdev code. I also started with
> the Exynos GEM code as a template, but reworked it later to be more like the
> UDL code, which made it a bit more compact. I think it would be a good idea
> to put at least the GEM handling in some common code as I expect that we'll
> see more similar "simple" DRM drivers pop up.

Ok, I'll try to put the GEM stuff into helper functions. Would you care
to review/test it? I have something else to do right now but I hope I'll
be there next week.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2013-08-13  9:56 (unknown), Christian König
@ 2013-08-13 14:47 ` Alex Deucher
  0 siblings, 0 replies; 62+ messages in thread
From: Alex Deucher @ 2013-08-13 14:47 UTC (permalink / raw)
  To: Christian König; +Cc: dri-devel

On Tue, Aug 13, 2013 at 5:56 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Hey Alex,
>
> here are my patches for reworking the ring function pointers and separating out the UVD and DMA rings.
>
> Everything is rebased on your drm-next-3.12-wip branch, please review and add them to your branch.

Patches look good to me.  I've added them to my 3.12 tree.

Alex

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2014-08-24 13:14 (unknown), Christian König
@ 2014-08-24 13:34 ` Mike Lothian
  2014-08-25  9:10   ` Re: Christian König
  0 siblings, 1 reply; 62+ messages in thread
From: Mike Lothian @ 2014-08-24 13:34 UTC (permalink / raw)
  To: Christian König; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1070 bytes --]

Thanks for this

Good work
On 24 Aug 2014 14:15, "Christian König" <deathsimple@vodafone.de> wrote:

> Hello everyone,
>
> the following patches add UVD support for older ASICs (RV6xx, RS[78]80,
> RV7[79]0). For everybody wanting to test it I've also uploaded a branch to
> FDO:
> http://cgit.freedesktop.org/~deathsimple/linux/log/?h=uvd-r600-release
>
> Additionally to the patches you need UVD firmware as well, which can be
> found at the usual location:
> http://people.freedesktop.org/~agd5f/radeon_ucode/
>
> A small Mesa patch is needed as well, cause the older hardware doesn't
> support field based output of video frames. So unfortunately VDPAU/OpenGL
> interop won't work either.
>
> We can only provide best effort support for those older ASICs, but at
> least on my RS[78]80 based laptop it seems to work perfectly fine.
>
> Happy testing,
> Christian.
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 1682 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2014-08-24 13:34 ` Mike Lothian
@ 2014-08-25  9:10   ` Christian König
  2014-09-07 13:24     ` Re: Markus Trippelsdorf
  0 siblings, 1 reply; 62+ messages in thread
From: Christian König @ 2014-08-25  9:10 UTC (permalink / raw)
  To: Mike Lothian; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1550 bytes --]

Let me know if it works for you, cause we don't really have any hardware 
any more to test it.

Christian.

Am 24.08.2014 um 15:34 schrieb Mike Lothian:
>
> Thanks for this
>
> Good work
>
> On 24 Aug 2014 14:15, "Christian König" <deathsimple@vodafone.de 
> <mailto:deathsimple@vodafone.de>> wrote:
>
>     Hello everyone,
>
>     the following patches add UVD support for older ASICs (RV6xx,
>     RS[78]80, RV7[79]0). For everybody wanting to test it I've also
>     uploaded a branch to FDO:
>     http://cgit.freedesktop.org/~deathsimple/linux/log/?h=uvd-r600-release
>     <http://cgit.freedesktop.org/%7Edeathsimple/linux/log/?h=uvd-r600-release>
>
>     Additionally to the patches you need UVD firmware as well, which
>     can be found at the usual location:
>     http://people.freedesktop.org/~agd5f/radeon_ucode/
>     <http://people.freedesktop.org/%7Eagd5f/radeon_ucode/>
>
>     A small Mesa patch is needed as well, cause the older hardware
>     doesn't support field based output of video frames. So
>     unfortunately VDPAU/OpenGL interop won't work either.
>
>     We can only provide best effort support for those older ASICs, but
>     at least on my RS[78]80 based laptop it seems to work perfectly fine.
>
>     Happy testing,
>     Christian.
>
>     _______________________________________________
>     dri-devel mailing list
>     dri-devel@lists.freedesktop.org
>     <mailto:dri-devel@lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/dri-devel
>


[-- Attachment #1.2: Type: text/html, Size: 2761 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2014-08-25  9:10   ` Re: Christian König
@ 2014-09-07 13:24     ` Markus Trippelsdorf
  2014-09-08  3:47       ` Re: Alex Deucher
  0 siblings, 1 reply; 62+ messages in thread
From: Markus Trippelsdorf @ 2014-09-07 13:24 UTC (permalink / raw)
  To: Christian König; +Cc: dri-devel

On 2014.08.25 at 11:10 +0200, Christian König wrote:
> Let me know if it works for you, cause we don't really have any hardware 
> any more to test it.

I've tested your patch series today (using drm-next-3.18 from
~agd5f/linux) on a RS780D/Radeon HD 3300 system with a couple of H264
videos. While it sometimes works as expected, it stalled the GPU far too
often to be usable. The stalls are not recoverable and the machine ends
up with a black sreen, but still accepts SysRq keyboard inputs.

Here are some logs:

vdpauinfo:
display: :0   screen: 0
API version: 1
Information string: G3DVL VDPAU Driver Shared Library version 1.0

Video surface:

name   width height types
-------------------------------------------
420     8192  8192  NV12 YV12 
422     8192  8192  UYVY YUYV 
444     8192  8192  Y8U8V8A8 V8U8Y8A8 

Decoder capabilities:

name               level macbs width height
-------------------------------------------
MPEG1                 0  9216  2048  1152
MPEG2_SIMPLE          3  9216  2048  1152
MPEG2_MAIN            3  9216  2048  1152
H264_BASELINE        41  9216  2048  1152
H264_MAIN            41  9216  2048  1152
H264_HIGH            41  9216  2048  1152
VC1_ADVANCED          4  9216  2048  1152

Output surface:

name              width height nat types
----------------------------------------------------
B8G8R8A8          8192  8192    y  NV12 YV12 UYVY YUYV Y8U8V8A8 V8U8Y8A8 
R8G8B8A8          8192  8192    y  NV12 YV12 UYVY YUYV Y8U8V8A8 V8U8Y8A8 
R10G10B10A2       8192  8192    y  NV12 YV12 UYVY YUYV Y8U8V8A8 V8U8Y8A8 
B10G10R10A2       8192  8192    y  NV12 YV12 UYVY YUYV Y8U8V8A8 V8U8Y8A8 

Bitmap surface:

name              width height
------------------------------
B8G8R8A8          8192  8192
R8G8B8A8          8192  8192
R10G10B10A2       8192  8192
B10G10R10A2       8192  8192
A8                8192  8192

Video mixer:

feature name                    sup
------------------------------------
DEINTERLACE_TEMPORAL             y
DEINTERLACE_TEMPORAL_SPATIAL     -
INVERSE_TELECINE                 -
NOISE_REDUCTION                  y
SHARPNESS                        y
LUMA_KEY                         -
HIGH QUALITY SCALING - L1        -
HIGH QUALITY SCALING - L2        -
HIGH QUALITY SCALING - L3        -
HIGH QUALITY SCALING - L4        -
HIGH QUALITY SCALING - L5        -
HIGH QUALITY SCALING - L6        -
HIGH QUALITY SCALING - L7        -
HIGH QUALITY SCALING - L8        -
HIGH QUALITY SCALING - L9        -

parameter name                  sup      min      max
-----------------------------------------------------
VIDEO_SURFACE_WIDTH              y        48     2048
VIDEO_SURFACE_HEIGHT             y        48     1152
CHROMA_TYPE                      y  
LAYERS                           y         0        4

attribute name                  sup      min      max
-----------------------------------------------------
BACKGROUND_COLOR                 y  
CSC_MATRIX                       y  
NOISE_REDUCTION_LEVEL            y      0.00     1.00
SHARPNESS_LEVEL                  y     -1.00     1.00
LUMA_KEY_MIN_LUMA                y  
LUMA_KEY_MAX_LUMA                y  


Sep  7 14:03:45 x4 kernel: [drm] Initialized drm 1.1.0 20060810
Sep  7 14:03:45 x4 kernel: [drm] radeon kernel modesetting enabled.
Sep  7 14:03:45 x4 kernel: [drm] initializing kernel modesetting (RS780 0x1002:0x9614 0x1043:0x834D).
Sep  7 14:03:45 x4 kernel: [drm] register mmio base: 0xFBEE0000
Sep  7 14:03:45 x4 kernel: [drm] register mmio size: 65536
Sep  7 14:03:45 x4 kernel: ATOM BIOS: 113
Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: VRAM: 128M 0x00000000C0000000 - 0x00000000C7FFFFFF (128M used)
Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: GTT: 512M 0x00000000A0000000 - 0x00000000BFFFFFFF
Sep  7 14:03:45 x4 kernel: [drm] Detected VRAM RAM=128M, BAR=128M
Sep  7 14:03:45 x4 kernel: [drm] RAM width 32bits DDR
Sep  7 14:03:45 x4 kernel: [TTM] Zone  kernel: Available graphics memory: 4083350 kiB
Sep  7 14:03:45 x4 kernel: [TTM] Zone   dma32: Available graphics memory: 2097152 kiB
Sep  7 14:03:45 x4 kernel: [TTM] Initializing pool allocator
Sep  7 14:03:45 x4 kernel: [TTM] Initializing DMA pool allocator
Sep  7 14:03:45 x4 kernel: [drm] radeon: 128M of VRAM memory ready
Sep  7 14:03:45 x4 kernel: [drm] radeon: 512M of GTT memory ready.
Sep  7 14:03:45 x4 kernel: [drm] Loading RS780 Microcode
Sep  7 14:03:45 x4 kernel: == power state 0 ==
Sep  7 14:03:45 x4 kernel: 	ui class: none
Sep  7 14:03:45 x4 kernel: 	internal class: boot 
Sep  7 14:03:45 x4 kernel: 	caps: video 
Sep  7 14:03:45 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:03:45 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 2
Sep  7 14:03:45 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 2
Sep  7 14:03:45 x4 kernel: 	status: c r b 
Sep  7 14:03:45 x4 kernel: == power state 1 ==
Sep  7 14:03:45 x4 kernel: 	ui class: performance
Sep  7 14:03:45 x4 kernel: 	internal class: none
Sep  7 14:03:45 x4 kernel: 	caps: video 
Sep  7 14:03:45 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:03:45 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:03:45 x4 kernel: 		power level 1    sclk: 70000 vddc_index: 2
Sep  7 14:03:45 x4 kernel: 	status: 
Sep  7 14:03:45 x4 kernel: == power state 2 ==
Sep  7 14:03:45 x4 kernel: 	ui class: none
Sep  7 14:03:45 x4 kernel: 	internal class: uvd 
Sep  7 14:03:45 x4 kernel: 	caps: video 
Sep  7 14:03:45 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:03:45 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:03:45 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:03:45 x4 kernel: 	status: 
Sep  7 14:03:45 x4 kernel: [drm] radeon: dpm initialized
Sep  7 14:03:45 x4 kernel: [drm] GART: num cpu pages 131072, num gpu pages 131072
Sep  7 14:03:45 x4 kernel: [drm] PCIE GART of 512M enabled (table at 0x00000000C0258000).
Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: WB enabled
Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: fence driver on ring 0 use gpu addr 0x00000000a0000c00 and cpu addr 0xffff8800db8dcc00
Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: fence driver on ring 5 use gpu addr 0x00000000c0056038 and cpu addr 0xffffc90000116038
Sep  7 14:03:45 x4 kernel: [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
Sep  7 14:03:45 x4 kernel: [drm] Driver supports precise vblank timestamp query.
Sep  7 14:03:45 x4 kernel: [drm] radeon: irq initialized.
Sep  7 14:03:45 x4 kernel: [drm] ring test on 0 succeeded in 1 usecs
Sep  7 14:03:45 x4 kernel: [drm] ring test on 5 succeeded in 1 usecs
Sep  7 14:03:45 x4 kernel: [drm] UVD initialized successfully.
Sep  7 14:03:45 x4 kernel: [drm] ib test on ring 0 succeeded in 0 usecs
Sep  7 14:03:45 x4 kernel: [drm] ib test on ring 5 succeeded
Sep  7 14:03:45 x4 kernel: [drm] Radeon Display Connectors
Sep  7 14:03:45 x4 kernel: [drm] Connector 0:
Sep  7 14:03:45 x4 kernel: [drm]   VGA-1
Sep  7 14:03:45 x4 kernel: [drm]   DDC: 0x7e40 0x7e40 0x7e44 0x7e44 0x7e48 0x7e48 0x7e4c 0x7e4c
Sep  7 14:03:45 x4 kernel: [drm]   Encoders:
Sep  7 14:03:45 x4 kernel: [drm]     CRT1: INTERNAL_KLDSCP_DAC1
Sep  7 14:03:45 x4 kernel: [drm] Connector 1:
Sep  7 14:03:45 x4 kernel: [drm]   DVI-D-1
Sep  7 14:03:45 x4 kernel: [drm]   HPD3
Sep  7 14:03:45 x4 kernel: [drm]   DDC: 0x7e50 0x7e50 0x7e54 0x7e54 0x7e58 0x7e58 0x7e5c 0x7e5c
Sep  7 14:03:45 x4 kernel: [drm]   Encoders:
Sep  7 14:03:45 x4 kernel: [drm]     DFP3: INTERNAL_KLDSCP_LVTMA
Sep  7 14:03:45 x4 kernel: switching from power state:
Sep  7 14:03:45 x4 kernel: 	ui class: none
Sep  7 14:03:45 x4 kernel: 	internal class: boot 
Sep  7 14:03:45 x4 kernel: 	caps: video 
Sep  7 14:03:45 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:03:45 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 2
Sep  7 14:03:45 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 2
Sep  7 14:03:45 x4 kernel: 	status: c b 
Sep  7 14:03:45 x4 kernel: switching to power state:
Sep  7 14:03:45 x4 kernel: 	ui class: performance
Sep  7 14:03:45 x4 kernel: 	internal class: none
Sep  7 14:03:45 x4 kernel: 	caps: video 
Sep  7 14:03:45 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:03:45 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:03:45 x4 kernel: 		power level 1    sclk: 70000 vddc_index: 2
Sep  7 14:03:45 x4 kernel: 	status: r 
Sep  7 14:03:45 x4 kernel: [drm] fb mappable at 0xF0359000
Sep  7 14:03:45 x4 kernel: [drm] vram apper at 0xF0000000
Sep  7 14:03:45 x4 kernel: [drm] size 7299072
Sep  7 14:03:45 x4 kernel: [drm] fb depth is 24
Sep  7 14:03:45 x4 kernel: [drm]    pitch is 6912
Sep  7 14:03:45 x4 kernel: fbcon: radeondrmfb (fb0) is primary device
Sep  7 14:03:45 x4 kernel: Console: switching to colour frame buffer device 131x105
Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: fb0: radeondrmfb frame buffer device
Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: registered panic notifier
Sep  7 14:03:45 x4 kernel: tsc: Refined TSC clocksource calibration: 3210.826 MHz
Sep  7 14:03:45 x4 kernel: [drm] Initialized radeon 2.40.0 20080528 for 0000:01:05.0 on minor 0
...
Sep  7 14:20:37 x4 kernel: switching to power state:
Sep  7 14:20:37 x4 kernel: 	ui class: none
Sep  7 14:20:37 x4 kernel: 	internal class: uvd 
Sep  7 14:20:37 x4 kernel: 	caps: video 
Sep  7 14:20:37 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:20:37 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:20:37 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:20:37 x4 kernel: 	status: r 
Sep  7 14:20:54 x4 kernel: switching from power state:
Sep  7 14:20:54 x4 kernel: 	ui class: none
Sep  7 14:20:54 x4 kernel: 	internal class: uvd 
Sep  7 14:20:54 x4 kernel: 	caps: video 
Sep  7 14:20:54 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:20:54 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:20:54 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:20:54 x4 kernel: 	status: c 
Sep  7 14:20:54 x4 kernel: switching to power state:
Sep  7 14:20:54 x4 kernel: 	ui class: performance
Sep  7 14:20:54 x4 kernel: 	internal class: none
Sep  7 14:20:54 x4 kernel: 	caps: video 
Sep  7 14:20:54 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:20:54 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:20:54 x4 kernel: 		power level 1    sclk: 70000 vddc_index: 2
Sep  7 14:20:54 x4 kernel: 	status: r 
Sep  7 14:21:02 x4 kernel: switching from power state:
Sep  7 14:21:02 x4 kernel: 	ui class: performance
Sep  7 14:21:02 x4 kernel: 	internal class: none
Sep  7 14:21:02 x4 kernel: 	caps: video 
Sep  7 14:21:02 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:21:02 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:21:02 x4 kernel: 		power level 1    sclk: 70000 vddc_index: 2
Sep  7 14:21:02 x4 kernel: 	status: c 
Sep  7 14:21:02 x4 kernel: switching to power state:
Sep  7 14:21:02 x4 kernel: 	ui class: none
Sep  7 14:21:02 x4 kernel: 	internal class: uvd 
Sep  7 14:21:02 x4 kernel: 	caps: video 
Sep  7 14:21:02 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:21:02 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:21:02 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:21:02 x4 kernel: 	status: r 
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10106msec
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000072ac last fence id 0x00000000000072b4 on ring 0)
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: Saved 377 dwords of commands on ring 0.
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: GPU softreset: 0x00000099
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA00034AF
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20044040
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000004
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000002
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00005087
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80098645
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: R_008020_GRBM_SOFT_RESET=0x00007FEF
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: SRBM_SOFT_RESET=0x00008100
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA0003030
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20048040
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000000
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000000
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00000000
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80100000
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: GPU reset succeeded, trying to resume
Sep  7 14:21:13 x4 kernel: [drm] PCIE GART of 512M enabled (table at 0x00000000C0258000).
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: WB enabled
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: fence driver on ring 0 use gpu addr 0x00000000a0000c00 and cpu addr 0xffff8800db8dcc00
Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: fence driver on ring 5 use gpu addr 0x00000000c0056038 and cpu addr 0xffffc90000116038
Sep  7 14:21:13 x4 kernel: [drm] ring test on 0 succeeded in 1 usecs
Sep  7 14:21:13 x4 kernel: [drm] ring test on 5 succeeded in 1 usecs
Sep  7 14:21:13 x4 kernel: [drm] UVD initialized successfully.
Sep  7 14:21:13 x4 kernel: switching from power state:
Sep  7 14:21:13 x4 kernel: 	ui class: none
Sep  7 14:21:13 x4 kernel: 	internal class: boot 
Sep  7 14:21:13 x4 kernel: 	caps: video 
Sep  7 14:21:13 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:21:13 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 2
Sep  7 14:21:13 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 2
Sep  7 14:21:13 x4 kernel: 	status: c b 
Sep  7 14:21:13 x4 kernel: switching to power state:
Sep  7 14:21:13 x4 kernel: 	ui class: none
Sep  7 14:21:13 x4 kernel: 	internal class: uvd 
Sep  7 14:21:13 x4 kernel: 	caps: video 
Sep  7 14:21:13 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:21:13 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:21:13 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:21:13 x4 kernel: 	status: r 
Sep  7 14:21:20 x4 kernel: SysRq : Emergency Sync
(new boot)
Sep  7 14:44:49 x4 kernel: switching from power state:
Sep  7 14:44:49 x4 kernel: 	ui class: performance
Sep  7 14:44:49 x4 kernel: 	internal class: none
Sep  7 14:44:49 x4 kernel: 	caps: video 
Sep  7 14:44:49 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:44:49 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:44:49 x4 kernel: 		power level 1    sclk: 70000 vddc_index: 2
Sep  7 14:44:49 x4 kernel: 	status: c 
Sep  7 14:44:49 x4 kernel: switching to power state:
Sep  7 14:44:49 x4 kernel: 	ui class: none
Sep  7 14:44:49 x4 kernel: 	internal class: uvd 
Sep  7 14:44:49 x4 kernel: 	caps: video 
Sep  7 14:44:49 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:44:49 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:44:49 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:44:49 x4 kernel: 	status: r 
Sep  7 14:44:59 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 10000msec
Sep  7 14:44:59 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000eac last fence id 0x0000000000000eaf on ring 5)
Sep  7 14:44:59 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10466msec
Sep  7 14:44:59 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000064ff last fence id 0x0000000000006504 on ring 0)
Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 10500msec
Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000eac last fence id 0x0000000000000eaf on ring 5)
Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10966msec
Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000064ff last fence id 0x0000000000006504 on ring 0)
Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 11000msec
...
Sep  7 14:45:17 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000eac last fence id 0x0000000000000eaf on ring 5)
Sep  7 14:45:17 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 28466msec
Sep  7 14:45:17 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000064ff last fence id 0x0000000000006504 on ring 0)
Sep  7 14:45:18 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 28500msec
Sep  7 14:45:18 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000eac last fence id 0x0000000000000eaf on ring 5)
Sep  7 14:45:18 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 28966msec
Sep  7 14:45:18 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000064ff last fence id 0x0000000000006504 on ring 0)
Sep  7 14:45:18 x4 kernel: SysRq : Emergency Sync
(new boot)
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 10000msec
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10000msec
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000001ed last fence id 0x00000000000001f6 on ring 0)
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x000000000000000b last fence id 0x000000000000000f on ring 5)
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: Saved 409 dwords of commands on ring 0.
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: GPU softreset: 0x00000099
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA00034E0
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20045040
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x01000004
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00200002
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00005087
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x808D8645
Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: R_008020_GRBM_SOFT_RESET=0x00007FEF
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: SRBM_SOFT_RESET=0x00008100
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA0003030
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20048040
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000000
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000000
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00000000
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80100000
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: GPU reset succeeded, trying to resume
Sep  7 14:48:15 x4 kernel: [drm] PCIE GART of 512M enabled (table at 0x00000000C0258000).
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: WB enabled
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: fence driver on ring 0 use gpu addr 0x00000000a0000c00 and cpu addr 0xffff8800db8dcc00
Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: fence driver on ring 5 use gpu addr 0x00000000c0056038 and cpu addr 0xffffc90000116038
Sep  7 14:48:15 x4 kernel: [drm] ring test on 0 succeeded in 1 usecs
Sep  7 14:48:15 x4 kernel: [drm] ring test on 5 succeeded in 1 usecs
Sep  7 14:48:15 x4 kernel: [drm] UVD initialized successfully.
Sep  7 14:48:15 x4 kernel: switching from power state:
Sep  7 14:48:15 x4 kernel: 	ui class: none
Sep  7 14:48:15 x4 kernel: 	internal class: boot 
Sep  7 14:48:15 x4 kernel: 	caps: video 
Sep  7 14:48:15 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:48:15 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 2
Sep  7 14:48:15 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 2
Sep  7 14:48:15 x4 kernel: 	status: c b 
Sep  7 14:48:15 x4 kernel: switching to power state:
Sep  7 14:48:15 x4 kernel: 	ui class: none
Sep  7 14:48:15 x4 kernel: 	internal class: uvd 
Sep  7 14:48:15 x4 kernel: 	caps: video 
Sep  7 14:48:15 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:48:15 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:48:15 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:48:15 x4 kernel: 	status: r 
Sep  7 14:48:15 x4 kernel: SysRq : Emergency Sync
(new boot)
Sep  7 14:53:20 x4 kernel: switching to power state:
Sep  7 14:53:20 x4 kernel: 	ui class: none
Sep  7 14:53:20 x4 kernel: 	internal class: uvd 
Sep  7 14:53:20 x4 kernel: 	caps: video 
Sep  7 14:53:20 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:53:20 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:53:20 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:53:20 x4 kernel: 	status: r 
Sep  7 14:53:30 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10050msec
Sep  7 14:53:30 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000596 last fence id 0x00000000000005a3 on ring 0)
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: Saved 601 dwords of commands on ring 0.
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: GPU softreset: 0x00000088
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA00030B0
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20045040
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000004
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000002
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00005087
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80098645
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: R_008020_GRBM_SOFT_RESET=0x00004001
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: SRBM_SOFT_RESET=0x00008100
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA0003030
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20048040
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000000
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000000
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00000000
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80100000
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: GPU reset succeeded, trying to resume
Sep  7 14:53:31 x4 kernel: [drm] PCIE GART of 512M enabled (table at 0x00000000C0258000).
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: WB enabled
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: fence driver on ring 0 use gpu addr 0x00000000a0000c00 and cpu addr 0xffff8800db8dcc00
Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: fence driver on ring 5 use gpu addr 0x00000000c0056038 and cpu addr 0xffffc90000116038
Sep  7 14:53:31 x4 kernel: [drm] ring test on 0 succeeded in 1 usecs
Sep  7 14:53:31 x4 kernel: [drm] ring test on 5 succeeded in 1 usecs
Sep  7 14:53:31 x4 kernel: [drm] UVD initialized successfully.
Sep  7 14:53:31 x4 kernel: switching from power state:
Sep  7 14:53:31 x4 kernel: 	ui class: none
Sep  7 14:53:31 x4 kernel: 	internal class: boot 
Sep  7 14:53:31 x4 kernel: 	caps: video 
Sep  7 14:53:31 x4 kernel: 	uvd    vclk: 0 dclk: 0
Sep  7 14:53:31 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 2
Sep  7 14:53:31 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 2
Sep  7 14:53:31 x4 kernel: 	status: c b 
Sep  7 14:53:31 x4 kernel: switching to power state:
Sep  7 14:53:31 x4 kernel: 	ui class: none
Sep  7 14:53:31 x4 kernel: 	internal class: uvd 
Sep  7 14:53:31 x4 kernel: 	caps: video 
Sep  7 14:53:31 x4 kernel: 	uvd    vclk: 53300 dclk: 40000
Sep  7 14:53:31 x4 kernel: 		power level 0    sclk: 50000 vddc_index: 1
Sep  7 14:53:31 x4 kernel: 		power level 1    sclk: 50000 vddc_index: 1
Sep  7 14:53:31 x4 kernel: 	status: r 
Sep  7 14:53:39 x4 kernel: SysRq : Emergency Sync

-- 
Markus

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2014-09-07 13:24     ` Re: Markus Trippelsdorf
@ 2014-09-08  3:47       ` Alex Deucher
  2014-09-08  7:13         ` Re: Markus Trippelsdorf
  0 siblings, 1 reply; 62+ messages in thread
From: Alex Deucher @ 2014-09-08  3:47 UTC (permalink / raw)
  To: Markus Trippelsdorf; +Cc: Maling list - DRI developers

On Sun, Sep 7, 2014 at 9:24 AM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2014.08.25 at 11:10 +0200, Christian König wrote:
>> Let me know if it works for you, cause we don't really have any hardware
>> any more to test it.
>
> I've tested your patch series today (using drm-next-3.18 from
> ~agd5f/linux) on a RS780D/Radeon HD 3300 system with a couple of H264
> videos. While it sometimes works as expected, it stalled the GPU far too
> often to be usable. The stalls are not recoverable and the machine ends
> up with a black sreen, but still accepts SysRq keyboard inputs.


Does it work any better if dpm is disabled?

Alex

>
> Here are some logs:
>
> vdpauinfo:
> display: :0   screen: 0
> API version: 1
> Information string: G3DVL VDPAU Driver Shared Library version 1.0
>
> Video surface:
>
> name   width height types
> -------------------------------------------
> 420     8192  8192  NV12 YV12
> 422     8192  8192  UYVY YUYV
> 444     8192  8192  Y8U8V8A8 V8U8Y8A8
>
> Decoder capabilities:
>
> name               level macbs width height
> -------------------------------------------
> MPEG1                 0  9216  2048  1152
> MPEG2_SIMPLE          3  9216  2048  1152
> MPEG2_MAIN            3  9216  2048  1152
> H264_BASELINE        41  9216  2048  1152
> H264_MAIN            41  9216  2048  1152
> H264_HIGH            41  9216  2048  1152
> VC1_ADVANCED          4  9216  2048  1152
>
> Output surface:
>
> name              width height nat types
> ----------------------------------------------------
> B8G8R8A8          8192  8192    y  NV12 YV12 UYVY YUYV Y8U8V8A8 V8U8Y8A8
> R8G8B8A8          8192  8192    y  NV12 YV12 UYVY YUYV Y8U8V8A8 V8U8Y8A8
> R10G10B10A2       8192  8192    y  NV12 YV12 UYVY YUYV Y8U8V8A8 V8U8Y8A8
> B10G10R10A2       8192  8192    y  NV12 YV12 UYVY YUYV Y8U8V8A8 V8U8Y8A8
>
> Bitmap surface:
>
> name              width height
> ------------------------------
> B8G8R8A8          8192  8192
> R8G8B8A8          8192  8192
> R10G10B10A2       8192  8192
> B10G10R10A2       8192  8192
> A8                8192  8192
>
> Video mixer:
>
> feature name                    sup
> ------------------------------------
> DEINTERLACE_TEMPORAL             y
> DEINTERLACE_TEMPORAL_SPATIAL     -
> INVERSE_TELECINE                 -
> NOISE_REDUCTION                  y
> SHARPNESS                        y
> LUMA_KEY                         -
> HIGH QUALITY SCALING - L1        -
> HIGH QUALITY SCALING - L2        -
> HIGH QUALITY SCALING - L3        -
> HIGH QUALITY SCALING - L4        -
> HIGH QUALITY SCALING - L5        -
> HIGH QUALITY SCALING - L6        -
> HIGH QUALITY SCALING - L7        -
> HIGH QUALITY SCALING - L8        -
> HIGH QUALITY SCALING - L9        -
>
> parameter name                  sup      min      max
> -----------------------------------------------------
> VIDEO_SURFACE_WIDTH              y        48     2048
> VIDEO_SURFACE_HEIGHT             y        48     1152
> CHROMA_TYPE                      y
> LAYERS                           y         0        4
>
> attribute name                  sup      min      max
> -----------------------------------------------------
> BACKGROUND_COLOR                 y
> CSC_MATRIX                       y
> NOISE_REDUCTION_LEVEL            y      0.00     1.00
> SHARPNESS_LEVEL                  y     -1.00     1.00
> LUMA_KEY_MIN_LUMA                y
> LUMA_KEY_MAX_LUMA                y
>
>
> Sep  7 14:03:45 x4 kernel: [drm] Initialized drm 1.1.0 20060810
> Sep  7 14:03:45 x4 kernel: [drm] radeon kernel modesetting enabled.
> Sep  7 14:03:45 x4 kernel: [drm] initializing kernel modesetting (RS780 0x1002:0x9614 0x1043:0x834D).
> Sep  7 14:03:45 x4 kernel: [drm] register mmio base: 0xFBEE0000
> Sep  7 14:03:45 x4 kernel: [drm] register mmio size: 65536
> Sep  7 14:03:45 x4 kernel: ATOM BIOS: 113
> Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: VRAM: 128M 0x00000000C0000000 - 0x00000000C7FFFFFF (128M used)
> Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: GTT: 512M 0x00000000A0000000 - 0x00000000BFFFFFFF
> Sep  7 14:03:45 x4 kernel: [drm] Detected VRAM RAM=128M, BAR=128M
> Sep  7 14:03:45 x4 kernel: [drm] RAM width 32bits DDR
> Sep  7 14:03:45 x4 kernel: [TTM] Zone  kernel: Available graphics memory: 4083350 kiB
> Sep  7 14:03:45 x4 kernel: [TTM] Zone   dma32: Available graphics memory: 2097152 kiB
> Sep  7 14:03:45 x4 kernel: [TTM] Initializing pool allocator
> Sep  7 14:03:45 x4 kernel: [TTM] Initializing DMA pool allocator
> Sep  7 14:03:45 x4 kernel: [drm] radeon: 128M of VRAM memory ready
> Sep  7 14:03:45 x4 kernel: [drm] radeon: 512M of GTT memory ready.
> Sep  7 14:03:45 x4 kernel: [drm] Loading RS780 Microcode
> Sep  7 14:03:45 x4 kernel: == power state 0 ==
> Sep  7 14:03:45 x4 kernel:      ui class: none
> Sep  7 14:03:45 x4 kernel:      internal class: boot
> Sep  7 14:03:45 x4 kernel:      caps: video
> Sep  7 14:03:45 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:03:45 x4 kernel:              power level 0    sclk: 50000 vddc_index: 2
> Sep  7 14:03:45 x4 kernel:              power level 1    sclk: 50000 vddc_index: 2
> Sep  7 14:03:45 x4 kernel:      status: c r b
> Sep  7 14:03:45 x4 kernel: == power state 1 ==
> Sep  7 14:03:45 x4 kernel:      ui class: performance
> Sep  7 14:03:45 x4 kernel:      internal class: none
> Sep  7 14:03:45 x4 kernel:      caps: video
> Sep  7 14:03:45 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:03:45 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:03:45 x4 kernel:              power level 1    sclk: 70000 vddc_index: 2
> Sep  7 14:03:45 x4 kernel:      status:
> Sep  7 14:03:45 x4 kernel: == power state 2 ==
> Sep  7 14:03:45 x4 kernel:      ui class: none
> Sep  7 14:03:45 x4 kernel:      internal class: uvd
> Sep  7 14:03:45 x4 kernel:      caps: video
> Sep  7 14:03:45 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:03:45 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:03:45 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:03:45 x4 kernel:      status:
> Sep  7 14:03:45 x4 kernel: [drm] radeon: dpm initialized
> Sep  7 14:03:45 x4 kernel: [drm] GART: num cpu pages 131072, num gpu pages 131072
> Sep  7 14:03:45 x4 kernel: [drm] PCIE GART of 512M enabled (table at 0x00000000C0258000).
> Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: WB enabled
> Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: fence driver on ring 0 use gpu addr 0x00000000a0000c00 and cpu addr 0xffff8800db8dcc00
> Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: fence driver on ring 5 use gpu addr 0x00000000c0056038 and cpu addr 0xffffc90000116038
> Sep  7 14:03:45 x4 kernel: [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> Sep  7 14:03:45 x4 kernel: [drm] Driver supports precise vblank timestamp query.
> Sep  7 14:03:45 x4 kernel: [drm] radeon: irq initialized.
> Sep  7 14:03:45 x4 kernel: [drm] ring test on 0 succeeded in 1 usecs
> Sep  7 14:03:45 x4 kernel: [drm] ring test on 5 succeeded in 1 usecs
> Sep  7 14:03:45 x4 kernel: [drm] UVD initialized successfully.
> Sep  7 14:03:45 x4 kernel: [drm] ib test on ring 0 succeeded in 0 usecs
> Sep  7 14:03:45 x4 kernel: [drm] ib test on ring 5 succeeded
> Sep  7 14:03:45 x4 kernel: [drm] Radeon Display Connectors
> Sep  7 14:03:45 x4 kernel: [drm] Connector 0:
> Sep  7 14:03:45 x4 kernel: [drm]   VGA-1
> Sep  7 14:03:45 x4 kernel: [drm]   DDC: 0x7e40 0x7e40 0x7e44 0x7e44 0x7e48 0x7e48 0x7e4c 0x7e4c
> Sep  7 14:03:45 x4 kernel: [drm]   Encoders:
> Sep  7 14:03:45 x4 kernel: [drm]     CRT1: INTERNAL_KLDSCP_DAC1
> Sep  7 14:03:45 x4 kernel: [drm] Connector 1:
> Sep  7 14:03:45 x4 kernel: [drm]   DVI-D-1
> Sep  7 14:03:45 x4 kernel: [drm]   HPD3
> Sep  7 14:03:45 x4 kernel: [drm]   DDC: 0x7e50 0x7e50 0x7e54 0x7e54 0x7e58 0x7e58 0x7e5c 0x7e5c
> Sep  7 14:03:45 x4 kernel: [drm]   Encoders:
> Sep  7 14:03:45 x4 kernel: [drm]     DFP3: INTERNAL_KLDSCP_LVTMA
> Sep  7 14:03:45 x4 kernel: switching from power state:
> Sep  7 14:03:45 x4 kernel:      ui class: none
> Sep  7 14:03:45 x4 kernel:      internal class: boot
> Sep  7 14:03:45 x4 kernel:      caps: video
> Sep  7 14:03:45 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:03:45 x4 kernel:              power level 0    sclk: 50000 vddc_index: 2
> Sep  7 14:03:45 x4 kernel:              power level 1    sclk: 50000 vddc_index: 2
> Sep  7 14:03:45 x4 kernel:      status: c b
> Sep  7 14:03:45 x4 kernel: switching to power state:
> Sep  7 14:03:45 x4 kernel:      ui class: performance
> Sep  7 14:03:45 x4 kernel:      internal class: none
> Sep  7 14:03:45 x4 kernel:      caps: video
> Sep  7 14:03:45 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:03:45 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:03:45 x4 kernel:              power level 1    sclk: 70000 vddc_index: 2
> Sep  7 14:03:45 x4 kernel:      status: r
> Sep  7 14:03:45 x4 kernel: [drm] fb mappable at 0xF0359000
> Sep  7 14:03:45 x4 kernel: [drm] vram apper at 0xF0000000
> Sep  7 14:03:45 x4 kernel: [drm] size 7299072
> Sep  7 14:03:45 x4 kernel: [drm] fb depth is 24
> Sep  7 14:03:45 x4 kernel: [drm]    pitch is 6912
> Sep  7 14:03:45 x4 kernel: fbcon: radeondrmfb (fb0) is primary device
> Sep  7 14:03:45 x4 kernel: Console: switching to colour frame buffer device 131x105
> Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: fb0: radeondrmfb frame buffer device
> Sep  7 14:03:45 x4 kernel: radeon 0000:01:05.0: registered panic notifier
> Sep  7 14:03:45 x4 kernel: tsc: Refined TSC clocksource calibration: 3210.826 MHz
> Sep  7 14:03:45 x4 kernel: [drm] Initialized radeon 2.40.0 20080528 for 0000:01:05.0 on minor 0
> ...
> Sep  7 14:20:37 x4 kernel: switching to power state:
> Sep  7 14:20:37 x4 kernel:      ui class: none
> Sep  7 14:20:37 x4 kernel:      internal class: uvd
> Sep  7 14:20:37 x4 kernel:      caps: video
> Sep  7 14:20:37 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:20:37 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:20:37 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:20:37 x4 kernel:      status: r
> Sep  7 14:20:54 x4 kernel: switching from power state:
> Sep  7 14:20:54 x4 kernel:      ui class: none
> Sep  7 14:20:54 x4 kernel:      internal class: uvd
> Sep  7 14:20:54 x4 kernel:      caps: video
> Sep  7 14:20:54 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:20:54 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:20:54 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:20:54 x4 kernel:      status: c
> Sep  7 14:20:54 x4 kernel: switching to power state:
> Sep  7 14:20:54 x4 kernel:      ui class: performance
> Sep  7 14:20:54 x4 kernel:      internal class: none
> Sep  7 14:20:54 x4 kernel:      caps: video
> Sep  7 14:20:54 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:20:54 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:20:54 x4 kernel:              power level 1    sclk: 70000 vddc_index: 2
> Sep  7 14:20:54 x4 kernel:      status: r
> Sep  7 14:21:02 x4 kernel: switching from power state:
> Sep  7 14:21:02 x4 kernel:      ui class: performance
> Sep  7 14:21:02 x4 kernel:      internal class: none
> Sep  7 14:21:02 x4 kernel:      caps: video
> Sep  7 14:21:02 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:21:02 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:21:02 x4 kernel:              power level 1    sclk: 70000 vddc_index: 2
> Sep  7 14:21:02 x4 kernel:      status: c
> Sep  7 14:21:02 x4 kernel: switching to power state:
> Sep  7 14:21:02 x4 kernel:      ui class: none
> Sep  7 14:21:02 x4 kernel:      internal class: uvd
> Sep  7 14:21:02 x4 kernel:      caps: video
> Sep  7 14:21:02 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:21:02 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:21:02 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:21:02 x4 kernel:      status: r
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10106msec
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000072ac last fence id 0x00000000000072b4 on ring 0)
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: Saved 377 dwords of commands on ring 0.
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: GPU softreset: 0x00000099
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA00034AF
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20044040
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000004
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000002
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00005087
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80098645
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: R_008020_GRBM_SOFT_RESET=0x00007FEF
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: SRBM_SOFT_RESET=0x00008100
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA0003030
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20048040
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000000
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000000
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00000000
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80100000
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: GPU reset succeeded, trying to resume
> Sep  7 14:21:13 x4 kernel: [drm] PCIE GART of 512M enabled (table at 0x00000000C0258000).
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: WB enabled
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: fence driver on ring 0 use gpu addr 0x00000000a0000c00 and cpu addr 0xffff8800db8dcc00
> Sep  7 14:21:13 x4 kernel: radeon 0000:01:05.0: fence driver on ring 5 use gpu addr 0x00000000c0056038 and cpu addr 0xffffc90000116038
> Sep  7 14:21:13 x4 kernel: [drm] ring test on 0 succeeded in 1 usecs
> Sep  7 14:21:13 x4 kernel: [drm] ring test on 5 succeeded in 1 usecs
> Sep  7 14:21:13 x4 kernel: [drm] UVD initialized successfully.
> Sep  7 14:21:13 x4 kernel: switching from power state:
> Sep  7 14:21:13 x4 kernel:      ui class: none
> Sep  7 14:21:13 x4 kernel:      internal class: boot
> Sep  7 14:21:13 x4 kernel:      caps: video
> Sep  7 14:21:13 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:21:13 x4 kernel:              power level 0    sclk: 50000 vddc_index: 2
> Sep  7 14:21:13 x4 kernel:              power level 1    sclk: 50000 vddc_index: 2
> Sep  7 14:21:13 x4 kernel:      status: c b
> Sep  7 14:21:13 x4 kernel: switching to power state:
> Sep  7 14:21:13 x4 kernel:      ui class: none
> Sep  7 14:21:13 x4 kernel:      internal class: uvd
> Sep  7 14:21:13 x4 kernel:      caps: video
> Sep  7 14:21:13 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:21:13 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:21:13 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:21:13 x4 kernel:      status: r
> Sep  7 14:21:20 x4 kernel: SysRq : Emergency Sync
> (new boot)
> Sep  7 14:44:49 x4 kernel: switching from power state:
> Sep  7 14:44:49 x4 kernel:      ui class: performance
> Sep  7 14:44:49 x4 kernel:      internal class: none
> Sep  7 14:44:49 x4 kernel:      caps: video
> Sep  7 14:44:49 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:44:49 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:44:49 x4 kernel:              power level 1    sclk: 70000 vddc_index: 2
> Sep  7 14:44:49 x4 kernel:      status: c
> Sep  7 14:44:49 x4 kernel: switching to power state:
> Sep  7 14:44:49 x4 kernel:      ui class: none
> Sep  7 14:44:49 x4 kernel:      internal class: uvd
> Sep  7 14:44:49 x4 kernel:      caps: video
> Sep  7 14:44:49 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:44:49 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:44:49 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:44:49 x4 kernel:      status: r
> Sep  7 14:44:59 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 10000msec
> Sep  7 14:44:59 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000eac last fence id 0x0000000000000eaf on ring 5)
> Sep  7 14:44:59 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10466msec
> Sep  7 14:44:59 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000064ff last fence id 0x0000000000006504 on ring 0)
> Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 10500msec
> Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000eac last fence id 0x0000000000000eaf on ring 5)
> Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10966msec
> Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000064ff last fence id 0x0000000000006504 on ring 0)
> Sep  7 14:45:00 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 11000msec
> ...
> Sep  7 14:45:17 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000eac last fence id 0x0000000000000eaf on ring 5)
> Sep  7 14:45:17 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 28466msec
> Sep  7 14:45:17 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000064ff last fence id 0x0000000000006504 on ring 0)
> Sep  7 14:45:18 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 28500msec
> Sep  7 14:45:18 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000eac last fence id 0x0000000000000eaf on ring 5)
> Sep  7 14:45:18 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 28966msec
> Sep  7 14:45:18 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000064ff last fence id 0x0000000000006504 on ring 0)
> Sep  7 14:45:18 x4 kernel: SysRq : Emergency Sync
> (new boot)
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: ring 5 stalled for more than 10000msec
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10000msec
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x00000000000001ed last fence id 0x00000000000001f6 on ring 0)
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x000000000000000b last fence id 0x000000000000000f on ring 5)
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: Saved 409 dwords of commands on ring 0.
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0: GPU softreset: 0x00000099
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA00034E0
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20045040
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x01000004
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00200002
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00005087
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x808D8645
> Sep  7 14:48:14 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: R_008020_GRBM_SOFT_RESET=0x00007FEF
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: SRBM_SOFT_RESET=0x00008100
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA0003030
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20048040
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000000
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000000
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00000000
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80100000
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: GPU reset succeeded, trying to resume
> Sep  7 14:48:15 x4 kernel: [drm] PCIE GART of 512M enabled (table at 0x00000000C0258000).
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: WB enabled
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: fence driver on ring 0 use gpu addr 0x00000000a0000c00 and cpu addr 0xffff8800db8dcc00
> Sep  7 14:48:15 x4 kernel: radeon 0000:01:05.0: fence driver on ring 5 use gpu addr 0x00000000c0056038 and cpu addr 0xffffc90000116038
> Sep  7 14:48:15 x4 kernel: [drm] ring test on 0 succeeded in 1 usecs
> Sep  7 14:48:15 x4 kernel: [drm] ring test on 5 succeeded in 1 usecs
> Sep  7 14:48:15 x4 kernel: [drm] UVD initialized successfully.
> Sep  7 14:48:15 x4 kernel: switching from power state:
> Sep  7 14:48:15 x4 kernel:      ui class: none
> Sep  7 14:48:15 x4 kernel:      internal class: boot
> Sep  7 14:48:15 x4 kernel:      caps: video
> Sep  7 14:48:15 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:48:15 x4 kernel:              power level 0    sclk: 50000 vddc_index: 2
> Sep  7 14:48:15 x4 kernel:              power level 1    sclk: 50000 vddc_index: 2
> Sep  7 14:48:15 x4 kernel:      status: c b
> Sep  7 14:48:15 x4 kernel: switching to power state:
> Sep  7 14:48:15 x4 kernel:      ui class: none
> Sep  7 14:48:15 x4 kernel:      internal class: uvd
> Sep  7 14:48:15 x4 kernel:      caps: video
> Sep  7 14:48:15 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:48:15 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:48:15 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:48:15 x4 kernel:      status: r
> Sep  7 14:48:15 x4 kernel: SysRq : Emergency Sync
> (new boot)
> Sep  7 14:53:20 x4 kernel: switching to power state:
> Sep  7 14:53:20 x4 kernel:      ui class: none
> Sep  7 14:53:20 x4 kernel:      internal class: uvd
> Sep  7 14:53:20 x4 kernel:      caps: video
> Sep  7 14:53:20 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:53:20 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:53:20 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:53:20 x4 kernel:      status: r
> Sep  7 14:53:30 x4 kernel: radeon 0000:01:05.0: ring 0 stalled for more than 10050msec
> Sep  7 14:53:30 x4 kernel: radeon 0000:01:05.0: GPU lockup (current fence id 0x0000000000000596 last fence id 0x00000000000005a3 on ring 0)
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: Saved 601 dwords of commands on ring 0.
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: GPU softreset: 0x00000088
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA00030B0
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20045040
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000004
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000002
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00005087
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80098645
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: R_008020_GRBM_SOFT_RESET=0x00004001
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: SRBM_SOFT_RESET=0x00008100
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008010_GRBM_STATUS      = 0xA0003030
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008014_GRBM_STATUS2     = 0x00000003
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_000E50_SRBM_STATUS      = 0x20048040
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008674_CP_STALLED_STAT1 = 0x00000000
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008678_CP_STALLED_STAT2 = 0x00000000
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_00867C_CP_BUSY_STAT     = 0x00000000
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_008680_CP_STAT          = 0x80100000
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: GPU reset succeeded, trying to resume
> Sep  7 14:53:31 x4 kernel: [drm] PCIE GART of 512M enabled (table at 0x00000000C0258000).
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: WB enabled
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: fence driver on ring 0 use gpu addr 0x00000000a0000c00 and cpu addr 0xffff8800db8dcc00
> Sep  7 14:53:31 x4 kernel: radeon 0000:01:05.0: fence driver on ring 5 use gpu addr 0x00000000c0056038 and cpu addr 0xffffc90000116038
> Sep  7 14:53:31 x4 kernel: [drm] ring test on 0 succeeded in 1 usecs
> Sep  7 14:53:31 x4 kernel: [drm] ring test on 5 succeeded in 1 usecs
> Sep  7 14:53:31 x4 kernel: [drm] UVD initialized successfully.
> Sep  7 14:53:31 x4 kernel: switching from power state:
> Sep  7 14:53:31 x4 kernel:      ui class: none
> Sep  7 14:53:31 x4 kernel:      internal class: boot
> Sep  7 14:53:31 x4 kernel:      caps: video
> Sep  7 14:53:31 x4 kernel:      uvd    vclk: 0 dclk: 0
> Sep  7 14:53:31 x4 kernel:              power level 0    sclk: 50000 vddc_index: 2
> Sep  7 14:53:31 x4 kernel:              power level 1    sclk: 50000 vddc_index: 2
> Sep  7 14:53:31 x4 kernel:      status: c b
> Sep  7 14:53:31 x4 kernel: switching to power state:
> Sep  7 14:53:31 x4 kernel:      ui class: none
> Sep  7 14:53:31 x4 kernel:      internal class: uvd
> Sep  7 14:53:31 x4 kernel:      caps: video
> Sep  7 14:53:31 x4 kernel:      uvd    vclk: 53300 dclk: 40000
> Sep  7 14:53:31 x4 kernel:              power level 0    sclk: 50000 vddc_index: 1
> Sep  7 14:53:31 x4 kernel:              power level 1    sclk: 50000 vddc_index: 1
> Sep  7 14:53:31 x4 kernel:      status: r
> Sep  7 14:53:39 x4 kernel: SysRq : Emergency Sync
>
> --
> Markus
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2014-09-08  3:47       ` Re: Alex Deucher
@ 2014-09-08  7:13         ` Markus Trippelsdorf
  0 siblings, 0 replies; 62+ messages in thread
From: Markus Trippelsdorf @ 2014-09-08  7:13 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Maling list - DRI developers

On 2014.09.07 at 23:47 -0400, Alex Deucher wrote:
> On Sun, Sep 7, 2014 at 9:24 AM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
> > On 2014.08.25 at 11:10 +0200, Christian König wrote:
> >> Let me know if it works for you, cause we don't really have any hardware
> >> any more to test it.
> >
> > I've tested your patch series today (using drm-next-3.18 from
> > ~agd5f/linux) on a RS780D/Radeon HD 3300 system with a couple of H264
> > videos. While it sometimes works as expected, it stalled the GPU far too
> > often to be usable. The stalls are not recoverable and the machine ends
> > up with a black sreen, but still accepts SysRq keyboard inputs.
> 
> 
> Does it work any better if dpm is disabled?

Unfortunately no. The symptoms are unchanged.

-- 
Markus

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2018-03-05 17:06 (unknown) Meghana Madhyastha
@ 2018-03-05 19:24 ` Noralf Trønnes
  0 siblings, 0 replies; 62+ messages in thread
From: Noralf Trønnes @ 2018-03-05 19:24 UTC (permalink / raw)
  To: Meghana Madhyastha, Daniel Vetter, dri-devel


Den 05.03.2018 18.06, skrev Meghana Madhyastha:
> linux-spi@vger.kernel.org,Noralf Trønnes <noralf@tronnes.org>,Sean Paul <seanpaul@chromium.org>,kernel@martin.sperl.org
> Cc:
> Bcc:
> Subject: Re: [PATCH v2 0/2] Chunk splitting of spi transfers
> Reply-To:
> In-Reply-To: <f6dbf3ca-4c1b-90cc-c4af-8889f7407180@tronnes.org>
>
> On Sun, Mar 04, 2018 at 06:38:42PM +0100, Noralf Trønnes wrote:
>> Den 02.03.2018 12.11, skrev Meghana Madhyastha:
>>> On Sun, Feb 25, 2018 at 02:19:10PM +0100, Lukas Wunner wrote:
>>>> [cc += linux-rpi-kernel@lists.infradead.org]
>>>>
>>>> On Sat, Feb 24, 2018 at 06:15:59PM +0000, Meghana Madhyastha wrote:
>>>>> I've added bcm2835_spi_transfer_one_message in spi-bcm2835. This calls
>>>>> spi_split_transfers_maxsize to split large chunks for spi dma transfers.
>>>>> I then removed chunk splitting in the tinydrm spi helper (as now the core
>>>>> is handling the chunk splitting). However, although the SPI HW should be
>>>>> able to accomodate up to 65535 bytes for dma transfers, the splitting of
>>>>> chunks to 65535 bytes results in a dma transfer time out error. However,
>>>>> when the chunks are split to < 64 bytes it seems to work fine.
>>>> Hm, that is really odd, how did you test this exactly, what did you
>>>> use as SPI slave?  It contradicts our own experience, we're using
>>>> Micrel KSZ8851 Ethernet chips as SPI slave on spi0 of a BCM2837
>>>> and can send/receive messages via DMA to the tune of several hundred
>>>> bytes without any issues.  In fact, for messages < 96 bytes, DMA is
>>>> not used at all, so you've probably been using interrupt mode,
>>>> see the BCM2835_SPI_DMA_MIN_LENGTH macro in spi-bcm2835.c.
>>> Hi Lukas,
>>>
>>> I think you are right. I checked it and its not using the DMA mode which
>>> is why its working with 64 bytes.
>>> Noralf, that leaves us back to the
>>> initial time out problem. I've tried doing the message splitting in
>>> spi_sync as well as spi_pump_messages. Martin had explained that DMA
>>> will wait for
>>> the SPI HW to set the send_more_data line, but the SPI-HW itself will
>>> stop triggering it when SPI_LEN is 0 causing DMA to wait forever. I
>>> thought if we split it before itself, the SPI_LEN will not go to zero
>>> thus preventing this problem, however it didn't work and started
>>> hanging. So I'm a little uncertain as to how to proceed and debug what
>>> exactly has caused the time out due to the asynchronous methods.
>> I did a quick test and at least this is working:
>>
>> int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz,
>>               struct spi_transfer *header, u8 bpw, const void *buf,
>>               size_t len)
>> {
>>      struct spi_transfer tr = {
>>          .bits_per_word = bpw,
>>          .speed_hz = speed_hz,
>>          .tx_buf = buf,
>>          .len = len,
>>      };
>>      struct spi_message m;
>>      size_t maxsize;
>>      int ret;
>>
>>      maxsize = tinydrm_spi_max_transfer_size(spi, 0);
>>
>>      if (drm_debug & DRM_UT_DRIVER)
>>          pr_debug("[drm:%s] bpw=%u, maxsize=%zu, transfers:\n",
>>               __func__, bpw, maxsize);
>>
>>      spi_message_init(&m);
>>      m.spi = spi;
>>      if (header)
>>          spi_message_add_tail(header, &m);
>>      spi_message_add_tail(&tr, &m);
>>
>>      ret = spi_split_transfers_maxsize(spi->controller, &m, maxsize,
>> GFP_KERNEL);
>>      if (ret)
>>          return ret;
>>
>>      tinydrm_dbg_spi_message(spi, &m);
>>
>>      return spi_sync(spi, &m);
>> }
>> EXPORT_SYMBOL(tinydrm_spi_transfer);
>>
>>
>> Log:
>> [   39.015644] [drm:mipi_dbi_fb_dirty [mipi_dbi]] Flushing [FB:36] x1=0,
>> x2=320, y1=0, y2=240
>>
>> [   39.018079] [drm:mipi_dbi_typec3_command [mipi_dbi]] cmd=2a, par=00 00 01
>> 3f
>> [   39.018129] [drm:tinydrm_spi_transfer] bpw=8, maxsize=65532, transfers:
>> [   39.018152]     tr(0): speed=10MHz, bpw=8, len=1, tx_buf=[2a]
>> [   39.018231] [drm:tinydrm_spi_transfer] bpw=8, maxsize=65532, transfers:
>> [   39.018248]     tr(0): speed=10MHz, bpw=8, len=4, tx_buf=[00 00 01 3f]
>>
>> [   39.018330] [drm:mipi_dbi_typec3_command [mipi_dbi]] cmd=2b, par=00 00 00
>> ef
>> [   39.018347] [drm:tinydrm_spi_transfer] bpw=8, maxsize=65532, transfers:
>> [   39.018362]     tr(0): speed=10MHz, bpw=8, len=1, tx_buf=[2b]
>> [   39.018396] [drm:tinydrm_spi_transfer] bpw=8, maxsize=65532, transfers:
>> [   39.018428]     tr(0): speed=10MHz, bpw=8, len=4, tx_buf=[00 00 00 ef]
>>
>> [   39.018487] [drm:mipi_dbi_typec3_command [mipi_dbi]] cmd=2c, len=153600
>> [   39.018502] [drm:tinydrm_spi_transfer] bpw=8, maxsize=65532, transfers:
>> [   39.018517]     tr(0): speed=10MHz, bpw=8, len=1, tx_buf=[2c]
>> [   39.018565] [drm:tinydrm_spi_transfer] bpw=8, maxsize=65532, transfers:
>> [   39.018594]     tr(0): speed=48MHz, bpw=8, len=65532, tx_buf=[c6 18 c6 18
>> c6 18 c6 18 c6 18 c6 18 c6 18 c6 18 ...]
>> [   39.018608]     tr(1): speed=48MHz, bpw=8, len=65532, tx_buf=[06 18 06 18
>> 06 18 06 18 06 18 06 18 06 18 06 18 ...]
>> [   39.018621]     tr(2): speed=48MHz, bpw=8, len=22536, tx_buf=[10 82 10 82
>> 10 82 10 82 10 82 10 82 18 e3 18 e3 ...]
> Hi Noralf,
>
> Yes this works but splitting in the spi subsystem doesn't seem to work.
> So this means that spi_split_transfers_maxsize is working.
> Should I just send in a patch with splitting done here in tinydrm? (I
> had thought we wanted to avoid splitting in the tinydrm helper).

Oh, I assumed you didn't get this to work in any way.
Yes, I prefer splitting without the client's knowledge.

Looking at the code the splitting has to happen before spi_map_msg() is
called. Have you tried to do it in the prepare_message callback?

static void __spi_pump_messages(struct spi_controller *ctlr, bool 
in_kthread)
{
<...>
     if (ctlr->prepare_message) {
         ret = ctlr->prepare_message(ctlr, ctlr->cur_msg);
<...>
     ret = spi_map_msg(ctlr, ctlr->cur_msg);
<...>
     ret = ctlr->transfer_one_message(ctlr, ctlr->cur_msg);
<...>
}

There was something wrong with this email, it was missing subject and
several recipients.

Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2018-10-22  0:26 ` Dave Airlie
@ 2018-10-21 20:23   ` Michael Tirado
  2018-10-22  1:50     ` Re: Dave Airlie
  0 siblings, 1 reply; 62+ messages in thread
From: Michael Tirado @ 2018-10-21 20:23 UTC (permalink / raw)
  To: airlied
  Cc: Airlied, dri-devel, LKML, kraxel, alexander.deucher,
	christian.koenig, David1.zhou, Hongbo.He, Sean Paul, Gustavo,
	maarten.lankhorst

On Mon, Oct 22, 2018 at 12:26 AM Dave Airlie <airlied@gmail.com> wrote:
>
> This shouldn't be necessary, did someone misbackport the mmap changes without:
>
> drm: set FMODE_UNSIGNED_OFFSET for drm files
>
> Dave.

The latest kernel I have had to patch was a 4.18-rc6.  I'll try with a
newer 4.19 and let you know if it decides to work.  If not I'll
prepare a test case for demonstration on qemu-system-i386.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2018-10-22  1:50     ` Re: Dave Airlie
@ 2018-10-21 22:20       ` Michael Tirado
  2018-10-23  1:47       ` Re: Michael Tirado
  1 sibling, 0 replies; 62+ messages in thread
From: Michael Tirado @ 2018-10-21 22:20 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Airlied, dri-devel, LKML, kraxel, alexander.deucher,
	christian.koenig, David1.zhou, Hongbo.He, Sean Paul, Gustavo,
	maarten.lankhorst

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

On Mon, Oct 22, 2018 at 1:50 AM Dave Airlie <airlied@gmail.com> wrote:
>
> On Mon, 22 Oct 2018 at 10:49, Michael Tirado <mtirado418@gmail.com> wrote:
> >
> > On Mon, Oct 22, 2018 at 12:26 AM Dave Airlie <airlied@gmail.com> wrote:
> > >
> > > This shouldn't be necessary, did someone misbackport the mmap changes without:
> If you have custom userspace software, make sure it's using
> AC_SYS_LARGEFILE or whatever the equivalant is in your build system.
>
> 64-bit file offsets are important.
>

That fixed it! -D_FILE_OFFSET_BITS=64 is the pre-processor define
needed. It's a bit more than unintuitive but I'm glad I don't need
this stupid patch anymore, Thanks.

In case anyone is further interested I have attached test program
since I spent the last hour or so chopping it up anyway :S   [ gcc -o
kms -D_FILE_OFFSET_BITS=64 main.c ]

[-- Attachment #2: main.c --]
[-- Type: application/octet-stream, Size: 17153 bytes --]

/* Copyright (C) 2017 Michael R. Tirado <mtirado418@gmail.com> -- GPLv3+
 *
 * This program is libre software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation, either version 3 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details. You should have
 * received a copy of the GNU General Public License version 3
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */


#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <stdio.h>
#include <malloc.h>
#include <signal.h>
#include <stdlib.h>
#include <stdint.h>
#include <stddef.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <stdio.h>
#include <sys/mman.h>
#include <sys/ioctl.h>
#include <drm/drm.h>
#include <drm/drm_mode.h>

#define STRERR strerror(errno)

#ifndef PTRBITCOUNT
	#define PTRBITCOUNT 32
#endif
/* kernel structs use __u64 for pointer types */
#if (PTRBITCOUNT == 32)
	#define ptr_from_krn(ptr) ((void *)(uint32_t)(ptr))
	#define ptr_to_krn(ptr)   ((uint32_t)(ptr))
#elif (PTRBITCOUNT == 64)
	#define ptr_from_krn(ptr) ((void *)(uint64_t)(ptr))
	#define ptr_to_krn(ptr)   ((uint64_t)(ptr))
#else
	#error "PTRBITCOUNT is undefined"
#endif
#ifndef MAX_FBS
	#define MAX_FBS   12
#endif
#ifndef MAX_CRTCS
	#define MAX_CRTCS 12
#endif
#ifndef MAX_CONNECTORS
	#define MAX_CONNECTORS 12
#endif
#ifndef MAX_ENCODERS
	#define MAX_ENCODERS 12
#endif
#ifndef MAX_PROPS
	#define MAX_PROPS 256
#endif
#ifndef MAX_MODES
	#define MAX_MODES 256
#endif
#if (PTRBITCOUNT == 32)
	#define drm_to_ptr(ptr)   ((void *)(uint32_t)(ptr))
	#define drm_from_ptr(ptr) ((uint32_t)(ptr))
#elif (PTRBITCOUNT == 64)
	#define drm_to_ptr(ptr)   ((void *)(uint64_t)(ptr))
	#define drm_from_ptr(ptr) ((uint64_t)(ptr))
#else
	#error "PTRBITCOUNT is undefined"
#endif
#define drm_alloc(size) (drm_from_ptr(calloc(1,size)))

struct drm_buffer
{
	uint32_t drm_id;
	uint32_t fb_id;
	uint32_t pitch;
	uint32_t width;
	uint32_t height;
	uint32_t depth;
	uint32_t bpp;
	char    *addr;
	size_t   size;
};
struct drm_display
{
	struct drm_mode_get_encoder encoder;
	struct drm_mode_crtc crtc;
	struct drm_mode_get_connector *conn; /* do we need array for multi-screen? */
	struct drm_mode_modeinfo *modes; /* these both point to conn's mode array */
	struct drm_mode_modeinfo *cur_mode;
	uint32_t cur_mode_idx;
	uint32_t mode_count;
	uint32_t conn_id;
};
struct drm_kms
{
	struct drm_display display;
	struct drm_buffer *sfb;
	struct drm_mode_card_res *res;
	int card_fd;
};

/* get id out of drm_id_ptr */
static uint32_t drm_get_id(uint64_t addr, uint32_t idx)
{
	return ((uint32_t *)drm_to_ptr(addr))[idx];
}

static int free_mode_card_res(struct drm_mode_card_res *res)
{
	if (!res)
		return -1;
	if (res->fb_id_ptr)
		free(drm_to_ptr(res->fb_id_ptr));
	if (res->crtc_id_ptr)
		free(drm_to_ptr(res->crtc_id_ptr));
	if (res->encoder_id_ptr)
		free(drm_to_ptr(res->encoder_id_ptr));
	if (res->connector_id_ptr)
		free(drm_to_ptr(res->connector_id_ptr));
	free(res);
	return 0;
}

static struct drm_mode_card_res *alloc_mode_card_res(int fd)
{
	struct drm_mode_card_res res;
	struct drm_mode_card_res *ret;
	uint32_t count_fbs, count_crtcs, count_connectors, count_encoders;

	memset(&res, 0, sizeof(struct drm_mode_card_res));
	if (ioctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res)) {
		printf("ioctl(DRM_IOCTL_MODE_GETRESOURCES, &res): %s\n", STRERR);
		return NULL;
	}
	if (res.count_fbs > MAX_FBS
			|| res.count_crtcs > MAX_CRTCS
			|| res.count_encoders > MAX_ENCODERS
			|| res.count_connectors > MAX_CONNECTORS) {
		printf("resource limit reached, see defines.h\n");
		return NULL;
	}
	if (res.count_fbs) {
		res.fb_id_ptr = drm_alloc(sizeof(uint32_t)*res.count_fbs);
		if (!res.fb_id_ptr)
			goto alloc_err;
	}
	if (res.count_crtcs) {
		res.crtc_id_ptr = drm_alloc(sizeof(uint32_t)*res.count_crtcs);
		if (!res.crtc_id_ptr)
			goto alloc_err;
	}
	if (res.count_encoders) {
		res.encoder_id_ptr = drm_alloc(sizeof(uint32_t)*res.count_encoders);
		if (!res.encoder_id_ptr)
			goto alloc_err;
	}
	if (res.count_connectors) {
		res.connector_id_ptr = drm_alloc(sizeof(uint32_t)*res.count_connectors);
		if (!res.connector_id_ptr)
			goto alloc_err;
	}
	count_fbs = res.count_fbs;
	count_crtcs = res.count_crtcs;
	count_encoders = res.count_encoders;
	count_connectors = res.count_connectors;

	if (ioctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_GETRESOURCES, &res): %s\n", STRERR);
		goto free_err;
	}

	if (count_fbs != res.count_fbs
			|| count_crtcs != res.count_crtcs
			|| count_encoders != res.count_encoders
			|| count_connectors != res.count_connectors) {
		errno = EAGAIN;
		goto free_err;
	}

	ret = calloc(1, sizeof(struct drm_mode_card_res));
	if (!ret)
		goto alloc_err;

	memcpy(ret, &res, sizeof(struct drm_mode_card_res));
	return ret;

alloc_err:
	errno = ENOMEM;
free_err:
	free(drm_to_ptr(res.fb_id_ptr));
	free(drm_to_ptr(res.crtc_id_ptr));
	free(drm_to_ptr(res.connector_id_ptr));
	free(drm_to_ptr(res.encoder_id_ptr));
	return NULL;
}


static struct drm_mode_get_connector *alloc_connector(int fd, uint32_t conn_id)
{
	struct drm_mode_get_connector conn;
	struct drm_mode_get_connector *ret;
	uint32_t count_modes, count_props, count_encoders;

	memset(&conn, 0, sizeof(struct drm_mode_get_connector));
	conn.connector_id = conn_id;

	if (ioctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_GETCONNECTOR, &conn): %s\n", STRERR);
		return NULL;
	}
	if (conn.count_modes > MAX_MODES
			|| conn.count_props > MAX_PROPS
			|| conn.count_encoders > MAX_ENCODERS) {
		printf("resource limit reached, see defines.h\n");
		return NULL;
	}
	if (conn.count_modes) {
		conn.modes_ptr = drm_alloc(sizeof(struct drm_mode_modeinfo)
					   * conn.count_modes);
		if (!conn.modes_ptr)
			goto alloc_err;
	}
	if (conn.count_props) {
		conn.props_ptr = drm_alloc(sizeof(uint32_t)*conn.count_props);
		if (!conn.props_ptr)
			goto alloc_err;
		conn.prop_values_ptr = drm_alloc(sizeof(uint64_t)*conn.count_props);
		if (!conn.prop_values_ptr)
			goto alloc_err;
	}
	if (conn.count_encoders) {
		conn.encoders_ptr = drm_alloc(sizeof(uint32_t)*conn.count_encoders);
		if (!conn.encoders_ptr)
			goto alloc_err;
	}
	count_modes = conn.count_modes;
	count_props = conn.count_props;
	count_encoders = conn.count_encoders;

	if (ioctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_GETCONNECTOR, &conn): %s\n", STRERR);
		goto free_err;
	}

	if (count_modes != conn.count_modes
			|| count_props != conn.count_props
			|| count_encoders != conn.count_encoders) {
		errno = EAGAIN;
		goto free_err;
	}

	ret = calloc(1, sizeof(struct drm_mode_get_connector));
	if (!ret)
		goto alloc_err;

	memcpy(ret, &conn, sizeof(struct drm_mode_get_connector));
	return ret;

alloc_err:
	errno = ENOMEM;
free_err:
	free(drm_to_ptr(conn.modes_ptr));
	free(drm_to_ptr(conn.props_ptr));
	free(drm_to_ptr(conn.encoders_ptr));
	free(drm_to_ptr(conn.prop_values_ptr));
	return NULL;
}

static struct drm_mode_modeinfo *get_connector_modeinfo(struct drm_mode_get_connector *conn,  uint32_t *count)
{
	if (!conn || !count)
		return NULL;
	*count = conn->count_modes;
	return drm_to_ptr(conn->modes_ptr);

}

static int free_connector(struct drm_mode_get_connector *conn)
{
	if (!conn)
		return -1;
	if (conn->modes_ptr)
		free(drm_to_ptr(conn->modes_ptr));
	if (conn->props_ptr)
		free(drm_to_ptr(conn->props_ptr));
	if (conn->encoders_ptr)
		free(drm_to_ptr(conn->encoders_ptr));
	if (conn->prop_values_ptr)
		free(drm_to_ptr(conn->prop_values_ptr));
	free(conn);
	return 0;
}


static int drm_kms_connect_sfb(struct drm_kms *self)
{
	struct drm_display *display = &self->display;
	struct drm_mode_get_connector *conn;
	struct drm_mode_modeinfo *cur_mode;
	struct drm_mode_get_encoder *encoder;
	struct drm_mode_crtc *crtc;
	if (!display || !display->conn || !display->cur_mode || !self->sfb)
		return -1;

	conn = display->conn;
	cur_mode = display->cur_mode;
	encoder = &self->display.encoder;
	crtc = &self->display.crtc;
	memset(crtc, 0, sizeof(struct drm_mode_crtc));
	memset(encoder,  0, sizeof(struct drm_mode_get_encoder));


	/* XXX: there can be multiple encoders, have not investigated this much */
	if (conn->encoder_id == 0) {
		printf("conn->encoder_id was 0, defaulting to encoder[0]\n");
		conn->encoder_id = ((uint32_t *)drm_to_ptr(conn->encoders_ptr))[0];
	}
	encoder->encoder_id = conn->encoder_id;

	if (ioctl(self->card_fd, DRM_IOCTL_MODE_GETENCODER, encoder) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_GETENCODER): %s\n", STRERR);
		return -1;
	}

	if (encoder->crtc_id == 0) {
		printf("encoder->crtc_id was 0, defaulting to crtc[0]\n");
		encoder->crtc_id = ((uint32_t *)drm_to_ptr(self->res->crtc_id_ptr))[0];
	}
	crtc->crtc_id = encoder->crtc_id;

	if (ioctl(self->card_fd, DRM_IOCTL_MODE_GETCRTC, crtc) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_GETCRTC): %s\n", STRERR);
		return -1;
	}

	/* set crtc mode */
	crtc->fb_id = self->sfb->fb_id;
	crtc->set_connectors_ptr = drm_from_ptr((void *)&conn->connector_id);
	crtc->count_connectors = 1;
	crtc->mode = *cur_mode;
	/*printf("\nsetting mode:\n\n");
	print_mode_modeinfo(cur_mode);*/
	crtc->mode_valid = 1;
	if (ioctl(self->card_fd, DRM_IOCTL_MODE_SETCRTC, crtc) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_SETCRTC): %s\n", STRERR);
		return -1;
	}
	return 0;
}

/* stupid frame buffer */
static struct drm_buffer *alloc_sfb(int card_fd,
			     uint32_t width,
			     uint32_t height,
			     uint32_t depth,
			     uint32_t bpp)
{
	struct drm_mode_create_dumb cdumb;
	struct drm_mode_map_dumb    moff;
	struct drm_mode_fb_cmd      cmd;
	struct drm_buffer *ret;
	void  *fbmap;

	memset(&cdumb, 0, sizeof(cdumb));
	memset(&moff,  0, sizeof(moff));
	memset(&cmd,   0, sizeof(cmd));

	/* create dumb buffer */
	cdumb.width  = width;
	cdumb.height = height;
	cdumb.bpp    = bpp;
	cdumb.flags  = 0;
	cdumb.pitch  = 0;
	cdumb.size   = 0;
	cdumb.handle = 0;
	if (ioctl(card_fd, DRM_IOCTL_MODE_CREATE_DUMB, &cdumb) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_CREATE_DUMB): %s\n", STRERR);
		return NULL;
	}
	/* add framebuffer object */
	cmd.width  = cdumb.width;
	cmd.height = cdumb.height;
	cmd.bpp    = cdumb.bpp;
	cmd.pitch  = cdumb.pitch;
	cmd.depth  = depth;
	cmd.handle = cdumb.handle;
	if (ioctl(card_fd, DRM_IOCTL_MODE_ADDFB, &cmd) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_ADDFB): %s\n", STRERR);
		ioctl(card_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &cdumb.handle);
		return NULL;
	}
	/* get mmap offset */
	moff.handle = cdumb.handle;
	if (ioctl(card_fd, DRM_IOCTL_MODE_MAP_DUMB, &moff) == -1) {
		printf("ioctl(DRM_IOCTL_MODE_MAP_DUMB): %s\n", STRERR);
		ioctl(card_fd, DRM_IOCTL_MODE_RMFB, &cmd.fb_id);
		ioctl(card_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &cdumb.handle);
		return NULL;
	}
	/* XXX this is probably better off as MAP_PRIVATE, we can't prime
	 * the main framebuffer if it's "dumb", AFAIK */
	fbmap = mmap(0, (size_t)cdumb.size, PROT_READ|PROT_WRITE,
			MAP_SHARED, card_fd, (off_t)moff.offset);
	if (fbmap == MAP_FAILED) {
		printf("framebuffer mmap failed: %s\n", STRERR);
		ioctl(card_fd, DRM_IOCTL_MODE_RMFB, &cmd.fb_id);
		ioctl(card_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &cdumb.handle);
		return NULL;
	}

	ret = calloc(1, sizeof(struct drm_buffer));
	if (!ret) {
		printf("-ENOMEM\n");
		munmap(fbmap, cdumb.size);
		ioctl(card_fd, DRM_IOCTL_MODE_RMFB, &cmd.fb_id);
		ioctl(card_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &cdumb.handle);
		return NULL;
	}
	ret->addr     = fbmap;
	ret->size     = cdumb.size;
	ret->pitch    = cdumb.pitch;
	ret->width    = cdumb.width;
	ret->height   = cdumb.height;
	ret->bpp      = cdumb.bpp;
	ret->depth    = cmd.depth;
	ret->fb_id    = cmd.fb_id;
	ret->drm_id   = cdumb.handle;
	memset(fbmap, 0x27, cdumb.size);
	return ret;
}

static int destroy_sfb(int card_fd, struct drm_buffer *sfb)
{
	if (!sfb)
		return -1;

	if (munmap(sfb->addr, sfb->size) == -1)
		printf("munmap: %s\n", STRERR);
	if (ioctl(card_fd, DRM_IOCTL_MODE_RMFB, &sfb->fb_id))
		printf("ioctl(DRM_IOCTL_MODE_RMFB): %s\n", STRERR);
	if (ioctl(card_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &sfb->drm_id))
		printf("ioctl(DRM_IOCTL_MODE_DESTROY_DUMB): %s\n", STRERR);
	free(sfb);
	return 0;
}
static int card_set_master(int card_fd)
{
	if (ioctl(card_fd, DRM_IOCTL_SET_MASTER, 0)) {
		printf("ioctl(DRM_IOCTL_SET_MASTER, 0): %s\n", STRERR);
		return -1;
	}
	return 0;
}
static int card_drop_master(int card_fd)
{
	if (ioctl(card_fd, DRM_IOCTL_DROP_MASTER, 0)) {
		printf("ioctl(DRM_IOCTL_DROP_MASTER, 0): %s\n", STRERR);
		return -1;
	}
	return 0;
}
static int drm_display_destroy(struct drm_display *display)
{
	if (display->conn)
		free_connector(display->conn);
	memset(display, 0, sizeof(struct drm_display));
	return 0;
}
int drm_kms_destroy(struct drm_kms *self)
{
	if (self->sfb)
		destroy_sfb(self->card_fd, self->sfb);
	if (self->res)
		free_mode_card_res(self->res);
	drm_display_destroy(&self->display);

	close(self->card_fd);
	memset(self, 0, sizeof(struct drm_kms));
	free(self);
	return 0;
}
static int get_mode_idx(struct drm_mode_modeinfo *modes,
			uint16_t count,
			uint16_t width,
			uint16_t height,
			uint16_t refresh)
{
	int i;
	int pick = -1;
	if (width == 0)
		width = 0xffff;
	if (height == 0)
		height = 0xffff;
	for (i = 0; i < count; ++i)
	{
		if (modes[i].hdisplay > width || modes[i].vdisplay > height)
			continue;
		/* pretend these radical modes don't exist for now */
		if (modes[i].hdisplay % 16 == 0) {
			if (pick < 0) {
				pick = i;
				continue;
			}
			if (modes[i].hdisplay > modes[pick].hdisplay)
				pick = i;
			else if (modes[i].vdisplay > modes[pick].vdisplay)
				pick = i;
			else if (modes[i].hdisplay == modes[pick].hdisplay
					&& modes[i].vdisplay == modes[pick].vdisplay) {
				if (abs(refresh - modes[i].vrefresh)
					  < abs(refresh - modes[pick].vrefresh)) {
					pick = i;
				}
			}
		}
	}
	if (pick < 0) {
		printf("could not find any usable modes for (%dx%d@%dhz)\n",
				width, height, refresh);
		return -1;
	}
	return pick;
}
/* TODO handle hotplugging */
static int drm_display_load(struct drm_kms *self,
		     uint16_t req_width,
		     uint16_t req_height,
		     uint16_t req_refresh,
		     struct drm_display *out)
{
	uint32_t conn_id;
	int idx = -1;

	/* FIXME uses primary connector? "0" */
	conn_id = drm_get_id(self->res->connector_id_ptr, 0);
	out->conn = alloc_connector(self->card_fd, conn_id);
	if (!out->conn) {
		printf("unable to create drm connector structure\n");
		return -1;
	}

	out->conn_id = conn_id;
	out->modes = get_connector_modeinfo(out->conn, &out->mode_count);
	idx = get_mode_idx(out->modes, out->mode_count,
			   req_width, req_height, req_refresh);
	if (idx < 0)
		goto free_err;

	out->cur_mode_idx = (uint32_t)idx;
	out->cur_mode = &out->modes[out->cur_mode_idx];
	return 0;
free_err:
	drm_display_destroy(out);
	return -1;
}
struct drm_kms *drm_mode_create(char *devname,
				int no_connect,
				uint16_t req_width,
				uint16_t req_height,
				uint16_t req_refresh)
{
	char devpath[128];
	struct drm_kms *self;
	struct drm_mode_modeinfo *cur_mode;
	int card_fd;

	snprintf(devpath, sizeof(devpath), "/dev/dri/%s", devname);
	card_fd = open(devpath, O_RDWR|O_CLOEXEC);
	if (card_fd == -1) {
		printf("open(%s): %s\n", devpath, STRERR);
		return NULL;
	}
	if (card_set_master(card_fd)) {
		printf("card_set_master failed\n");
		return NULL;
	}

	self = calloc(1, sizeof(struct drm_kms));
	if (!self)
		return NULL;

	self->card_fd = card_fd;
	self->res = alloc_mode_card_res(card_fd);
	if (!self->res) {
		printf("unable to create drm structure\n");
		goto free_err;
	}

	if (drm_display_load(self, req_width, req_height, req_refresh, &self->display)) {
		printf("drm_display_load failed\n");
		goto free_err;
	}
	cur_mode = self->display.cur_mode;
	printf("connector(%d) using mode[%d] (%dx%d@%dhz)\n",
				self->display.conn_id,
				self->display.cur_mode_idx,
				cur_mode->hdisplay,
				cur_mode->vdisplay,
				cur_mode->vrefresh);

	/* buffer pitch must divide evenly by 16,
	 * TODO check against bpp here when that is variable instead of 32 */
	self->sfb = alloc_sfb(card_fd, cur_mode->hdisplay, cur_mode->vdisplay, 24, 32);
	if (!self->sfb) {
		printf("alloc_sfb failed\n");
		goto free_err;
	}

	if (!no_connect && drm_kms_connect_sfb(self)) {
		printf("drm_kms_connect_sfb failed\n");
		goto free_err;
	}
	return self;

free_err:
	drm_kms_destroy(self);
	return NULL;
}


int main(int argc, char *argv[])
{
	int ret = -1;
	struct drm_kms *card0;
	/*card0 = drm_mode_create("card0", g_srv_opts.inactive_vt,
					   g_srv_opts.request_width,
					   g_srv_opts.request_height,
					   g_srv_opts.request_refresh);*/
	/* do not connect to vt */
	card0 = drm_mode_create("card0", 1, 640, 480, 60);
	if (card0 == NULL) {
		printf("drm_mode_create failed\n");
		return -1;
	}


	drm_kms_destroy(card0);

	printf("looks ok, returning 0\n");
	return 0;
}

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2018-10-21 16:25 (unknown), Michael Tirado
@ 2018-10-22  0:26 ` Dave Airlie
  2018-10-21 20:23   ` Re: Michael Tirado
  0 siblings, 1 reply; 62+ messages in thread
From: Dave Airlie @ 2018-10-22  0:26 UTC (permalink / raw)
  To: mtirado418
  Cc: Dave Airlie, LKML, dri-devel, Hongbo.He, Gerd Hoffmann,
	Deucher, Alexander, Sean Paul, Koenig, Christian

On Mon, 22 Oct 2018 at 07:22, Michael Tirado <mtirado418@gmail.com> wrote:
>
> Mapping a drm "dumb" buffer fails on 32-bit system (i686) from what
> appears to be a truncated memory address that has been copied
> throughout several files. The bug manifests as an -EINVAL when calling
> mmap with the offset gathered from DRM_IOCTL_MODE_MAP_DUMB <--
> DRM_IOCTL_MODE_ADDFB <-- DRM_IOCTL_MODE_CREATE_DUMB.  I can provide
> test code if needed.
>
> The following patch will apply to 4.18 though I've only been able to
> test through qemu bochs driver and nouveau. Intel driver worked
> without any issues.  I'm not sure if everyone is going to want to
> share a constant, and the whitespace is screwed up from gmail's awful
> javascript client, so let me know if I should resend this with any
> specific changes.  I have also attached the file with preserved
> whitespace.
>

This shouldn't be necessary, did someone misbackport the mmap changes without:

drm: set FMODE_UNSIGNED_OFFSET for drm files

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2018-10-21 20:23   ` Re: Michael Tirado
@ 2018-10-22  1:50     ` Dave Airlie
  2018-10-21 22:20       ` Re: Michael Tirado
  2018-10-23  1:47       ` Re: Michael Tirado
  0 siblings, 2 replies; 62+ messages in thread
From: Dave Airlie @ 2018-10-22  1:50 UTC (permalink / raw)
  To: mtirado418
  Cc: Dave Airlie, LKML, dri-devel, Hongbo.He, Gerd Hoffmann,
	Deucher, Alexander, Sean Paul, Koenig, Christian

On Mon, 22 Oct 2018 at 10:49, Michael Tirado <mtirado418@gmail.com> wrote:
>
> On Mon, Oct 22, 2018 at 12:26 AM Dave Airlie <airlied@gmail.com> wrote:
> >
> > This shouldn't be necessary, did someone misbackport the mmap changes without:
> >
> > drm: set FMODE_UNSIGNED_OFFSET for drm files
> >
> > Dave.
>
> The latest kernel I have had to patch was a 4.18-rc6.  I'll try with a
> newer 4.19 and let you know if it decides to work.  If not I'll
> prepare a test case for demonstration on qemu-system-i386.

If you have custom userspace software, make sure it's using
AC_SYS_LARGEFILE or whatever the equivalant is in your build system.

64-bit file offsets are important.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2018-10-22  1:50     ` Re: Dave Airlie
  2018-10-21 22:20       ` Re: Michael Tirado
@ 2018-10-23  1:47       ` Michael Tirado
  2018-10-23  6:23         ` Re: Dave Airlie
  1 sibling, 1 reply; 62+ messages in thread
From: Michael Tirado @ 2018-10-23  1:47 UTC (permalink / raw)
  To: Dave Airlie, LKML, dri-devel

That preprocessor define worked but I'm still confused about this
DRM_FILE_PAGE_OFFSET thing.  Check out drivers/gpu/drm/drm_gem.c
right above drm_gem_init.

---

/*
 * We make up offsets for buffer objects so we can recognize them at
 * mmap time.
 */

/* pgoff in mmap is an unsigned long, so we need to make sure that
 * the faked up offset will fit
 */

#if BITS_PER_LONG == 64
#define DRM_FILE_PAGE_OFFSET_START ((0xFFFFFFFFUL >> PAGE_SHIFT) + 1)
#define DRM_FILE_PAGE_OFFSET_SIZE ((0xFFFFFFFFUL >> PAGE_SHIFT) * 16)
#else
#define DRM_FILE_PAGE_OFFSET_START ((0xFFFFFFFUL >> PAGE_SHIFT) + 1)
#define DRM_FILE_PAGE_OFFSET_SIZE ((0xFFFFFFFUL >> PAGE_SHIFT) * 16)
#endif


---

Why is having a 64-bit file offsets critical, causing -EINVAL on mmap?
What problems might be associated with using (0x10000000UL >>
PAGE_SHIFT) ?
On Mon, Oct 22, 2018 at 1:50 AM Dave Airlie <airlied@gmail.com> wrote:
>
> On Mon, 22 Oct 2018 at 10:49, Michael Tirado <mtirado418@gmail.com> wrote:
> >
> > On Mon, Oct 22, 2018 at 12:26 AM Dave Airlie <airlied@gmail.com> wrote:
> > >
> > > This shouldn't be necessary, did someone misbackport the mmap changes without:
> > >
> > > drm: set FMODE_UNSIGNED_OFFSET for drm files
> > >
> > > Dave.
> >
> > The latest kernel I have had to patch was a 4.18-rc6.  I'll try with a
> > newer 4.19 and let you know if it decides to work.  If not I'll
> > prepare a test case for demonstration on qemu-system-i386.
>
> If you have custom userspace software, make sure it's using
> AC_SYS_LARGEFILE or whatever the equivalant is in your build system.
>
> 64-bit file offsets are important.
>
> Dave.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2018-10-23  1:47       ` Re: Michael Tirado
@ 2018-10-23  6:23         ` Dave Airlie
  0 siblings, 0 replies; 62+ messages in thread
From: Dave Airlie @ 2018-10-23  6:23 UTC (permalink / raw)
  To: mtirado418; +Cc: LKML, dri-devel

On Tue, 23 Oct 2018 at 16:13, Michael Tirado <mtirado418@gmail.com> wrote:
>
> That preprocessor define worked but I'm still confused about this
> DRM_FILE_PAGE_OFFSET thing.  Check out drivers/gpu/drm/drm_gem.c
> right above drm_gem_init.
>
> ---
>
> /*
>  * We make up offsets for buffer objects so we can recognize them at
>  * mmap time.
>  */
>
> /* pgoff in mmap is an unsigned long, so we need to make sure that
>  * the faked up offset will fit
>  */
>
> #if BITS_PER_LONG == 64
> #define DRM_FILE_PAGE_OFFSET_START ((0xFFFFFFFFUL >> PAGE_SHIFT) + 1)
> #define DRM_FILE_PAGE_OFFSET_SIZE ((0xFFFFFFFFUL >> PAGE_SHIFT) * 16)
> #else
> #define DRM_FILE_PAGE_OFFSET_START ((0xFFFFFFFUL >> PAGE_SHIFT) + 1)
> #define DRM_FILE_PAGE_OFFSET_SIZE ((0xFFFFFFFUL >> PAGE_SHIFT) * 16)
> #endif
>
>
> ---
>
> Why is having a 64-bit file offsets critical, causing -EINVAL on mmap?
> What problems might be associated with using (0x10000000UL >>
> PAGE_SHIFT) ?

a) it finds people not using the correct userspace defines. mostly
libdrm should handle this,
and possibly mesa.

b) there used to be legacy maps below that address on older drivers,
so we decided to never put stuff in the first 32-bit range that they
could clash with.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
       [not found] <86d0ec$ae4ffc@fmsmga001.fm.intel.com>
@ 2020-02-26 12:08 ` Linus Walleij
  2020-02-26 14:34   ` Re: Ville Syrjälä
  0 siblings, 1 reply; 62+ messages in thread
From: Linus Walleij @ 2020-02-26 12:08 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Josh Wu, Bhuvanchandra DV, Neil Armstrong, nouveau,
	Guido Günther, Paul Kocialkowski,
	open list:DRM PANEL DRIVERS, Gustaf Lindström, Andrzej Hajda,
	Thierry Reding, Laurent Pinchart, Sam Ravnborg,
	Marian-Cristian Rotariu, Jagan Teki, Thomas Hellstrom,
	Joonyoung Shim, Jonathan Marek, Stefan Mavrodiev, Adam Ford,
	Jerry Han, VMware Graphics, Ben Skeggs, H. Nikolaus Schaller,
	Robert Chiras, Heiko Schocher, Icenowy Zheng, Jonas Karlman,
	intel-gfx, Alexandre Courbot, open list:ARM/Amlogic Meson...,
	Vincent Abriou, Andreas Pretzsch, Jernej Skrabec, Alex Gonzalez,
	Purism Kernel Team, Boris Brezillon, Seung-Woo Kim,
	Christoph Fritz, Kyungmin Park, Heiko Stuebner, Eugen Hristev,
	Giulio Benetti

On Wed, Feb 26, 2020 at 12:57 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Tue, Feb 25, 2020 at 10:52:25PM +0100, Linus Walleij wrote:

> > I have long suspected that a whole bunch of the "simple" displays
> > are not simple but contains a display controller and memory.
> > That means that the speed over the link to the display and
> > actual refresh rate on the actual display is asymmetric because
> > well we are just updating a RAM, the resolution just limits how
> > much data we are sending, the clock limits the speed on the
> > bus over to the RAM on the other side.
>
> IMO even in command mode mode->clock should probably be the actual
> dotclock used by the display. If there's another clock for the bus
> speed/etc. it should be stored somewhere else.

Good point. For the DSI panels we have the field hs_rate
for the HS clock in struct mipi_dsi_device which is based
on exactly this reasoning. And that is what I actually use for
setting the HS clock.

The problem is however that we in many cases have so
substandard documentation of these panels that we have
absolutely no idea about the dotclock. Maybe we should
just set it to 0 in these cases?

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2020-02-26 12:08 ` Re: Linus Walleij
@ 2020-02-26 14:34   ` Ville Syrjälä
  2020-02-26 14:56     ` Re: Linus Walleij
  0 siblings, 1 reply; 62+ messages in thread
From: Ville Syrjälä @ 2020-02-26 14:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Josh Wu, Bhuvanchandra DV, Neil Armstrong, nouveau,
	Guido Günther, Paul Kocialkowski,
	open list:DRM PANEL DRIVERS, Gustaf Lindström, Andrzej Hajda,
	Thierry Reding, Laurent Pinchart, Sam Ravnborg,
	Marian-Cristian Rotariu, Jagan Teki, Thomas Hellstrom,
	Joonyoung Shim, Jonathan Marek, Stefan Mavrodiev, Adam Ford,
	Jerry Han, VMware Graphics, Ben Skeggs, H. Nikolaus Schaller,
	Robert Chiras, Heiko Schocher, Icenowy Zheng, Jonas Karlman,
	intel-gfx, Alexandre Courbot, open list:ARM/Amlogic Meson...,
	Vincent Abriou, Andreas Pretzsch, Jernej Skrabec, Alex Gonzalez,
	Purism Kernel Team, Boris Brezillon, Seung-Woo Kim,
	Christoph Fritz, Kyungmin Park, Heiko Stuebner, Eugen Hristev,
	Giulio Benetti

On Wed, Feb 26, 2020 at 01:08:06PM +0100, Linus Walleij wrote:
> On Wed, Feb 26, 2020 at 12:57 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Tue, Feb 25, 2020 at 10:52:25PM +0100, Linus Walleij wrote:
> 
> > > I have long suspected that a whole bunch of the "simple" displays
> > > are not simple but contains a display controller and memory.
> > > That means that the speed over the link to the display and
> > > actual refresh rate on the actual display is asymmetric because
> > > well we are just updating a RAM, the resolution just limits how
> > > much data we are sending, the clock limits the speed on the
> > > bus over to the RAM on the other side.
> >
> > IMO even in command mode mode->clock should probably be the actual
> > dotclock used by the display. If there's another clock for the bus
> > speed/etc. it should be stored somewhere else.
> 
> Good point. For the DSI panels we have the field hs_rate
> for the HS clock in struct mipi_dsi_device which is based
> on exactly this reasoning. And that is what I actually use for
> setting the HS clock.
> 
> The problem is however that we in many cases have so
> substandard documentation of these panels that we have
> absolutely no idea about the dotclock. Maybe we should
> just set it to 0 in these cases?

Don't you always have a TE interrupt or something like that
available? Could just measure it from that if no better
information is available?

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2020-02-26 14:34   ` Re: Ville Syrjälä
@ 2020-02-26 14:56     ` Linus Walleij
  2020-02-26 15:08       ` Re: Ville Syrjälä
  0 siblings, 1 reply; 62+ messages in thread
From: Linus Walleij @ 2020-02-26 14:56 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Josh Wu, Bhuvanchandra DV, Neil Armstrong, nouveau,
	Guido Günther, Paul Kocialkowski,
	open list:DRM PANEL DRIVERS, Gustaf Lindström, Andrzej Hajda,
	Thierry Reding, Laurent Pinchart, Sam Ravnborg,
	Marian-Cristian Rotariu, Jagan Teki, Thomas Hellstrom,
	Joonyoung Shim, Jonathan Marek, Stefan Mavrodiev, Adam Ford,
	Jerry Han, VMware Graphics, Ben Skeggs, H. Nikolaus Schaller,
	Robert Chiras, Heiko Schocher, Icenowy Zheng, Jonas Karlman,
	intel-gfx, Alexandre Courbot, open list:ARM/Amlogic Meson...,
	Vincent Abriou, Andreas Pretzsch, Jernej Skrabec, Alex Gonzalez,
	Purism Kernel Team, Boris Brezillon, Seung-Woo Kim,
	Christoph Fritz, Kyungmin Park, Heiko Stuebner, Eugen Hristev,
	Giulio Benetti

On Wed, Feb 26, 2020 at 3:34 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Feb 26, 2020 at 01:08:06PM +0100, Linus Walleij wrote:
> > On Wed, Feb 26, 2020 at 12:57 PM Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > > On Tue, Feb 25, 2020 at 10:52:25PM +0100, Linus Walleij wrote:
> >
> > > > I have long suspected that a whole bunch of the "simple" displays
> > > > are not simple but contains a display controller and memory.
> > > > That means that the speed over the link to the display and
> > > > actual refresh rate on the actual display is asymmetric because
> > > > well we are just updating a RAM, the resolution just limits how
> > > > much data we are sending, the clock limits the speed on the
> > > > bus over to the RAM on the other side.
> > >
> > > IMO even in command mode mode->clock should probably be the actual
> > > dotclock used by the display. If there's another clock for the bus
> > > speed/etc. it should be stored somewhere else.
> >
> > Good point. For the DSI panels we have the field hs_rate
> > for the HS clock in struct mipi_dsi_device which is based
> > on exactly this reasoning. And that is what I actually use for
> > setting the HS clock.
> >
> > The problem is however that we in many cases have so
> > substandard documentation of these panels that we have
> > absolutely no idea about the dotclock. Maybe we should
> > just set it to 0 in these cases?
>
> Don't you always have a TE interrupt or something like that
> available? Could just measure it from that if no better
> information is available?

Yes and I did exactly that, so that is why this comment is in
the driver:

static const struct drm_display_mode sony_acx424akp_cmd_mode = {
(...)
        /*
         * Some desired refresh rate, experiments at the maximum "pixel"
         * clock speed (HS clock 420 MHz) yields around 117Hz.
         */
        .vrefresh = 60,

I got a review comment at the time saying 117 Hz was weird.
We didn't reach a proper conclusion on this:
https://lore.kernel.org/dri-devel/CACRpkdYW3YNPSNMY3A44GQn8DqK-n9TLvr7uipF7LM_DHZ5=Lg@mail.gmail.com/

Thierry wasn't sure if 60Hz was good or not, so I just had to
go with something.

We could calculate the resulting pixel clock for ~117 Hz with
this resolution and put that in the clock field but ... don't know
what is the best?

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2020-02-26 14:56     ` Re: Linus Walleij
@ 2020-02-26 15:08       ` Ville Syrjälä
  0 siblings, 0 replies; 62+ messages in thread
From: Ville Syrjälä @ 2020-02-26 15:08 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Josh Wu, Bhuvanchandra DV, Neil Armstrong, nouveau,
	Guido Günther, Paul Kocialkowski,
	open list:DRM PANEL DRIVERS, Gustaf Lindström, Andrzej Hajda,
	Thierry Reding, Laurent Pinchart, Sam Ravnborg,
	Marian-Cristian Rotariu, Jagan Teki, Thomas Hellstrom,
	Joonyoung Shim, Jonathan Marek, Stefan Mavrodiev, Adam Ford,
	Jerry Han, VMware Graphics, Ben Skeggs, H. Nikolaus Schaller,
	Robert Chiras, Heiko Schocher, Icenowy Zheng, Jonas Karlman,
	intel-gfx, Alexandre Courbot, open list:ARM/Amlogic Meson...,
	Vincent Abriou, Andreas Pretzsch, Jernej Skrabec, Alex Gonzalez,
	Purism Kernel Team, Boris Brezillon, Seung-Woo Kim,
	Christoph Fritz, Kyungmin Park, Heiko Stuebner, Eugen Hristev,
	Giulio Benetti

On Wed, Feb 26, 2020 at 03:56:36PM +0100, Linus Walleij wrote:
> On Wed, Feb 26, 2020 at 3:34 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Feb 26, 2020 at 01:08:06PM +0100, Linus Walleij wrote:
> > > On Wed, Feb 26, 2020 at 12:57 PM Ville Syrjälä
> > > <ville.syrjala@linux.intel.com> wrote:
> > > > On Tue, Feb 25, 2020 at 10:52:25PM +0100, Linus Walleij wrote:
> > >
> > > > > I have long suspected that a whole bunch of the "simple" displays
> > > > > are not simple but contains a display controller and memory.
> > > > > That means that the speed over the link to the display and
> > > > > actual refresh rate on the actual display is asymmetric because
> > > > > well we are just updating a RAM, the resolution just limits how
> > > > > much data we are sending, the clock limits the speed on the
> > > > > bus over to the RAM on the other side.
> > > >
> > > > IMO even in command mode mode->clock should probably be the actual
> > > > dotclock used by the display. If there's another clock for the bus
> > > > speed/etc. it should be stored somewhere else.
> > >
> > > Good point. For the DSI panels we have the field hs_rate
> > > for the HS clock in struct mipi_dsi_device which is based
> > > on exactly this reasoning. And that is what I actually use for
> > > setting the HS clock.
> > >
> > > The problem is however that we in many cases have so
> > > substandard documentation of these panels that we have
> > > absolutely no idea about the dotclock. Maybe we should
> > > just set it to 0 in these cases?
> >
> > Don't you always have a TE interrupt or something like that
> > available? Could just measure it from that if no better
> > information is available?
> 
> Yes and I did exactly that, so that is why this comment is in
> the driver:
> 
> static const struct drm_display_mode sony_acx424akp_cmd_mode = {
> (...)
>         /*
>          * Some desired refresh rate, experiments at the maximum "pixel"
>          * clock speed (HS clock 420 MHz) yields around 117Hz.
>          */
>         .vrefresh = 60,
> 
> I got a review comment at the time saying 117 Hz was weird.
> We didn't reach a proper conclusion on this:
> https://lore.kernel.org/dri-devel/CACRpkdYW3YNPSNMY3A44GQn8DqK-n9TLvr7uipF7LM_DHZ5=Lg@mail.gmail.com/
> 
> Thierry wasn't sure if 60Hz was good or not, so I just had to
> go with something.
> 
> We could calculate the resulting pixel clock for ~117 Hz with
> this resolution and put that in the clock field but ... don't know
> what is the best?

I would vote for that approach.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2020-09-15  2:40 Dave Airlie
@ 2020-09-15  7:53 ` Christian König
  0 siblings, 0 replies; 62+ messages in thread
From: Christian König @ 2020-09-15  7:53 UTC (permalink / raw)
  To: dri-devel; +Cc: bskeggs

Reviewed-by: Christian König <christian.koenig@amd.com> for patches #1, 
#3 and #5-#7.

Minor comments on the other two.

Christian.

Am 15.09.20 um 04:40 schrieb Dave Airlie:
> The goal here is to make the ttm_tt object just represent a
> memory backing store, and now whether the store is bound to a
> global translation table. It moves binding up to the bo level.
>
> There's a lot more work on removing the global TT from the core
> of TTM, but this seems like a good start.
>
> Dave.
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
       [not found] <20201008181606.460499-1-sandy.8925@gmail.com>
@ 2020-10-09  6:47 ` Thomas Zimmermann
  2020-10-09  7:14   ` Re: Thomas Zimmermann
  0 siblings, 1 reply; 62+ messages in thread
From: Thomas Zimmermann @ 2020-10-09  6:47 UTC (permalink / raw)
  To: sandy.8925, alexander.deucher; +Cc: dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 559 bytes --]

NACK for the entire lack of any form of commit description.

Am 08.10.20 um 20:16 schrieb sandy.8925@gmail.com:
> Signed-off-by: Sandeep Raghuraman <sandy.8925@gmail.com>
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Re:
  2020-10-09  6:47 ` Re: Thomas Zimmermann
@ 2020-10-09  7:14   ` Thomas Zimmermann
  2020-10-09  7:38     ` Re: Sandeep Raghuraman
  0 siblings, 1 reply; 62+ messages in thread
From: Thomas Zimmermann @ 2020-10-09  7:14 UTC (permalink / raw)
  To: sandy.8925, alexander.deucher; +Cc: dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1038 bytes --]

Hi

Am 09.10.20 um 08:47 schrieb Thomas Zimmermann:
> NACK for the entire lack of any form of commit description.

Please see the documentation at [1] on how to describe the changes and
getting your patches merged.

Best regards
Thomas

[1]
https://dri.freedesktop.org/docs/drm/process/submitting-patches.html#describe-your-changes

> 
> Am 08.10.20 um 20:16 schrieb sandy.8925@gmail.com:
>> Signed-off-by: Sandeep Raghuraman <sandy.8925@gmail.com>
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2020-10-09  7:14   ` Re: Thomas Zimmermann
@ 2020-10-09  7:38     ` Sandeep Raghuraman
  2020-10-09  7:51       ` Re: Thomas Zimmermann
  0 siblings, 1 reply; 62+ messages in thread
From: Sandeep Raghuraman @ 2020-10-09  7:38 UTC (permalink / raw)
  To: Thomas Zimmermann, alexander.deucher; +Cc: dri-devel



On 10/9/20 12:44 PM, Thomas Zimmermann wrote:
> Hi
> 
> Am 09.10.20 um 08:47 schrieb Thomas Zimmermann:
>> NACK for the entire lack of any form of commit description.
> 
> Please see the documentation at [1] on how to describe the changes and
> getting your patches merged.

Yes, I tried to use git send-email to send patches this time, and it resulted in this disaster. I'll stick to sending them through Thunderbird.

> 
> Best regards
> Thomas
> 
> [1]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2020-10-09  7:38     ` Re: Sandeep Raghuraman
@ 2020-10-09  7:51       ` Thomas Zimmermann
  2020-10-09 15:48         ` Re: Alex Deucher
  0 siblings, 1 reply; 62+ messages in thread
From: Thomas Zimmermann @ 2020-10-09  7:51 UTC (permalink / raw)
  To: Sandeep Raghuraman, alexander.deucher; +Cc: dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1328 bytes --]

Hi

Am 09.10.20 um 09:38 schrieb Sandeep Raghuraman:
> 
> 
> On 10/9/20 12:44 PM, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 09.10.20 um 08:47 schrieb Thomas Zimmermann:
>>> NACK for the entire lack of any form of commit description.
>>
>> Please see the documentation at [1] on how to describe the changes and
>> getting your patches merged.
> 
> Yes, I tried to use git send-email to send patches this time, and it resulted in this disaster. I'll stick to sending them through Thunderbird.

What's the problem with send-email?

A typical call for your patchset would look like

  git send-mail <upstream-branch>...HEAD --cover-letter --annotate
--to=... --cc=...

That allows you to write the cover letter and have it sent out. IIRC you
need ot set $EDITOR to your favorite text editor; and configure the SMTP
server in ~/.gitconfig, under [sendemail].

Best regards
Thomas

> 
>>
>> Best regards
>> Thomas
>>
>> [1]
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2020-10-09  7:51       ` Re: Thomas Zimmermann
@ 2020-10-09 15:48         ` Alex Deucher
  0 siblings, 0 replies; 62+ messages in thread
From: Alex Deucher @ 2020-10-09 15:48 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Deucher, Alexander, Sandeep Raghuraman,
	Maling list - DRI developers

On Fri, Oct 9, 2020 at 3:51 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 09.10.20 um 09:38 schrieb Sandeep Raghuraman:
> >
> >
> > On 10/9/20 12:44 PM, Thomas Zimmermann wrote:
> >> Hi
> >>
> >> Am 09.10.20 um 08:47 schrieb Thomas Zimmermann:
> >>> NACK for the entire lack of any form of commit description.
> >>
> >> Please see the documentation at [1] on how to describe the changes and
> >> getting your patches merged.
> >
> > Yes, I tried to use git send-email to send patches this time, and it resulted in this disaster. I'll stick to sending them through Thunderbird.
>
> What's the problem with send-email?
>
> A typical call for your patchset would look like
>
>   git send-mail <upstream-branch>...HEAD --cover-letter --annotate
> --to=... --cc=...
>
> That allows you to write the cover letter and have it sent out. IIRC you
> need ot set $EDITOR to your favorite text editor; and configure the SMTP
> server in ~/.gitconfig, under [sendemail].
>

You can also do `git format-patch -3 --cover-letter` and manually edit
the coverletter as needed then send them with git send-email.

Alex

> Best regards
> Thomas
>
> >
> >>
> >> Best regards
> >> Thomas
> >>
> >> [1]
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2021-05-15 22:57 Dmitry Baryshkov
@ 2021-06-02 21:45 ` Dmitry Baryshkov
  0 siblings, 0 replies; 62+ messages in thread
From: Dmitry Baryshkov @ 2021-06-02 21:45 UTC (permalink / raw)
  To: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar
  Cc: Jonathan Marek, Stephen Boyd, linux-arm-msm, dri-devel,
	David Airlie, freedreno

On 16/05/2021 01:57, Dmitry Baryshkov wrote:
>  From Dmitry Baryshkov <dmitry.baryshkov@linaro.org> # This line is ignored.
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reply-To:
> Subject: [PATCH v2 0/6] drm/msm/dpu: simplify RM code
> In-Reply-To:
> 
> There is no need to request most of hardware blocks through the resource
> manager (RM), since typically there is 1:1 or N:1 relationship between
> corresponding blocks. Each LM is tied to the single PP. Each MERGE_3D
> can be used by the specified pair of PPs.  Each DSPP is also tied to
> single LM. So instead of allocating them through the RM, get them via
> static configuration.
> 
> Depends on: https://lore.kernel.org/linux-arm-msm/20210515190909.1809050-1-dmitry.baryshkov@linaro.org
> 
> Changes since v1:
>   - Split into separate patch series to ease review.

Another gracious ping, now for this series.

I want to send next version with minor changes, but I'd like to hear 
your overall opinion before doing that.

> 
> ----------------------------------------------------------------
> Dmitry Baryshkov (6):
>        drm/msm/dpu: get DSPP blocks directly rather than through RM
>        drm/msm/dpu: get MERGE_3D blocks directly rather than through RM
>        drm/msm/dpu: get PINGPONG blocks directly rather than through RM
>        drm/msm/dpu: get INTF blocks directly rather than through RM
>        drm/msm/dpu: drop unused lm_max_width from RM
>        drm/msm/dpu: simplify peer LM handling
> 
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  54 +---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h        |   8 -
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |   5 -
>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |   8 -
>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |   8 -
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     |   2 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |   4 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c          |  14 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h          |   7 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c    |   7 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h    |   4 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |  53 +++-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h            |   5 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             | 310 ++-------------------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h             |  18 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h          |   9 +-
>   16 files changed, 115 insertions(+), 401 deletions(-)
> 
> 


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
       [not found] <CAGsV3ysM+p_HAq+LgOe4db09e+zRtvELHUQzCjF8FVE2UF+3Ow@mail.gmail.com>
@ 2021-06-29 13:52 ` Alex Deucher
  0 siblings, 0 replies; 62+ messages in thread
From: Alex Deucher @ 2021-06-29 13:52 UTC (permalink / raw)
  To: shashank singh; +Cc: Maling list - DRI developers

Yes, please see this page for more information:
https://www.x.org/wiki/XorgEVoC/

Alex

On Mon, Jun 21, 2021 at 2:26 PM shashank singh
<shashanksingh819@gmail.com> wrote:
>
> Hello everyone, my name is Shashank Singh. I hope this is the right platform to reach out to the 'X.org' community. I was curious about the X.org Endless Vacation of Code. Is this program still active?
>
>

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2022-04-06  7:51 Christian König
@ 2022-04-06 12:59 ` Daniel Vetter
  0 siblings, 0 replies; 62+ messages in thread
From: Daniel Vetter @ 2022-04-06 12:59 UTC (permalink / raw)
  To: DMA-resvusage; +Cc: daniel.vetter, dri-devel

On Wed, Apr 06, 2022 at 09:51:16AM +0200, Christian König wrote:
> Hi Daniel,
> 
> rebased on top of all the changes in drm-misc-next now and hopefully
> ready for 5.19.
> 
> I think I addressed all concern, but there was a bunch of rebase fallout
> from i915, so better to double check that once more.

No idea what you managed to do with this series, but
- cover letter isn't showing up in archives
- you have Reply-To: DMA-resvusage sprinkled all over, which means my
  replies are bouncing in funny ways

Please fix for next time around.

Also the split up patches lack a bit cc.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2022-05-19  9:54 Christian König
@ 2022-05-19 10:50 ` Matthew Auld
  2022-05-20  7:11   ` Re: Christian König
  0 siblings, 1 reply; 62+ messages in thread
From: Matthew Auld @ 2022-05-19 10:50 UTC (permalink / raw)
  To: Christian König; +Cc: Intel Graphics Development, ML dri-devel

On Thu, 19 May 2022 at 10:55, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Just sending that out once more to intel-gfx to let the CI systems take
> a look.

If all went well it should normally appear at [1][2], if CI was able
to pick up the series.

Since it's not currently there, I assume it's temporarily stuck in the
moderation queue, assuming you are not subscribed to intel-gfx ml? If
so, perhaps consider subscribing at [3] and then disable receiving any
mail from the ml, so you get full use of CI without getting spammed.

[1] https://intel-gfx-ci.01.org/queue/index.html
[2] https://patchwork.freedesktop.org/project/intel-gfx/series/
[3] https://lists.freedesktop.org/mailman/listinfo/intel-gfx

>
> No functional change compared to the last version.
>
> Christian.
>
>

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2022-05-19 10:50 ` Matthew Auld
@ 2022-05-20  7:11   ` Christian König
  0 siblings, 0 replies; 62+ messages in thread
From: Christian König @ 2022-05-20  7:11 UTC (permalink / raw)
  To: Matthew Auld; +Cc: Intel Graphics Development, ML dri-devel

Am 19.05.22 um 12:50 schrieb Matthew Auld:
> On Thu, 19 May 2022 at 10:55, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Just sending that out once more to intel-gfx to let the CI systems take
>> a look.
> If all went well it should normally appear at [1][2], if CI was able
> to pick up the series.
>
> Since it's not currently there, I assume it's temporarily stuck in the
> moderation queue, assuming you are not subscribed to intel-gfx ml?

Ah! Well I am subscribed, just not with the e-Mail address I use to send 
out those patches.

Going to fix that ASAP!

Thanks,
Christian.

>   If
> so, perhaps consider subscribing at [3] and then disable receiving any
> mail from the ml, so you get full use of CI without getting spammed.
>
> [1] https://intel-gfx-ci.01.org/queue/index.html
> [2] https://patchwork.freedesktop.org/project/intel-gfx/series/
> [3] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>> No functional change compared to the last version.
>>
>> Christian.
>>
>>


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2023-11-11  4:21 Andrew Worsley
@ 2023-11-11  8:22 ` Javier Martinez Canillas
  0 siblings, 0 replies; 62+ messages in thread
From: Javier Martinez Canillas @ 2023-11-11  8:22 UTC (permalink / raw)
  To: Andrew Worsley, Thomas Zimmermann, Maarten Lankhorst,
	Maxime Ripard, David Airlie, Daniel Vetter,
	open list:DRM DRIVER FOR FIRMWARE FRAMEBUFFERS, open list

Andrew Worsley <amworsley@gmail.com> writes:

Hello Andrew,

>    This patch fix's the failure of the frame buffer driver on my Asahi kernel
> which prevented X11 from starting on my Apple M1 laptop. It seems like a straight
> forward failure to follow the procedure described in drivers/video/aperture.c
> to remove the ealier driver. This patch is very simple and minimal. Very likely
> there may be better ways to fix this and very like there may be other drivers
> which have the same problem but I submit this so at least there is
> an interim fix for my problem.
>

Which partch? I think you forgot to include in your email?

>     Thanks
>
>     Andrew Worsley
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 1/3] drm/ttm: use apply_page_range instead of vmf_insert_pfn_prot
       [not found] <20250820143739.3422-1-christian.koenig@amd.com>
@ 2025-08-20 14:33 ` Christian König
  2025-08-20 14:33 ` [PATCH 2/3] drm/ttm: reapply increase ttm pre-fault value to PMD size" Christian König
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 62+ messages in thread
From: Christian König @ 2025-08-20 14:33 UTC (permalink / raw)
  To: intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, david, dave.hansen,
	luto, peterz

Thomas pointed out that i915 is using apply_page_range instead of
vm_insert_pfn_prot to circumvent the PAT lookup and generally speed up
the page fault handling.

I've thought I give it a try and measure how much this can improve
things and it turned that mapping a 1GiB buffer is now more than 4x times
faster than before.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 130 ++++++++++++++++----------------
 1 file changed, 64 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index a194db83421d..93764b166678 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -160,6 +160,38 @@ vm_fault_t ttm_bo_vm_reserve(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(ttm_bo_vm_reserve);
 
+/* State bag for calls to ttm_bo_vm_apply_cb */
+struct ttm_bo_vm_bag {
+	struct mm_struct		*mm;
+	struct ttm_buffer_object	*bo;
+	struct ttm_tt			*ttm;
+	unsigned long			page_offset;
+	pgprot_t			prot;
+};
+
+/* Callback to fill in a specific PTE */
+static int ttm_bo_vm_apply_cb(pte_t *pte, unsigned long addr, void *data)
+{
+        struct ttm_bo_vm_bag *bag = data;
+	struct ttm_buffer_object *bo = bag->bo;
+	unsigned long pfn;
+
+	if (bo->resource->bus.is_iomem) {
+		pfn = ttm_bo_io_mem_pfn(bo, bag->page_offset);
+	} else {
+		struct page *page = bag->ttm->pages[bag->page_offset];
+
+		if (unlikely(!page))
+			return -ENOMEM;
+		pfn = page_to_pfn(page);
+	}
+
+        /* Special PTE are not associated with any struct page */
+        set_pte_at(bag->mm, addr, pte, pte_mkspecial(pfn_pte(pfn, bag->prot)));
+	bag->page_offset++;
+	return 0;
+}
+
 /**
  * ttm_bo_vm_fault_reserved - TTM fault helper
  * @vmf: The struct vm_fault given as argument to the fault callback
@@ -183,101 +215,67 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
 				    pgoff_t num_prefault)
 {
 	struct vm_area_struct *vma = vmf->vma;
-	struct ttm_buffer_object *bo = vma->vm_private_data;
-	struct ttm_device *bdev = bo->bdev;
-	unsigned long page_offset;
-	unsigned long page_last;
-	unsigned long pfn;
-	struct ttm_tt *ttm = NULL;
-	struct page *page;
+	struct ttm_bo_vm_bag bag = {
+		.mm = vma->vm_mm,
+		.bo = vma->vm_private_data
+	};
+	unsigned long size;
+	vm_fault_t ret;
 	int err;
-	pgoff_t i;
-	vm_fault_t ret = VM_FAULT_NOPAGE;
-	unsigned long address = vmf->address;
 
 	/*
 	 * Wait for buffer data in transit, due to a pipelined
 	 * move.
 	 */
-	ret = ttm_bo_vm_fault_idle(bo, vmf);
+	ret = ttm_bo_vm_fault_idle(bag.bo, vmf);
 	if (unlikely(ret != 0))
 		return ret;
 
-	err = ttm_mem_io_reserve(bdev, bo->resource);
+	err = ttm_mem_io_reserve(bag.bo->bdev, bag.bo->resource);
 	if (unlikely(err != 0))
 		return VM_FAULT_SIGBUS;
 
-	page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
-		vma->vm_pgoff - drm_vma_node_start(&bo->base.vma_node);
-	page_last = vma_pages(vma) + vma->vm_pgoff -
-		drm_vma_node_start(&bo->base.vma_node);
-
-	if (unlikely(page_offset >= PFN_UP(bo->base.size)))
+	bag.page_offset = ((vmf->address - vma->vm_start) >> PAGE_SHIFT) +
+		vma->vm_pgoff - drm_vma_node_start(&bag.bo->base.vma_node);
+	if (unlikely(bag.page_offset >= PFN_UP(bag.bo->base.size)))
 		return VM_FAULT_SIGBUS;
 
-	prot = ttm_io_prot(bo, bo->resource, prot);
-	if (!bo->resource->bus.is_iomem) {
+	prot = ttm_io_prot(bag.bo, bag.bo->resource, prot);
+	if (!bag.bo->resource->bus.is_iomem) {
 		struct ttm_operation_ctx ctx = {
 			.interruptible = true,
 			.no_wait_gpu = false,
 			.force_alloc = true
 		};
 
-		ttm = bo->ttm;
-		err = ttm_bo_populate(bo, &ctx);
-		if (err) {
-			if (err == -EINTR || err == -ERESTARTSYS ||
-			    err == -EAGAIN)
-				return VM_FAULT_NOPAGE;
-
-			pr_debug("TTM fault hit %pe.\n", ERR_PTR(err));
-			return VM_FAULT_SIGBUS;
-		}
+		bag.ttm = bag.bo->ttm;
+		err = ttm_bo_populate(bag.bo, &ctx);
+		if (err)
+			goto error;
 	} else {
 		/* Iomem should not be marked encrypted */
 		prot = pgprot_decrypted(prot);
 	}
+	bag.prot = prot;
 
-	/*
-	 * Speculatively prefault a number of pages. Only error on
-	 * first page.
-	 */
-	for (i = 0; i < num_prefault; ++i) {
-		if (bo->resource->bus.is_iomem) {
-			pfn = ttm_bo_io_mem_pfn(bo, page_offset);
-		} else {
-			page = ttm->pages[page_offset];
-			if (unlikely(!page && i == 0)) {
-				return VM_FAULT_OOM;
-			} else if (unlikely(!page)) {
-				break;
-			}
-			pfn = page_to_pfn(page);
-		}
+	/* Speculatively prefault a number of pages. */
+	size = min(num_prefault << PAGE_SHIFT, vma->vm_end - vmf->address);
+	err = apply_to_page_range(vma->vm_mm, vmf->address, size,
+				  ttm_bo_vm_apply_cb, &bag);
 
-		/*
-		 * Note that the value of @prot at this point may differ from
-		 * the value of @vma->vm_page_prot in the caching- and
-		 * encryption bits. This is because the exact location of the
-		 * data may not be known at mmap() time and may also change
-		 * at arbitrary times while the data is mmap'ed.
-		 * See vmf_insert_pfn_prot() for a discussion.
-		 */
-		ret = vmf_insert_pfn_prot(vma, address, pfn, prot);
+error:
+	if (err == -EINTR || err == -ERESTARTSYS || err == -EAGAIN)
+		return VM_FAULT_NOPAGE;
 
-		/* Never error on prefaulted PTEs */
-		if (unlikely((ret & VM_FAULT_ERROR))) {
-			if (i == 0)
-				return VM_FAULT_NOPAGE;
-			else
-				break;
-		}
+	if (err == -ENOMEM)
+		return VM_FAULT_OOM;
 
-		address += PAGE_SIZE;
-		if (unlikely(++page_offset >= page_last))
-			break;
+	if (err) {
+		pr_debug("TTM fault hit %pe.\n", ERR_PTR(err));
+		return VM_FAULT_SIGBUS;
 	}
-	return ret;
+
+	return VM_FAULT_NOPAGE;
 }
 EXPORT_SYMBOL(ttm_bo_vm_fault_reserved);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 2/3] drm/ttm: reapply increase ttm pre-fault value to PMD size"
       [not found] <20250820143739.3422-1-christian.koenig@amd.com>
  2025-08-20 14:33 ` [PATCH 1/3] drm/ttm: use apply_page_range instead of vmf_insert_pfn_prot Christian König
@ 2025-08-20 14:33 ` Christian König
  2025-08-20 14:33 ` [PATCH 3/3] drm/ttm: disable changing the global caching flags on newer AMD CPUs v2 Christian König
  2025-08-20 15:23 ` David Hildenbrand
  3 siblings, 0 replies; 62+ messages in thread
From: Christian König @ 2025-08-20 14:33 UTC (permalink / raw)
  To: intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, david, dave.hansen,
	luto, peterz

Now that we have improved the handling faulting in a full PMD only
increases the overhead on my test system from 21us to 29us if only a
single page is requested, but massively improves the performance for
all other use cases.

So re-apply that change again to improve the fault handling for large
allocations bringing us close to improving it by a factor of 10.

This reverts commit c358a809cb58af944d496944391a240e02f5837a.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 include/drm/ttm/ttm_bo.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 903cd1030110..e96477606207 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -39,7 +39,11 @@
 #include "ttm_device.h"
 
 /* Default number of pre-faulted pages in the TTM fault handler */
+#if CONFIG_PGTABLE_LEVELS > 2
+#define TTM_BO_VM_NUM_PREFAULT (1 << (PMD_SHIFT - PAGE_SHIFT))
+#else
 #define TTM_BO_VM_NUM_PREFAULT 16
+#endif
 
 struct iosys_map;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 3/3] drm/ttm: disable changing the global caching flags on newer AMD CPUs v2
       [not found] <20250820143739.3422-1-christian.koenig@amd.com>
  2025-08-20 14:33 ` [PATCH 1/3] drm/ttm: use apply_page_range instead of vmf_insert_pfn_prot Christian König
  2025-08-20 14:33 ` [PATCH 2/3] drm/ttm: reapply increase ttm pre-fault value to PMD size" Christian König
@ 2025-08-20 14:33 ` Christian König
  2025-08-20 15:12   ` Borislav Petkov
  2025-08-20 15:23 ` David Hildenbrand
  3 siblings, 1 reply; 62+ messages in thread
From: Christian König @ 2025-08-20 14:33 UTC (permalink / raw)
  To: intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, david, dave.hansen,
	luto, peterz

On some x86 systems (old AMD Athlons, Intel Luna Lake) we have the problem
that changing the caching flags of system memory requires changing the
global MTRR/PAT tables since those CPUs can't handle aliasing caching
attributes.

But on most modern x86 system (e.g. AMD CPUs after 2004) we actually
don't need that any more and can update the caching flags directly in the
PTEs of the userspace and kernel mappings.

We already do this with encryption on x86 64bit for quite a while and all
other supported platforms (Sparc, PowerPC, ARM, MIPS, LONGARCH) as well as
the i915 driver have never done anything different either.

So stop changing the global chaching flags for CPU systems which don't
need it and just insert a clflush to be on the safe side so that we never
return memory with dirty cache lines.

Testing on a Ryzen 5 and 7 shows that the clflush has absolutely no
performance impact, but I'm still waiting for CI systems to confirm
functional correctness.

v2: drop the pool only on AMD CPUs for now

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_pool.c | 37 +++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 83b10706ba89..3f830fb2aea5 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -45,6 +45,7 @@
 #include <drm/ttm/ttm_pool.h>
 #include <drm/ttm/ttm_tt.h>
 #include <drm/ttm/ttm_bo.h>
+#include <drm/drm_cache.h>
 
 #include "ttm_module.h"
 
@@ -119,6 +120,8 @@ module_param(page_pool_size, ulong, 0644);
 
 static atomic_long_t allocated_pages;
 
+static bool skip_caching_adjustment;
+
 static struct ttm_pool_type global_write_combined[NR_PAGE_ORDERS];
 static struct ttm_pool_type global_uncached[NR_PAGE_ORDERS];
 
@@ -195,7 +198,8 @@ static void ttm_pool_free_page(struct ttm_pool *pool, enum ttm_caching caching,
 	/* We don't care that set_pages_wb is inefficient here. This is only
 	 * used when we have to shrink and CPU overhead is irrelevant then.
 	 */
-	if (caching != ttm_cached && !PageHighMem(p))
+	if (!skip_caching_adjustment &&
+	    caching != ttm_cached && !PageHighMem(p))
 		set_pages_wb(p, 1 << order);
 #endif
 
@@ -223,13 +227,19 @@ static int ttm_pool_apply_caching(struct ttm_pool_alloc_state *alloc)
 	if (!num_pages)
 		return 0;
 
-	switch (alloc->tt_caching) {
-	case ttm_cached:
-		break;
-	case ttm_write_combined:
-		return set_pages_array_wc(alloc->caching_divide, num_pages);
-	case ttm_uncached:
-		return set_pages_array_uc(alloc->caching_divide, num_pages);
+	if (skip_caching_adjustment) {
+		drm_clflush_pages(alloc->caching_divide, num_pages);
+	} else {
+		switch (alloc->tt_caching) {
+		case ttm_cached:
+			break;
+		case ttm_write_combined:
+			return set_pages_array_wc(alloc->caching_divide,
+						  num_pages);
+		case ttm_uncached:
+			return set_pages_array_uc(alloc->caching_divide,
+						  num_pages);
+		}
 	}
 #endif
 	alloc->caching_divide = alloc->pages;
@@ -342,6 +352,9 @@ static struct ttm_pool_type *ttm_pool_select_type(struct ttm_pool *pool,
 		return &pool->caching[caching].orders[order];
 
 #ifdef CONFIG_X86
+	if (skip_caching_adjustment)
+		return NULL;
+
 	switch (caching) {
 	case ttm_write_combined:
 		if (pool->nid != NUMA_NO_NODE)
@@ -981,7 +994,7 @@ long ttm_pool_backup(struct ttm_pool *pool, struct ttm_tt *tt,
 
 #ifdef CONFIG_X86
 	/* Anything returned to the system needs to be cached. */
-	if (tt->caching != ttm_cached)
+	if (!skip_caching_adjustment && tt->caching != ttm_cached)
 		set_pages_array_wb(tt->pages, tt->num_pages);
 #endif
 
@@ -1296,6 +1309,12 @@ int ttm_pool_mgr_init(unsigned long num_pages)
 	spin_lock_init(&shrinker_lock);
 	INIT_LIST_HEAD(&shrinker_list);
 
+#ifdef CONFIG_X86
+	skip_caching_adjustment =
+		(boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
+		static_cpu_has(X86_FEATURE_CLFLUSH);
+#endif
+
 	for (i = 0; i < NR_PAGE_ORDERS; ++i) {
 		ttm_pool_type_init(&global_write_combined[i], NULL,
 				   ttm_write_combined, i);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/3] drm/ttm: disable changing the global caching flags on newer AMD CPUs v2
  2025-08-20 14:33 ` [PATCH 3/3] drm/ttm: disable changing the global caching flags on newer AMD CPUs v2 Christian König
@ 2025-08-20 15:12   ` Borislav Petkov
  0 siblings, 0 replies; 62+ messages in thread
From: Borislav Petkov @ 2025-08-20 15:12 UTC (permalink / raw)
  To: Christian König
  Cc: intel-xe, intel-gfx, dri-devel, amd-gfx, x86, airlied,
	thomas.hellstrom, matthew.brost, david, dave.hansen, luto, peterz

On Wed, Aug 20, 2025 at 04:33:13PM +0200, Christian König wrote:
> +#ifdef CONFIG_X86
> +	skip_caching_adjustment =
> +		(boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
> +		static_cpu_has(X86_FEATURE_CLFLUSH);

cpu_feature_enabled()

> +#endif

I'd prefer if this would call a function in arch/x86/ which tests those
instead of using x86 artifacts in drivers.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
       [not found] <20250820143739.3422-1-christian.koenig@amd.com>
                   ` (2 preceding siblings ...)
  2025-08-20 14:33 ` [PATCH 3/3] drm/ttm: disable changing the global caching flags on newer AMD CPUs v2 Christian König
@ 2025-08-20 15:23 ` David Hildenbrand
  2025-08-21  8:10   ` Re: Christian König
  2025-08-21  9:16   ` your mail Lorenzo Stoakes
  3 siblings, 2 replies; 62+ messages in thread
From: David Hildenbrand @ 2025-08-20 15:23 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

CCing Lorenzo

On 20.08.25 16:33, Christian König wrote:
> Hi everyone,
> 
> sorry for CCing so many people, but that rabbit hole turned out to be
> deeper than originally thought.
> 
> TTM always had problems with UC/WC mappings on 32bit systems and drivers
> often had to revert to hacks like using GFP_DMA32 to get things working
> while having no rational explanation why that helped (see the TTM AGP,
> radeon and nouveau driver code for that).
> 
> It turned out that the PAT implementation we use on x86 not only enforces
> the same caching attributes for pages in the linear kernel mapping, but
> also for highmem pages through a separate R/B tree.
> 
> That was unexpected and TTM never updated that R/B tree for highmem pages,
> so the function pgprot_set_cachemode() just overwrote the caching
> attributes drivers passed in to vmf_insert_pfn_prot() and that essentially
> caused all kind of random trouble.
> 
> An R/B tree is potentially not a good data structure to hold thousands if
> not millions of different attributes for each page, so updating that is
> probably not the way to solve this issue.
> 
> Thomas pointed out that the i915 driver is using apply_page_range()
> instead of vmf_insert_pfn_prot() to circumvent the PAT implementation and
> just fill in the page tables with what the driver things is the right
> caching attribute.

I assume you mean apply_to_page_range() -- same issue in patch subjects.

Oh this sounds horrible. Why oh why do we have these hacks in core-mm 
and have drivers abuse them :(

Honestly, apply_to_pte_range() is just the entry in doing all kinds of 
weird crap to page tables because "you know better".

All the sanity checks from vmf_insert_pfn(), gone.

Can we please fix the underlying issue properly?

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-20 15:23 ` David Hildenbrand
@ 2025-08-21  8:10   ` Christian König
  2025-08-25 19:10     ` Re: David Hildenbrand
  2025-08-21  9:16   ` your mail Lorenzo Stoakes
  1 sibling, 1 reply; 62+ messages in thread
From: Christian König @ 2025-08-21  8:10 UTC (permalink / raw)
  To: David Hildenbrand, intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 20.08.25 17:23, David Hildenbrand wrote:
> CCing Lorenzo
> 
> On 20.08.25 16:33, Christian König wrote:
>> Hi everyone,
>>
>> sorry for CCing so many people, but that rabbit hole turned out to be
>> deeper than originally thought.
>>
>> TTM always had problems with UC/WC mappings on 32bit systems and drivers
>> often had to revert to hacks like using GFP_DMA32 to get things working
>> while having no rational explanation why that helped (see the TTM AGP,
>> radeon and nouveau driver code for that).
>>
>> It turned out that the PAT implementation we use on x86 not only enforces
>> the same caching attributes for pages in the linear kernel mapping, but
>> also for highmem pages through a separate R/B tree.
>>
>> That was unexpected and TTM never updated that R/B tree for highmem pages,
>> so the function pgprot_set_cachemode() just overwrote the caching
>> attributes drivers passed in to vmf_insert_pfn_prot() and that essentially
>> caused all kind of random trouble.
>>
>> An R/B tree is potentially not a good data structure to hold thousands if
>> not millions of different attributes for each page, so updating that is
>> probably not the way to solve this issue.
>>
>> Thomas pointed out that the i915 driver is using apply_page_range()
>> instead of vmf_insert_pfn_prot() to circumvent the PAT implementation and
>> just fill in the page tables with what the driver things is the right
>> caching attribute.
> 
> I assume you mean apply_to_page_range() -- same issue in patch subjects.

Oh yes, of course. Sorry.

> Oh this sounds horrible. Why oh why do we have these hacks in core-mm and have drivers abuse them :(

Yeah I was also a bit hesitated to use that, but the performance advantage is so high that we probably can't avoid the general approach.

> Honestly, apply_to_pte_range() is just the entry in doing all kinds of weird crap to page tables because "you know better".

Exactly that's the problem I'm pointing out, drivers *do* know it better. The core memory management has applied incorrect values which caused all kind of the trouble.

The problem is not a bug in PAT nor TTM/drivers but rather how they interact with each other.

What I don't understand is why do we have the PAT in the first place? No other architecture does it this way.

Is that because of the of x86 CPUs which have problems when different page tables contain different caching attributes for the same physical memory?

> All the sanity checks from vmf_insert_pfn(), gone.
> 
> Can we please fix the underlying issue properly?

I'm happy to implement anything advised, my question is what should we solve this issue?

Regards,
Christian.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: your mail
  2025-08-20 15:23 ` David Hildenbrand
  2025-08-21  8:10   ` Re: Christian König
@ 2025-08-21  9:16   ` Lorenzo Stoakes
  2025-08-21  9:30     ` David Hildenbrand
  1 sibling, 1 reply; 62+ messages in thread
From: Lorenzo Stoakes @ 2025-08-21  9:16 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86, airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Liam Howlett

+cc Liam as he's also had some fun with PAT in the past.

On Wed, Aug 20, 2025 at 05:23:07PM +0200, David Hildenbrand wrote:
> CCing Lorenzo
>
> On 20.08.25 16:33, Christian König wrote:
> > Hi everyone,
> >
> > sorry for CCing so many people, but that rabbit hole turned out to be
> > deeper than originally thought.
> >
> > TTM always had problems with UC/WC mappings on 32bit systems and drivers
> > often had to revert to hacks like using GFP_DMA32 to get things working
> > while having no rational explanation why that helped (see the TTM AGP,
> > radeon and nouveau driver code for that).
> >
> > It turned out that the PAT implementation we use on x86 not only enforces
> > the same caching attributes for pages in the linear kernel mapping, but
> > also for highmem pages through a separate R/B tree.

Obviously this aspect is on the PAT guys.

PAT has caused some concerns for us in mm before, cf. David's series at [0].

[0]:https://lore.kernel.org/linux-mm/20250512123424.637989-1-david@redhat.com/

> >
> > That was unexpected and TTM never updated that R/B tree for highmem pages,
> > so the function pgprot_set_cachemode() just overwrote the caching
> > attributes drivers passed in to vmf_insert_pfn_prot() and that essentially
> > caused all kind of random trouble.
> >
> > An R/B tree is potentially not a good data structure to hold thousands if
> > not millions of different attributes for each page, so updating that is
> > probably not the way to solve this issue.
> >
> > Thomas pointed out that the i915 driver is using apply_page_range()
> > instead of vmf_insert_pfn_prot() to circumvent the PAT implementation and
> > just fill in the page tables with what the driver things is the right
> > caching attribute.
>
> I assume you mean apply_to_page_range() -- same issue in patch subjects.
>
> Oh this sounds horrible. Why oh why do we have these hacks in core-mm and
> have drivers abuse them :(

Yeah this is not intended behaviour and I actually think we should not permit
this at all. In fact I think we should un-export this.

I think the hold up with it is xen, as the only other users are arch code.

Probably we need to find a new interface just for xen and provide that just for
them...

>
> Honestly, apply_to_pte_range() is just the entry in doing all kinds of weird
> crap to page tables because "you know better".

Yes. This is just not permitted for general driver usage and is an abuse of the
mm API really. Esp. when the underlying issue is not to do with core mm...

>
> All the sanity checks from vmf_insert_pfn(), gone.
>
> Can we please fix the underlying issue properly?

Yes, PLEASE.

>
> --
> Cheers
>
> David / dhildenb
>

I will add this xen/apply_to_page_range() thing to my TODOs, which atm
would invovle changing these drivers to use vmf_insert_pfn_prot() instead.

So ideally we'll have addressed the underlying issue before I get to this,
because this really really shouldn't be something we allow drivers to use
generally.

Cheers, Lorenzo

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: your mail
  2025-08-21  9:16   ` your mail Lorenzo Stoakes
@ 2025-08-21  9:30     ` David Hildenbrand
  2025-08-21 10:05       ` Lorenzo Stoakes
  0 siblings, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-21  9:30 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86, airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Liam Howlett

> I will add this xen/apply_to_page_range() thing to my TODOs, which atm
> would invovle changing these drivers to use vmf_insert_pfn_prot() instead.
> 

Busy today (want to reply to Christian) but

a) Re: performance, we would want something like
    vmf_insert_pfns_prot(), similar to vm_insert_pages(), to bulk-insert
    multiple PFNs.

b) Re: PAT, we'll have to figure out why PAT information is wrong here
    (was there no previous PAT reservation from the driver?), but IF we
    really have to override, we'd want a way to tell
    vmf_insert_pfn_prot() to force the selected caching mode.

-- 
Cheers

David / dhildenb

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: your mail
  2025-08-21  9:30     ` David Hildenbrand
@ 2025-08-21 10:05       ` Lorenzo Stoakes
  2025-08-21 10:16         ` David Hildenbrand
  2025-08-25 18:35         ` Christian König
  0 siblings, 2 replies; 62+ messages in thread
From: Lorenzo Stoakes @ 2025-08-21 10:05 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86, airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Liam Howlett

On Thu, Aug 21, 2025 at 11:30:43AM +0200, David Hildenbrand wrote:
> > I will add this xen/apply_to_page_range() thing to my TODOs, which atm
> > would invovle changing these drivers to use vmf_insert_pfn_prot() instead.
> >
>
> Busy today (want to reply to Christian) but
>
> a) Re: performance, we would want something like
>    vmf_insert_pfns_prot(), similar to vm_insert_pages(), to bulk-insert
>    multiple PFNs.
>
> b) Re: PAT, we'll have to figure out why PAT information is wrong here
>    (was there no previous PAT reservation from the driver?), but IF we
>    really have to override, we'd want a way to tell
>    vmf_insert_pfn_prot() to force the selected caching mode.
>

Ack, ok good that we have a feasible way forward.

FYI, spoke to Peter off-list and he mentioned he had a more general series
to get rid of this kind of [ab]use of apply_to_page_range() (see [0]), I
gather he hasn't the time to resurrect but perhaps one of us can at some
point?

Perhaps we need a shorter term fix to _this_ issue (which involves not
using this interface), and then follow it up with an adaptation of the
below?

Cheers, Lorenzo

[0]:https://lore.kernel.org/all/20210412080012.357146277@infradead.org/



> --
> Cheers
>
> David / dhildenb

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: your mail
  2025-08-21 10:05       ` Lorenzo Stoakes
@ 2025-08-21 10:16         ` David Hildenbrand
  2025-08-25 18:35         ` Christian König
  1 sibling, 0 replies; 62+ messages in thread
From: David Hildenbrand @ 2025-08-21 10:16 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86, airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Liam Howlett

On 21.08.25 12:05, Lorenzo Stoakes wrote:
> On Thu, Aug 21, 2025 at 11:30:43AM +0200, David Hildenbrand wrote:
>>> I will add this xen/apply_to_page_range() thing to my TODOs, which atm
>>> would invovle changing these drivers to use vmf_insert_pfn_prot() instead.
>>>
>>
>> Busy today (want to reply to Christian) but
>>
>> a) Re: performance, we would want something like
>>     vmf_insert_pfns_prot(), similar to vm_insert_pages(), to bulk-insert
>>     multiple PFNs.
>>
>> b) Re: PAT, we'll have to figure out why PAT information is wrong here
>>     (was there no previous PAT reservation from the driver?), but IF we
>>     really have to override, we'd want a way to tell
>>     vmf_insert_pfn_prot() to force the selected caching mode.
>>
> 
> Ack, ok good that we have a feasible way forward.
> 
> FYI, spoke to Peter off-list and he mentioned he had a more general series
> to get rid of this kind of [ab]use of apply_to_page_range() (see [0]), I
> gather he hasn't the time to resurrect but perhaps one of us can at some
> point?
> 
> Perhaps we need a shorter term fix to _this_ issue (which involves not
> using this interface), and then follow it up with an adaptation of the
> below?

We need to understand why PAT would be wrong and why it would even be ok
to ignore it. Not hacking around it.

FWIW, I just recently documented:

+/**
+ * pfnmap_setup_cachemode - setup the cachemode in the pgprot for a pfn range
+ * @pfn: the start of the pfn range
+ * @size: the size of the pfn range in bytes
+ * @prot: the pgprot to modify
+ *
+ * Lookup the cachemode for the pfn range starting at @pfn with the size
+ * @size and store it in @prot, leaving other data in @prot unchanged.
+ *
+ * This allows for a hardware implementation to have fine-grained control of
+ * memory cache behavior at page level granularity. Without a hardware
+ * implementation, this function does nothing.
+ *
+ * Currently there is only one implementation for this - x86 Page Attribute
+ * Table (PAT). See Documentation/arch/x86/pat.rst for more details.
+ *
+ * This function can fail if the pfn range spans pfns that require differing
+ * cachemodes. If the pfn range was previously verified to have a single
+ * cachemode, it is sufficient to query only a single pfn. The assumption is
+ * that this is the case for drivers using the vmf_insert_pfn*() interface.
+ *
+ * Returns 0 on success and -EINVAL on error.
+ */
+int pfnmap_setup_cachemode(unsigned long pfn, unsigned long size,
+               pgprot_t *prot);
  extern int track_pfn_copy(struct vm_area_struct *dst_vma,
                 struct vm_area_struct *src_vma, unsigned long *pfn);
  extern void untrack_pfn_copy(struct vm_area_struct *dst_vma,
@@ -1563,6 +1584,21 @@ extern void untrack_pfn(struct vm_area_struct *vma, unsigned long pfn,
  extern void untrack_pfn_clear(struct vm_area_struct *vma);
  #endif
  
+/**
+ * pfnmap_setup_cachemode_pfn - setup the cachemode in the pgprot for a pfn
+ * @pfn: the pfn
+ * @prot: the pgprot to modify
+ *
+ * Lookup the cachemode for @pfn and store it in @prot, leaving other
+ * data in @prot unchanged.
+ *
+ * See pfnmap_setup_cachemode() for details.
+ */
+static inline void pfnmap_setup_cachemode_pfn(unsigned long pfn, pgprot_t *prot)
+{
+       pfnmap_setup_cachemode(pfn, PAGE_SIZE, prot);
+}


There is certainly something missing that the driver would have previously
verified that the cachemode is as expected.

-- 
Cheers

David / dhildenb

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: your mail
  2025-08-21 10:05       ` Lorenzo Stoakes
  2025-08-21 10:16         ` David Hildenbrand
@ 2025-08-25 18:35         ` Christian König
  2025-08-25 19:20           ` David Hildenbrand
  1 sibling, 1 reply; 62+ messages in thread
From: Christian König @ 2025-08-25 18:35 UTC (permalink / raw)
  To: Lorenzo Stoakes, David Hildenbrand
  Cc: intel-xe, intel-gfx, dri-devel, amd-gfx, x86, airlied,
	thomas.hellstrom, matthew.brost, dave.hansen, luto, peterz,
	Liam Howlett

On 21.08.25 12:05, Lorenzo Stoakes wrote:
> On Thu, Aug 21, 2025 at 11:30:43AM +0200, David Hildenbrand wrote:
>>> I will add this xen/apply_to_page_range() thing to my TODOs, which atm
>>> would invovle changing these drivers to use vmf_insert_pfn_prot() instead.
>>>
>>
>> Busy today (want to reply to Christian) but
>>
>> a) Re: performance, we would want something like
>>    vmf_insert_pfns_prot(), similar to vm_insert_pages(), to bulk-insert
>>    multiple PFNs.

Yes, exactly that. Ideally something like an iterator/callback like interface.

I've seen at least four or five different representations of the PFNs in drivers.

>> b) Re: PAT, we'll have to figure out why PAT information is wrong here
>>    (was there no previous PAT reservation from the driver?), but IF we
>>    really have to override, we'd want a way to tell
>>    vmf_insert_pfn_prot() to force the selected caching mode.
>>

Well the difference between vmf_insert_pfn() and vmf_insert_pfn_prot() is that the driver actually want to specify the caching modes.

That this is overridden by the PAT even for pages which are not part of the linear mapping is really surprising.

As far as I can see there is no technical necessity for that. Even for pages in the linear mapping only a handful of x86 CPUs actually need that. See Intels i915 GPU driver for reference.

Intel has used that approach for ages and for AMD CPUs the only reference I could find where the kernel needs it are Athlons produced between 1996 and 2004.

Maybe we should disable the PAT on CPUs which actually don't need it?

> Ack, ok good that we have a feasible way forward.
> 
> FYI, spoke to Peter off-list and he mentioned he had a more general series
> to get rid of this kind of [ab]use of apply_to_page_range() (see [0]), I
> gather he hasn't the time to resurrect but perhaps one of us can at some
> point?
> 
> Perhaps we need a shorter term fix to _this_ issue (which involves not
> using this interface), and then follow it up with an adaptation of the
> below?

Sounds like a plan to me.

Regards,
Christian.

> 
> Cheers, Lorenzo
> 
> [0]:https://lore.kernel.org/all/20210412080012.357146277@infradead.org/
> 
> 
> 
>> --
>> Cheers
>>
>> David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-21  8:10   ` Re: Christian König
@ 2025-08-25 19:10     ` David Hildenbrand
  2025-08-26  8:38       ` Re: Christian König
  0 siblings, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-25 19:10 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 21.08.25 10:10, Christian König wrote:
> On 20.08.25 17:23, David Hildenbrand wrote:
>> CCing Lorenzo
>>
>> On 20.08.25 16:33, Christian König wrote:
>>> Hi everyone,
>>>
>>> sorry for CCing so many people, but that rabbit hole turned out to be
>>> deeper than originally thought.
>>>
>>> TTM always had problems with UC/WC mappings on 32bit systems and drivers
>>> often had to revert to hacks like using GFP_DMA32 to get things working
>>> while having no rational explanation why that helped (see the TTM AGP,
>>> radeon and nouveau driver code for that).
>>>
>>> It turned out that the PAT implementation we use on x86 not only enforces
>>> the same caching attributes for pages in the linear kernel mapping, but
>>> also for highmem pages through a separate R/B tree.
>>>
>>> That was unexpected and TTM never updated that R/B tree for highmem pages,
>>> so the function pgprot_set_cachemode() just overwrote the caching
>>> attributes drivers passed in to vmf_insert_pfn_prot() and that essentially
>>> caused all kind of random trouble.
>>>
>>> An R/B tree is potentially not a good data structure to hold thousands if
>>> not millions of different attributes for each page, so updating that is
>>> probably not the way to solve this issue.
>>>
>>> Thomas pointed out that the i915 driver is using apply_page_range()
>>> instead of vmf_insert_pfn_prot() to circumvent the PAT implementation and
>>> just fill in the page tables with what the driver things is the right
>>> caching attribute.
>>
>> I assume you mean apply_to_page_range() -- same issue in patch subjects.
> 
> Oh yes, of course. Sorry.
> 
>> Oh this sounds horrible. Why oh why do we have these hacks in core-mm and have drivers abuse them :(
> 
> Yeah I was also a bit hesitated to use that, but the performance advantage is so high that we probably can't avoid the general approach.
> 
>> Honestly, apply_to_pte_range() is just the entry in doing all kinds of weird crap to page tables because "you know better".
> 
> Exactly that's the problem I'm pointing out, drivers *do* know it better. The core memory management has applied incorrect values which caused all kind of the trouble.
> 
> The problem is not a bug in PAT nor TTM/drivers but rather how they interact with each other.
> 
> What I don't understand is why do we have the PAT in the first place? No other architecture does it this way.

Probably because no other architecture has these weird glitches I assume 
... skimming over memtype_reserve() and friends there are quite some 
corner cases the code is handling (BIOS, ACPI, low ISA, system RAM, ...)


I did a lot of work on the higher PAT level functions, but I am no 
expert on the lower level management functions, and in particular all 
the special cases with different memory types.

IIRC, the goal of the PAT subsystem is to make sure that no two page 
tables map the same PFN with different caching attributes.

It treats ordinary system RAM (IORESOURCE_SYSTEM_RAM) usually in a 
special way: no special caching mode.

For everything else, it expects that someone first reserves a memory 
range for a specific caching mode.

For example, remap_pfn_range()...->pfnmap_track()->memtype_reserve() 
will make sure that there are no conflicts, to the call 
memtype_kernel_map_sync() to make sure the identity mapping is updated 
to the new type.

In case someone ends up calling pfnmap_setup_cachemode(), the 
expectation is that there was a previous call to memtype_reserve_io() or 
similar, such that pfnmap_setup_cachemode() will find that caching mode.


So my assumption would be that that is missing for the drivers here?

Last time I asked where this reservation is done, Peter Xu explained [1] 
it at least for VFIO:

vfio_pci_core_mmap
   pci_iomap
     pci_iomap_range
       ...
         __ioremap_caller
           memtype_reserve


Now, could it be that something like that is missing in these drivers 
(ioremap etc)?



[1] https://lkml.kernel.org/r/aBDXr-Qp4z0tS50P@x1.local


> 
> Is that because of the of x86 CPUs which have problems when different page tables contain different caching attributes for the same physical memory?

Yes, but I don't think x86 is special here.

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: your mail
  2025-08-25 18:35         ` Christian König
@ 2025-08-25 19:20           ` David Hildenbrand
  0 siblings, 0 replies; 62+ messages in thread
From: David Hildenbrand @ 2025-08-25 19:20 UTC (permalink / raw)
  To: Christian König, Lorenzo Stoakes
  Cc: intel-xe, intel-gfx, dri-devel, amd-gfx, x86, airlied,
	thomas.hellstrom, matthew.brost, dave.hansen, luto, peterz,
	Liam Howlett

On 25.08.25 20:35, Christian König wrote:
> On 21.08.25 12:05, Lorenzo Stoakes wrote:
>> On Thu, Aug 21, 2025 at 11:30:43AM +0200, David Hildenbrand wrote:
>>>> I will add this xen/apply_to_page_range() thing to my TODOs, which atm
>>>> would invovle changing these drivers to use vmf_insert_pfn_prot() instead.
>>>>
>>>
>>> Busy today (want to reply to Christian) but
>>>
>>> a) Re: performance, we would want something like
>>>     vmf_insert_pfns_prot(), similar to vm_insert_pages(), to bulk-insert
>>>     multiple PFNs.
> 
> Yes, exactly that. Ideally something like an iterator/callback like interface.
> 
> I've seen at least four or five different representations of the PFNs in drivers.
> 
>>> b) Re: PAT, we'll have to figure out why PAT information is wrong here
>>>     (was there no previous PAT reservation from the driver?), but IF we
>>>     really have to override, we'd want a way to tell
>>>     vmf_insert_pfn_prot() to force the selected caching mode.
>>>
> 
> Well the difference between vmf_insert_pfn() and vmf_insert_pfn_prot() is that the driver actually want to specify the caching modes.

Yes, it's all a mess. x86/PAT doesn't want inconsistencies, so it 
expects that a previous reservation would make sure that that caching 
mode is actually valid.

> 
> That this is overridden by the PAT even for pages which are not part of the linear mapping is really surprising.

Yes, IIUC, it expects an earlier reservation on PAT systems.

> 
> As far as I can see there is no technical necessity for that. Even for pages in the linear mapping only a handful of x86 CPUs actually need that. See Intels i915 GPU driver for reference.
> 
> Intel has used that approach for ages and for AMD CPUs the only reference I could find where the kernel needs it are Athlons produced between 1996 and 2004.
> 
> Maybe we should disable the PAT on CPUs which actually don't need it?

Not sure if that will solve our problems on systems that need it because 
of some devices.

I guess the problem of pfnmap_setup_cachemode_pfn() is that there is no 
interface to undo it: pfnmap_track() is pared with pfnmap_untrack() such 
that it can simply do/undo the reservation itself.

That's why pfnmap_setup_cachemode_pfn() leaves it up to the caller that 
a reservation was trigger earlier differently -- which can properly be 
undone.

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-25 19:10     ` Re: David Hildenbrand
@ 2025-08-26  8:38       ` Christian König
  2025-08-26  8:46         ` Re: David Hildenbrand
  2025-08-26 12:37         ` David Hildenbrand
  0 siblings, 2 replies; 62+ messages in thread
From: Christian König @ 2025-08-26  8:38 UTC (permalink / raw)
  To: David Hildenbrand, intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 25.08.25 21:10, David Hildenbrand wrote:
> On 21.08.25 10:10, Christian König wrote:
>> On 20.08.25 17:23, David Hildenbrand wrote:
>>> CCing Lorenzo
>>>
>>> On 20.08.25 16:33, Christian König wrote:
>>>> Hi everyone,
>>>>
>>>> sorry for CCing so many people, but that rabbit hole turned out to be
>>>> deeper than originally thought.
>>>>
>>>> TTM always had problems with UC/WC mappings on 32bit systems and drivers
>>>> often had to revert to hacks like using GFP_DMA32 to get things working
>>>> while having no rational explanation why that helped (see the TTM AGP,
>>>> radeon and nouveau driver code for that).
>>>>
>>>> It turned out that the PAT implementation we use on x86 not only enforces
>>>> the same caching attributes for pages in the linear kernel mapping, but
>>>> also for highmem pages through a separate R/B tree.
>>>>
>>>> That was unexpected and TTM never updated that R/B tree for highmem pages,
>>>> so the function pgprot_set_cachemode() just overwrote the caching
>>>> attributes drivers passed in to vmf_insert_pfn_prot() and that essentially
>>>> caused all kind of random trouble.
>>>>
>>>> An R/B tree is potentially not a good data structure to hold thousands if
>>>> not millions of different attributes for each page, so updating that is
>>>> probably not the way to solve this issue.
>>>>
>>>> Thomas pointed out that the i915 driver is using apply_page_range()
>>>> instead of vmf_insert_pfn_prot() to circumvent the PAT implementation and
>>>> just fill in the page tables with what the driver things is the right
>>>> caching attribute.
>>>
>>> I assume you mean apply_to_page_range() -- same issue in patch subjects.
>>
>> Oh yes, of course. Sorry.
>>
>>> Oh this sounds horrible. Why oh why do we have these hacks in core-mm and have drivers abuse them :(
>>
>> Yeah I was also a bit hesitated to use that, but the performance advantage is so high that we probably can't avoid the general approach.
>>
>>> Honestly, apply_to_pte_range() is just the entry in doing all kinds of weird crap to page tables because "you know better".
>>
>> Exactly that's the problem I'm pointing out, drivers *do* know it better. The core memory management has applied incorrect values which caused all kind of the trouble.
>>
>> The problem is not a bug in PAT nor TTM/drivers but rather how they interact with each other.
>>
>> What I don't understand is why do we have the PAT in the first place? No other architecture does it this way.
> 
> Probably because no other architecture has these weird glitches I assume ... skimming over memtype_reserve() and friends there are quite some corner cases the code is handling (BIOS, ACPI, low ISA, system RAM, ...)
> 
> 
> I did a lot of work on the higher PAT level functions, but I am no expert on the lower level management functions, and in particular all the special cases with different memory types.
> 
> IIRC, the goal of the PAT subsystem is to make sure that no two page tables map the same PFN with different caching attributes.

Yeah, that actually makes sense. Thomas from Intel recently explained the technical background to me:

Some x86 CPUs write back cache lines even if they aren't dirty and what can happen is that because of the linear mapping the CPU speculatively loads a cache line which is elsewhere mapped uncached.

So the end result is that the writeback of not dirty cache lines potentially corrupts the data in the otherwise uncached system memory.

But that a) only applies to memory in the linear mapping and b) only to a handful of x86 CPU types (e.g. recently Intels Luna Lake, AMD Athlons produced before 2004, maybe others).

> It treats ordinary system RAM (IORESOURCE_SYSTEM_RAM) usually in a special way: no special caching mode.
> 
> For everything else, it expects that someone first reserves a memory range for a specific caching mode.
> 
> For example, remap_pfn_range()...->pfnmap_track()->memtype_reserve() will make sure that there are no conflicts, to the call memtype_kernel_map_sync() to make sure the identity mapping is updated to the new type.
> 
> In case someone ends up calling pfnmap_setup_cachemode(), the expectation is that there was a previous call to memtype_reserve_io() or similar, such that pfnmap_setup_cachemode() will find that caching mode.
> 
> 
> So my assumption would be that that is missing for the drivers here?

Well yes and no.

See the PAT is optimized for applying specific caching attributes to ranges [A..B] (e.g. it uses an R/B tree). But what drivers do here is that they have single pages (usually for get_free_page or similar) and want to apply a certain caching attribute to it.

So what would happen is that we completely clutter the R/B tree used by the PAT with thousands if not millions of entries.

> 
> Last time I asked where this reservation is done, Peter Xu explained [1] it at least for VFIO:
> 
> vfio_pci_core_mmap
>   pci_iomap
>     pci_iomap_range
>       ...
>         __ioremap_caller
>           memtype_reserve
> 
> 
> Now, could it be that something like that is missing in these drivers (ioremap etc)?

Well that would solve the issue temporary, but I'm pretty sure that will just go boom at a different place then :(

One possibility would be to say that the PAT only overrides the attributes if they aren't normal cached and leaves everything else alone.

What do you think?

Thanks,
Christian.

> 
> 
> 
> [1] https://lkml.kernel.org/r/aBDXr-Qp4z0tS50P@x1.local
> 
> 
>>
>> Is that because of the of x86 CPUs which have problems when different page tables contain different caching attributes for the same physical memory?
> 
> Yes, but I don't think x86 is special here.
> 


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-26  8:38       ` Re: Christian König
@ 2025-08-26  8:46         ` David Hildenbrand
  2025-08-26  9:00           ` Re: Christian König
  2025-08-26 12:37         ` David Hildenbrand
  1 sibling, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-26  8:46 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 26.08.25 10:38, Christian König wrote:
> On 25.08.25 21:10, David Hildenbrand wrote:
>> On 21.08.25 10:10, Christian König wrote:
>>> On 20.08.25 17:23, David Hildenbrand wrote:
>>>> CCing Lorenzo
>>>>
>>>> On 20.08.25 16:33, Christian König wrote:
>>>>> Hi everyone,
>>>>>
>>>>> sorry for CCing so many people, but that rabbit hole turned out to be
>>>>> deeper than originally thought.
>>>>>
>>>>> TTM always had problems with UC/WC mappings on 32bit systems and drivers
>>>>> often had to revert to hacks like using GFP_DMA32 to get things working
>>>>> while having no rational explanation why that helped (see the TTM AGP,
>>>>> radeon and nouveau driver code for that).
>>>>>
>>>>> It turned out that the PAT implementation we use on x86 not only enforces
>>>>> the same caching attributes for pages in the linear kernel mapping, but
>>>>> also for highmem pages through a separate R/B tree.
>>>>>
>>>>> That was unexpected and TTM never updated that R/B tree for highmem pages,
>>>>> so the function pgprot_set_cachemode() just overwrote the caching
>>>>> attributes drivers passed in to vmf_insert_pfn_prot() and that essentially
>>>>> caused all kind of random trouble.
>>>>>
>>>>> An R/B tree is potentially not a good data structure to hold thousands if
>>>>> not millions of different attributes for each page, so updating that is
>>>>> probably not the way to solve this issue.
>>>>>
>>>>> Thomas pointed out that the i915 driver is using apply_page_range()
>>>>> instead of vmf_insert_pfn_prot() to circumvent the PAT implementation and
>>>>> just fill in the page tables with what the driver things is the right
>>>>> caching attribute.
>>>>
>>>> I assume you mean apply_to_page_range() -- same issue in patch subjects.
>>>
>>> Oh yes, of course. Sorry.
>>>
>>>> Oh this sounds horrible. Why oh why do we have these hacks in core-mm and have drivers abuse them :(
>>>
>>> Yeah I was also a bit hesitated to use that, but the performance advantage is so high that we probably can't avoid the general approach.
>>>
>>>> Honestly, apply_to_pte_range() is just the entry in doing all kinds of weird crap to page tables because "you know better".
>>>
>>> Exactly that's the problem I'm pointing out, drivers *do* know it better. The core memory management has applied incorrect values which caused all kind of the trouble.
>>>
>>> The problem is not a bug in PAT nor TTM/drivers but rather how they interact with each other.
>>>
>>> What I don't understand is why do we have the PAT in the first place? No other architecture does it this way.
>>
>> Probably because no other architecture has these weird glitches I assume ... skimming over memtype_reserve() and friends there are quite some corner cases the code is handling (BIOS, ACPI, low ISA, system RAM, ...)
>>
>>
>> I did a lot of work on the higher PAT level functions, but I am no expert on the lower level management functions, and in particular all the special cases with different memory types.
>>
>> IIRC, the goal of the PAT subsystem is to make sure that no two page tables map the same PFN with different caching attributes.
> 
> Yeah, that actually makes sense. Thomas from Intel recently explained the technical background to me:
> 
> Some x86 CPUs write back cache lines even if they aren't dirty and what can happen is that because of the linear mapping the CPU speculatively loads a cache line which is elsewhere mapped uncached.
> 
> So the end result is that the writeback of not dirty cache lines potentially corrupts the data in the otherwise uncached system memory.
> 
> But that a) only applies to memory in the linear mapping and b) only to a handful of x86 CPU types (e.g. recently Intels Luna Lake, AMD Athlons produced before 2004, maybe others).
> 
>> It treats ordinary system RAM (IORESOURCE_SYSTEM_RAM) usually in a special way: no special caching mode.
>>
>> For everything else, it expects that someone first reserves a memory range for a specific caching mode.
>>
>> For example, remap_pfn_range()...->pfnmap_track()->memtype_reserve() will make sure that there are no conflicts, to the call memtype_kernel_map_sync() to make sure the identity mapping is updated to the new type.
>>
>> In case someone ends up calling pfnmap_setup_cachemode(), the expectation is that there was a previous call to memtype_reserve_io() or similar, such that pfnmap_setup_cachemode() will find that caching mode.
>>
>>
>> So my assumption would be that that is missing for the drivers here?
> 
> Well yes and no.
> 
> See the PAT is optimized for applying specific caching attributes to ranges [A..B] (e.g. it uses an R/B tree). But what drivers do here is that they have single pages (usually for get_free_page or similar) and want to apply a certain caching attribute to it.
> 
> So what would happen is that we completely clutter the R/B tree used by the PAT with thousands if not millions of entries.
> 

Hm, above you're saying that there is no direct map, but now you are 
saying that the pages were obtained through get_free_page()?

I agree that what you describe here sounds suboptimal. But if the pages 
where obtained from the buddy, there surely is a direct map -- unless we 
explicitly remove it :(

If we're talking about individual pages without a directmap, I would 
wonder if they are actually part of a bigger memory region that can just 
be reserved in one go (similar to how remap_pfn_range()) would handle it.

Can you briefly describe how your use case obtains these PFNs, and how 
scattered tehy + their caching attributes might be?

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-26  8:46         ` Re: David Hildenbrand
@ 2025-08-26  9:00           ` Christian König
  2025-08-26  9:17             ` Re: David Hildenbrand
  0 siblings, 1 reply; 62+ messages in thread
From: Christian König @ 2025-08-26  9:00 UTC (permalink / raw)
  To: David Hildenbrand, intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 26.08.25 10:46, David Hildenbrand wrote:
>>> So my assumption would be that that is missing for the drivers here?
>>
>> Well yes and no.
>>
>> See the PAT is optimized for applying specific caching attributes to ranges [A..B] (e.g. it uses an R/B tree). But what drivers do here is that they have single pages (usually for get_free_page or similar) and want to apply a certain caching attribute to it.
>>
>> So what would happen is that we completely clutter the R/B tree used by the PAT with thousands if not millions of entries.
>>
> 
> Hm, above you're saying that there is no direct map, but now you are saying that the pages were obtained through get_free_page()?

The problem only happens with highmem pages on 32bit kernels. Those pages are not in the linear mapping.

> I agree that what you describe here sounds suboptimal. But if the pages where obtained from the buddy, there surely is a direct map -- unless we explicitly remove it :(
> 
> If we're talking about individual pages without a directmap, I would wonder if they are actually part of a bigger memory region that can just be reserved in one go (similar to how remap_pfn_range()) would handle it.
> 
> Can you briefly describe how your use case obtains these PFNs, and how scattered tehy + their caching attributes might be?

What drivers do is to call get_free_page() or alloc_pages_node() with the GFP_HIGHUSER flag set.

For non highmem pages drivers then calls set_pages_wc/uc() which changes the caching of the linear mapping, but for highmem pages there is no linear mapping so set_pages_wc() or set_pages_uc() doesn't work and drivers avoid calling it.

Those are basically just random system memory pages. So they are potentially scattered over the whole memory address space.

Regards,
Christian.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-26  9:00           ` Re: Christian König
@ 2025-08-26  9:17             ` David Hildenbrand
  2025-08-26  9:56               ` Re: Christian König
  0 siblings, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-26  9:17 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 26.08.25 11:00, Christian König wrote:
> On 26.08.25 10:46, David Hildenbrand wrote:
>>>> So my assumption would be that that is missing for the drivers here?
>>>
>>> Well yes and no.
>>>
>>> See the PAT is optimized for applying specific caching attributes to ranges [A..B] (e.g. it uses an R/B tree). But what drivers do here is that they have single pages (usually for get_free_page or similar) and want to apply a certain caching attribute to it.
>>>
>>> So what would happen is that we completely clutter the R/B tree used by the PAT with thousands if not millions of entries.
>>>
>>
>> Hm, above you're saying that there is no direct map, but now you are saying that the pages were obtained through get_free_page()?
> 
> The problem only happens with highmem pages on 32bit kernels. Those pages are not in the linear mapping.

Right, in the common case there is a direct map.

> 
>> I agree that what you describe here sounds suboptimal. But if the pages where obtained from the buddy, there surely is a direct map -- unless we explicitly remove it :(
>>
>> If we're talking about individual pages without a directmap, I would wonder if they are actually part of a bigger memory region that can just be reserved in one go (similar to how remap_pfn_range()) would handle it.
>>
>> Can you briefly describe how your use case obtains these PFNs, and how scattered tehy + their caching attributes might be?
> 
> What drivers do is to call get_free_page() or alloc_pages_node() with the GFP_HIGHUSER flag set.
> 
> For non highmem pages drivers then calls set_pages_wc/uc() which changes the caching of the linear mapping, but for highmem pages there is no linear mapping so set_pages_wc() or set_pages_uc() doesn't work and drivers avoid calling it.
> 
> Those are basically just random system memory pages. So they are potentially scattered over the whole memory address space.

Thanks, that's valuable information.

So essentially these drivers maintain their own consistency and PAT is 
not aware of that.

And the real problem is ordinary system RAM.

There are various ways forward.

1) We use another interface that consumes pages instead of PFNs, like a
    vm_insert_pages_pgprot() we would be adding.

    Is there any strong requirement for inserting non-refcounted PFNs?

2) We add another interface that consumes PFNs, but explicitly states
    that it is only for ordinary system RAM, and that the user is
    required for updating the direct map.

    We could sanity-check the direct map in debug kernels.

3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating this
    system RAM differently.


There is also the option for a mixture between 1 and 2, where we get 
pages, but we map them non-refcounted in a VM_PFNMAP.

In general, having pages makes it easier to assert that they are likely 
ordinary system ram pages, and that the interface is not getting abused 
for something else.

We could also perform the set_pages_wc/uc() from inside that function, 
but maybe it depends on the use case whether we want to do that whenever 
we map them into a process?

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-26  9:17             ` Re: David Hildenbrand
@ 2025-08-26  9:56               ` Christian König
  2025-08-26 12:07                 ` Re: David Hildenbrand
  2025-08-26 14:27                 ` Thomas Hellström
  0 siblings, 2 replies; 62+ messages in thread
From: Christian König @ 2025-08-26  9:56 UTC (permalink / raw)
  To: David Hildenbrand, intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 26.08.25 11:17, David Hildenbrand wrote:
> On 26.08.25 11:00, Christian König wrote:
>> On 26.08.25 10:46, David Hildenbrand wrote:
>>>>> So my assumption would be that that is missing for the drivers here?
>>>>
>>>> Well yes and no.
>>>>
>>>> See the PAT is optimized for applying specific caching attributes to ranges [A..B] (e.g. it uses an R/B tree). But what drivers do here is that they have single pages (usually for get_free_page or similar) and want to apply a certain caching attribute to it.
>>>>
>>>> So what would happen is that we completely clutter the R/B tree used by the PAT with thousands if not millions of entries.
>>>>
>>>
>>> Hm, above you're saying that there is no direct map, but now you are saying that the pages were obtained through get_free_page()?
>>
>> The problem only happens with highmem pages on 32bit kernels. Those pages are not in the linear mapping.
> 
> Right, in the common case there is a direct map.
> 
>>
>>> I agree that what you describe here sounds suboptimal. But if the pages where obtained from the buddy, there surely is a direct map -- unless we explicitly remove it :(
>>>
>>> If we're talking about individual pages without a directmap, I would wonder if they are actually part of a bigger memory region that can just be reserved in one go (similar to how remap_pfn_range()) would handle it.
>>>
>>> Can you briefly describe how your use case obtains these PFNs, and how scattered tehy + their caching attributes might be?
>>
>> What drivers do is to call get_free_page() or alloc_pages_node() with the GFP_HIGHUSER flag set.
>>
>> For non highmem pages drivers then calls set_pages_wc/uc() which changes the caching of the linear mapping, but for highmem pages there is no linear mapping so set_pages_wc() or set_pages_uc() doesn't work and drivers avoid calling it.
>>
>> Those are basically just random system memory pages. So they are potentially scattered over the whole memory address space.
> 
> Thanks, that's valuable information.
> 
> So essentially these drivers maintain their own consistency and PAT is not aware of that.
> 
> And the real problem is ordinary system RAM.
> 
> There are various ways forward.
> 
> 1) We use another interface that consumes pages instead of PFNs, like a
>    vm_insert_pages_pgprot() we would be adding.
> 
>    Is there any strong requirement for inserting non-refcounted PFNs?

Yes, there is a strong requirement to insert non-refcounted PFNs.

We had a lot of trouble with KVM people trying to grab a reference to those pages even if the VMA had the VM_PFNMAP flag set.

> 2) We add another interface that consumes PFNs, but explicitly states
>    that it is only for ordinary system RAM, and that the user is
>    required for updating the direct map.
> 
>    We could sanity-check the direct map in debug kernels.

I would rather like to see vmf_insert_pfn_prot() fixed instead.

That function was explicitly added to insert the PFN with the given attributes and as far as I can see all users of that function expect exactly that.

> 
> 3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating this
>    system RAM differently.
> 
> 
> There is also the option for a mixture between 1 and 2, where we get pages, but we map them non-refcounted in a VM_PFNMAP.
> 
> In general, having pages makes it easier to assert that they are likely ordinary system ram pages, and that the interface is not getting abused for something else.

Well, exactly that's the use case here and that is not abusive at all as far as I can see.

What drivers want is to insert a PFN with a certain set of caching attributes regardless if it's system memory or iomem. That's why vmf_insert_pfn_prot() was created in the first place.

That drivers need to call set_pages_wc/uc() for the linear mapping on x86 manually is correct and checking that is clearly a good idea for debug kernels.

> We could also perform the set_pages_wc/uc() from inside that function, but maybe it depends on the use case whether we want to do that whenever we map them into a process?

It sounds like a good idea in theory, but I think it is potentially to much overhead to be applicable.

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-26  9:56               ` Re: Christian König
@ 2025-08-26 12:07                 ` David Hildenbrand
  2025-08-26 16:09                   ` Re: Christian König
  2025-08-26 14:27                 ` Thomas Hellström
  1 sibling, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-26 12:07 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

>>
>> 1) We use another interface that consumes pages instead of PFNs, like a
>>     vm_insert_pages_pgprot() we would be adding.
>>
>>     Is there any strong requirement for inserting non-refcounted PFNs?
> 
> Yes, there is a strong requirement to insert non-refcounted PFNs.
> 
> We had a lot of trouble with KVM people trying to grab a reference to those pages even if the VMA had the VM_PFNMAP flag set.

Yes, KVM ignored (and maybe still does) VM_PFNMAP to some degree, which 
is rather nasty.

> 
>> 2) We add another interface that consumes PFNs, but explicitly states
>>     that it is only for ordinary system RAM, and that the user is
>>     required for updating the direct map.
>>
>>     We could sanity-check the direct map in debug kernels.
> 
> I would rather like to see vmf_insert_pfn_prot() fixed instead.
> 
> That function was explicitly added to insert the PFN with the given attributes and as far as I can see all users of that function expect exactly that.

It's all a bit tricky :(

> 
>>
>> 3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating this
>>     system RAM differently.
>>
>>
>> There is also the option for a mixture between 1 and 2, where we get pages, but we map them non-refcounted in a VM_PFNMAP.
>>
>> In general, having pages makes it easier to assert that they are likely ordinary system ram pages, and that the interface is not getting abused for something else.
> 
> Well, exactly that's the use case here and that is not abusive at all as far as I can see.
> 
> What drivers want is to insert a PFN with a certain set of caching attributes regardless if it's system memory or iomem. That's why vmf_insert_pfn_prot() was created in the first place.

I mean, the use case of "allocate pages from the buddy and fixup the 
linear map" sounds perfectly reasonable to me. Absolutely no reason to 
get PAT involved. Nobody else should be messing with that memory after all.

As soon as we are talking about other memory ranges (iomem) that are not 
from the buddy, it gets weird to bypass PAT, and the question I am 
asking myself is, when is it okay, and when not.

> 
> That drivers need to call set_pages_wc/uc() for the linear mapping on x86 manually is correct and checking that is clearly a good idea for debug kernels.

I'll have to think about this a bit: assuming only vmf_insert_pfn() 
calls pfnmap_setup_cachemode_pfn() but vmf_insert_pfn_prot() doesn't, 
how could we sanity check that somebody is doing something against the 
will of PAT.

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-26  8:38       ` Re: Christian König
  2025-08-26  8:46         ` Re: David Hildenbrand
@ 2025-08-26 12:37         ` David Hildenbrand
  1 sibling, 0 replies; 62+ messages in thread
From: David Hildenbrand @ 2025-08-26 12:37 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 26.08.25 10:38, Christian König wrote:
> On 25.08.25 21:10, David Hildenbrand wrote:
>> On 21.08.25 10:10, Christian König wrote:
>>> On 20.08.25 17:23, David Hildenbrand wrote:
>>>> CCing Lorenzo
>>>>
>>>> On 20.08.25 16:33, Christian König wrote:
>>>>> Hi everyone,
>>>>>
>>>>> sorry for CCing so many people, but that rabbit hole turned out to be
>>>>> deeper than originally thought.
>>>>>
>>>>> TTM always had problems with UC/WC mappings on 32bit systems and drivers
>>>>> often had to revert to hacks like using GFP_DMA32 to get things working
>>>>> while having no rational explanation why that helped (see the TTM AGP,
>>>>> radeon and nouveau driver code for that).
>>>>>
>>>>> It turned out that the PAT implementation we use on x86 not only enforces
>>>>> the same caching attributes for pages in the linear kernel mapping, but
>>>>> also for highmem pages through a separate R/B tree.
>>>>>
>>>>> That was unexpected and TTM never updated that R/B tree for highmem pages,
>>>>> so the function pgprot_set_cachemode() just overwrote the caching
>>>>> attributes drivers passed in to vmf_insert_pfn_prot() and that essentially
>>>>> caused all kind of random trouble.
>>>>>
>>>>> An R/B tree is potentially not a good data structure to hold thousands if
>>>>> not millions of different attributes for each page, so updating that is
>>>>> probably not the way to solve this issue.
>>>>>
>>>>> Thomas pointed out that the i915 driver is using apply_page_range()
>>>>> instead of vmf_insert_pfn_prot() to circumvent the PAT implementation and
>>>>> just fill in the page tables with what the driver things is the right
>>>>> caching attribute.
>>>>
>>>> I assume you mean apply_to_page_range() -- same issue in patch subjects.
>>>
>>> Oh yes, of course. Sorry.
>>>
>>>> Oh this sounds horrible. Why oh why do we have these hacks in core-mm and have drivers abuse them :(
>>>
>>> Yeah I was also a bit hesitated to use that, but the performance advantage is so high that we probably can't avoid the general approach.
>>>
>>>> Honestly, apply_to_pte_range() is just the entry in doing all kinds of weird crap to page tables because "you know better".
>>>
>>> Exactly that's the problem I'm pointing out, drivers *do* know it better. The core memory management has applied incorrect values which caused all kind of the trouble.
>>>
>>> The problem is not a bug in PAT nor TTM/drivers but rather how they interact with each other.
>>>
>>> What I don't understand is why do we have the PAT in the first place? No other architecture does it this way.
>>
>> Probably because no other architecture has these weird glitches I assume ... skimming over memtype_reserve() and friends there are quite some corner cases the code is handling (BIOS, ACPI, low ISA, system RAM, ...)
>>
>>
>> I did a lot of work on the higher PAT level functions, but I am no expert on the lower level management functions, and in particular all the special cases with different memory types.
>>
>> IIRC, the goal of the PAT subsystem is to make sure that no two page tables map the same PFN with different caching attributes.
> 
> Yeah, that actually makes sense. Thomas from Intel recently explained the technical background to me:
> 
> Some x86 CPUs write back cache lines even if they aren't dirty and what can happen is that because of the linear mapping the CPU speculatively loads a cache line which is elsewhere mapped uncached.
> 
> So the end result is that the writeback of not dirty cache lines potentially corrupts the data in the otherwise uncached system memory.
> 
> But that a) only applies to memory in the linear mapping and b) only to a handful of x86 CPU types (e.g. recently Intels Luna Lake, AMD Athlons produced before 2004, maybe others).
> 
>> It treats ordinary system RAM (IORESOURCE_SYSTEM_RAM) usually in a special way: no special caching mode.
>>
>> For everything else, it expects that someone first reserves a memory range for a specific caching mode.
>>
>> For example, remap_pfn_range()...->pfnmap_track()->memtype_reserve() will make sure that there are no conflicts, to the call memtype_kernel_map_sync() to make sure the identity mapping is updated to the new type.
>>
>> In case someone ends up calling pfnmap_setup_cachemode(), the expectation is that there was a previous call to memtype_reserve_io() or similar, such that pfnmap_setup_cachemode() will find that caching mode.
>>
>>
>> So my assumption would be that that is missing for the drivers here?
> 
> Well yes and no.
> 
> See the PAT is optimized for applying specific caching attributes to ranges [A..B] (e.g. it uses an R/B tree). But what drivers do here is that they have single pages (usually for get_free_page or similar) and want to apply a certain caching attribute to it.

One clarification after staring at PAT code once again: for pages (RAM), 
the caching attribute is stored in the page flags, not in the R/B tree.

If nothing was set, it defaults to _PAGE_CACHE_MODE_WB AFAIKs.

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-26  9:56               ` Re: Christian König
  2025-08-26 12:07                 ` Re: David Hildenbrand
@ 2025-08-26 14:27                 ` Thomas Hellström
  2025-08-28 21:01                   ` stupid PAT :) David Hildenbrand
  1 sibling, 1 reply; 62+ messages in thread
From: Thomas Hellström @ 2025-08-26 14:27 UTC (permalink / raw)
  To: Christian König, David Hildenbrand, intel-xe, intel-gfx,
	dri-devel, amd-gfx, x86
  Cc: airlied, matthew.brost, dave.hansen, luto, peterz,
	Lorenzo Stoakes

Hi, Christian,

On Tue, 2025-08-26 at 11:56 +0200, Christian König wrote:
> On 26.08.25 11:17, David Hildenbrand wrote:
> > On 26.08.25 11:00, Christian König wrote:
> > > On 26.08.25 10:46, David Hildenbrand wrote:
> > > > > > So my assumption would be that that is missing for the
> > > > > > drivers here?
> > > > > 
> > > > > Well yes and no.
> > > > > 
> > > > > See the PAT is optimized for applying specific caching
> > > > > attributes to ranges [A..B] (e.g. it uses an R/B tree). But
> > > > > what drivers do here is that they have single pages (usually
> > > > > for get_free_page or similar) and want to apply a certain
> > > > > caching attribute to it.
> > > > > 
> > > > > So what would happen is that we completely clutter the R/B
> > > > > tree used by the PAT with thousands if not millions of
> > > > > entries.
> > > > > 
> > > > 
> > > > Hm, above you're saying that there is no direct map, but now
> > > > you are saying that the pages were obtained through
> > > > get_free_page()?
> > > 
> > > The problem only happens with highmem pages on 32bit kernels.
> > > Those pages are not in the linear mapping.
> > 
> > Right, in the common case there is a direct map.
> > 
> > > 
> > > > I agree that what you describe here sounds suboptimal. But if
> > > > the pages where obtained from the buddy, there surely is a
> > > > direct map -- unless we explicitly remove it :(
> > > > 
> > > > If we're talking about individual pages without a directmap, I
> > > > would wonder if they are actually part of a bigger memory
> > > > region that can just be reserved in one go (similar to how
> > > > remap_pfn_range()) would handle it.
> > > > 
> > > > Can you briefly describe how your use case obtains these PFNs,
> > > > and how scattered tehy + their caching attributes might be?
> > > 
> > > What drivers do is to call get_free_page() or alloc_pages_node()
> > > with the GFP_HIGHUSER flag set.
> > > 
> > > For non highmem pages drivers then calls set_pages_wc/uc() which
> > > changes the caching of the linear mapping, but for highmem pages
> > > there is no linear mapping so set_pages_wc() or set_pages_uc()
> > > doesn't work and drivers avoid calling it.
> > > 
> > > Those are basically just random system memory pages. So they are
> > > potentially scattered over the whole memory address space.
> > 
> > Thanks, that's valuable information.
> > 
> > So essentially these drivers maintain their own consistency and PAT
> > is not aware of that.
> > 
> > And the real problem is ordinary system RAM.
> > 
> > There are various ways forward.
> > 
> > 1) We use another interface that consumes pages instead of PFNs,
> > like a
> >    vm_insert_pages_pgprot() we would be adding.
> > 
> >    Is there any strong requirement for inserting non-refcounted
> > PFNs?
> 
> Yes, there is a strong requirement to insert non-refcounted PFNs.
> 
> We had a lot of trouble with KVM people trying to grab a reference to
> those pages even if the VMA had the VM_PFNMAP flag set.
> 
> > 2) We add another interface that consumes PFNs, but explicitly
> > states
> >    that it is only for ordinary system RAM, and that the user is
> >    required for updating the direct map.
> > 
> >    We could sanity-check the direct map in debug kernels.
> 
> I would rather like to see vmf_insert_pfn_prot() fixed instead.
> 
> That function was explicitly added to insert the PFN with the given
> attributes and as far as I can see all users of that function expect
> exactly that.
> 
> > 
> > 3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating
> > this
> >    system RAM differently.
> > 
> > 
> > There is also the option for a mixture between 1 and 2, where we
> > get pages, but we map them non-refcounted in a VM_PFNMAP.
> > 
> > In general, having pages makes it easier to assert that they are
> > likely ordinary system ram pages, and that the interface is not
> > getting abused for something else.
> 
> Well, exactly that's the use case here and that is not abusive at all
> as far as I can see.
> 
> What drivers want is to insert a PFN with a certain set of caching
> attributes regardless if it's system memory or iomem. That's why
> vmf_insert_pfn_prot() was created in the first place.
> 
> That drivers need to call set_pages_wc/uc() for the linear mapping on
> x86 manually is correct and checking that is clearly a good idea for
> debug kernels.

So where is this trending? Is the current suggestion to continue
disallowing aliased mappings with conflicting caching modes and enforce
checks in debug kernels?

/Thomas


> 
> > We could also perform the set_pages_wc/uc() from inside that
> > function, but maybe it depends on the use case whether we want to
> > do that whenever we map them into a process?
> 
> It sounds like a good idea in theory, but I think it is potentially
> to much overhead to be applicable.
> 
> Thanks,
> Christian.


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re:
  2025-08-26 12:07                 ` Re: David Hildenbrand
@ 2025-08-26 16:09                   ` Christian König
  2025-08-27  9:13                     ` [PATCH 0/3] drm/ttm: Michel Dänzer
  2025-08-28 21:18                     ` stupid and complicated PAT :) David Hildenbrand
  0 siblings, 2 replies; 62+ messages in thread
From: Christian König @ 2025-08-26 16:09 UTC (permalink / raw)
  To: David Hildenbrand, intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 26.08.25 14:07, David Hildenbrand wrote: 
>>
>>> 2) We add another interface that consumes PFNs, but explicitly states
>>>     that it is only for ordinary system RAM, and that the user is
>>>     required for updating the direct map.
>>>
>>>     We could sanity-check the direct map in debug kernels.
>>
>> I would rather like to see vmf_insert_pfn_prot() fixed instead.
>>
>> That function was explicitly added to insert the PFN with the given attributes and as far as I can see all users of that function expect exactly that.
> 
> It's all a bit tricky :(

I would rather say horrible complicated :(

>>>
>>> 3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating this
>>>     system RAM differently.
>>>
>>>
>>> There is also the option for a mixture between 1 and 2, where we get pages, but we map them non-refcounted in a VM_PFNMAP.
>>>
>>> In general, having pages makes it easier to assert that they are likely ordinary system ram pages, and that the interface is not getting abused for something else.
>>
>> Well, exactly that's the use case here and that is not abusive at all as far as I can see.
>>
>> What drivers want is to insert a PFN with a certain set of caching attributes regardless if it's system memory or iomem. That's why vmf_insert_pfn_prot() was created in the first place.
> 
> I mean, the use case of "allocate pages from the buddy and fixup the linear map" sounds perfectly reasonable to me. Absolutely no reason to get PAT involved. Nobody else should be messing with that memory after all.
> 
> As soon as we are talking about other memory ranges (iomem) that are not from the buddy, it gets weird to bypass PAT, and the question I am asking myself is, when is it okay, and when not.

Ok let me try to explain parts of the history and the big picture for at least the graphics use case on x86.

In 1996/97 Intel came up with the idea of AGP: https://en.wikipedia.org/wiki/Accelerated_Graphics_Port

At that time the CPUs, PCI bus and system memory were all connected together through the north bridge: https://en.wikipedia.org/wiki/Northbridge_(computing)

The problem was that AGP also introduced the concept of putting large amounts of data for the video controller (PCI device) into system memory when you don't have enough local device memory (VRAM).

But that meant when that memory is cached that the north bridge always had to snoop the CPU cache over the front side bus for every access the video controller made. This meant a huge performance bottleneck, so the idea was born to access that data uncached.


Well that was nearly 30years ago, PCI, AGP and front side bus are long gone, but the concept of putting video controller (GPU) stuff into uncached system memory has prevailed.

So for example even modern AMD CPU based laptops need uncached system memory if their local memory is not large enough to contain the picture to display on the monitor. And with modern 8k monitors that can actually happen quite fast...

What drivers do today is to call vmf_insert_pfn_prot() either with the PFN of their local memory (iomem) or uncached/wc system memory.


To summarize that we have an interface to fill in the page tables with either iomem or system memory is actually part of the design. That's how the HW driver is expected to work.

>> That drivers need to call set_pages_wc/uc() for the linear mapping on x86 manually is correct and checking that is clearly a good idea for debug kernels.
> 
> I'll have to think about this a bit: assuming only vmf_insert_pfn() calls pfnmap_setup_cachemode_pfn() but vmf_insert_pfn_prot() doesn't, how could we sanity check that somebody is doing something against the will of PAT.

I think the most defensive approach for a quick fix is this change here:

 static inline void pgprot_set_cachemode(pgprot_t *prot, enum page_cache_mode pcm)
 {
-       *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
-                        cachemode2protval(pcm));
+       if (pcm != _PAGE_CACHE_MODE_WB)
+               *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
+                                cachemode2protval(pcm));
 }

This applies the PAT value if it's anything else than _PAGE_CACHE_MODE_WB but still allows callers to use something different on normal WB system memory.

What do you think?

Regards,
Christian

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 0/3] drm/ttm: ...
  2025-08-26 16:09                   ` Re: Christian König
@ 2025-08-27  9:13                     ` Michel Dänzer
  2025-08-28 21:18                     ` stupid and complicated PAT :) David Hildenbrand
  1 sibling, 0 replies; 62+ messages in thread
From: Michel Dänzer @ 2025-08-27  9:13 UTC (permalink / raw)
  To: Christian König, David Hildenbrand, intel-xe, intel-gfx,
	dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes


Public Service Announcement from your friendly neighbourhood mailing list moderation queue processor:

I recommend setting a non-empty subject in follow-ups to this thread, or I might accidentally discard them from the moderation queue (I just almost did).


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

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: stupid PAT :)
  2025-08-26 14:27                 ` Thomas Hellström
@ 2025-08-28 21:01                   ` David Hildenbrand
  0 siblings, 0 replies; 62+ messages in thread
From: David Hildenbrand @ 2025-08-28 21:01 UTC (permalink / raw)
  To: Thomas Hellström, Christian König, intel-xe, intel-gfx,
	dri-devel, amd-gfx, x86
  Cc: airlied, matthew.brost, dave.hansen, luto, peterz,
	Lorenzo Stoakes

On 26.08.25 16:27, Thomas Hellström wrote:
> Hi, Christian,
> 
> On Tue, 2025-08-26 at 11:56 +0200, Christian König wrote:
>> On 26.08.25 11:17, David Hildenbrand wrote:
>>> On 26.08.25 11:00, Christian König wrote:
>>>> On 26.08.25 10:46, David Hildenbrand wrote:
>>>>>>> So my assumption would be that that is missing for the
>>>>>>> drivers here?
>>>>>>
>>>>>> Well yes and no.
>>>>>>
>>>>>> See the PAT is optimized for applying specific caching
>>>>>> attributes to ranges [A..B] (e.g. it uses an R/B tree). But
>>>>>> what drivers do here is that they have single pages (usually
>>>>>> for get_free_page or similar) and want to apply a certain
>>>>>> caching attribute to it.
>>>>>>
>>>>>> So what would happen is that we completely clutter the R/B
>>>>>> tree used by the PAT with thousands if not millions of
>>>>>> entries.
>>>>>>
>>>>>
>>>>> Hm, above you're saying that there is no direct map, but now
>>>>> you are saying that the pages were obtained through
>>>>> get_free_page()?
>>>>
>>>> The problem only happens with highmem pages on 32bit kernels.
>>>> Those pages are not in the linear mapping.
>>>
>>> Right, in the common case there is a direct map.
>>>
>>>>
>>>>> I agree that what you describe here sounds suboptimal. But if
>>>>> the pages where obtained from the buddy, there surely is a
>>>>> direct map -- unless we explicitly remove it :(
>>>>>
>>>>> If we're talking about individual pages without a directmap, I
>>>>> would wonder if they are actually part of a bigger memory
>>>>> region that can just be reserved in one go (similar to how
>>>>> remap_pfn_range()) would handle it.
>>>>>
>>>>> Can you briefly describe how your use case obtains these PFNs,
>>>>> and how scattered tehy + their caching attributes might be?
>>>>
>>>> What drivers do is to call get_free_page() or alloc_pages_node()
>>>> with the GFP_HIGHUSER flag set.
>>>>
>>>> For non highmem pages drivers then calls set_pages_wc/uc() which
>>>> changes the caching of the linear mapping, but for highmem pages
>>>> there is no linear mapping so set_pages_wc() or set_pages_uc()
>>>> doesn't work and drivers avoid calling it.
>>>>
>>>> Those are basically just random system memory pages. So they are
>>>> potentially scattered over the whole memory address space.
>>>
>>> Thanks, that's valuable information.
>>>
>>> So essentially these drivers maintain their own consistency and PAT
>>> is not aware of that.
>>>
>>> And the real problem is ordinary system RAM.
>>>
>>> There are various ways forward.
>>>
>>> 1) We use another interface that consumes pages instead of PFNs,
>>> like a
>>>     vm_insert_pages_pgprot() we would be adding.
>>>
>>>     Is there any strong requirement for inserting non-refcounted
>>> PFNs?
>>
>> Yes, there is a strong requirement to insert non-refcounted PFNs.
>>
>> We had a lot of trouble with KVM people trying to grab a reference to
>> those pages even if the VMA had the VM_PFNMAP flag set.
>>
>>> 2) We add another interface that consumes PFNs, but explicitly
>>> states
>>>     that it is only for ordinary system RAM, and that the user is
>>>     required for updating the direct map.
>>>
>>>     We could sanity-check the direct map in debug kernels.
>>
>> I would rather like to see vmf_insert_pfn_prot() fixed instead.
>>
>> That function was explicitly added to insert the PFN with the given
>> attributes and as far as I can see all users of that function expect
>> exactly that.
>>
>>>
>>> 3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating
>>> this
>>>     system RAM differently.
>>>
>>>
>>> There is also the option for a mixture between 1 and 2, where we
>>> get pages, but we map them non-refcounted in a VM_PFNMAP.
>>>
>>> In general, having pages makes it easier to assert that they are
>>> likely ordinary system ram pages, and that the interface is not
>>> getting abused for something else.
>>
>> Well, exactly that's the use case here and that is not abusive at all
>> as far as I can see.
>>
>> What drivers want is to insert a PFN with a certain set of caching
>> attributes regardless if it's system memory or iomem. That's why
>> vmf_insert_pfn_prot() was created in the first place.
>>
>> That drivers need to call set_pages_wc/uc() for the linear mapping on
>> x86 manually is correct and checking that is clearly a good idea for
>> debug kernels.
> 
> So where is this trending? Is the current suggestion to continue
> disallowing aliased mappings with conflicting caching modes and enforce
> checks in debug kernels?

Not sure, it's a mess. The big question is to find out when it is really 
ok to bypass PAT and when to better let it have a saying.

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: stupid and complicated PAT :)
  2025-08-26 16:09                   ` Re: Christian König
  2025-08-27  9:13                     ` [PATCH 0/3] drm/ttm: Michel Dänzer
@ 2025-08-28 21:18                     ` David Hildenbrand
  2025-08-28 21:28                       ` David Hildenbrand
  1 sibling, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-28 21:18 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 26.08.25 18:09, Christian König wrote:
> On 26.08.25 14:07, David Hildenbrand wrote:
>>>
>>>> 2) We add another interface that consumes PFNs, but explicitly states
>>>>      that it is only for ordinary system RAM, and that the user is
>>>>      required for updating the direct map.
>>>>
>>>>      We could sanity-check the direct map in debug kernels.
>>>
>>> I would rather like to see vmf_insert_pfn_prot() fixed instead.
>>>
>>> That function was explicitly added to insert the PFN with the given attributes and as far as I can see all users of that function expect exactly that.
>>
>> It's all a bit tricky :(
> 
> I would rather say horrible complicated :(
> 
>>>>
>>>> 3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating this
>>>>      system RAM differently.
>>>>
>>>>
>>>> There is also the option for a mixture between 1 and 2, where we get pages, but we map them non-refcounted in a VM_PFNMAP.
>>>>
>>>> In general, having pages makes it easier to assert that they are likely ordinary system ram pages, and that the interface is not getting abused for something else.
>>>
>>> Well, exactly that's the use case here and that is not abusive at all as far as I can see.
>>>
>>> What drivers want is to insert a PFN with a certain set of caching attributes regardless if it's system memory or iomem. That's why vmf_insert_pfn_prot() was created in the first place.
>>
>> I mean, the use case of "allocate pages from the buddy and fixup the linear map" sounds perfectly reasonable to me. Absolutely no reason to get PAT involved. Nobody else should be messing with that memory after all.
>>
>> As soon as we are talking about other memory ranges (iomem) that are not from the buddy, it gets weird to bypass PAT, and the question I am asking myself is, when is it okay, and when not.
> 
> Ok let me try to explain parts of the history and the big picture for at least the graphics use case on x86.
> 
> In 1996/97 Intel came up with the idea of AGP: https://en.wikipedia.org/wiki/Accelerated_Graphics_Port
> 
> At that time the CPUs, PCI bus and system memory were all connected together through the north bridge: https://en.wikipedia.org/wiki/Northbridge_(computing)
> 
> The problem was that AGP also introduced the concept of putting large amounts of data for the video controller (PCI device) into system memory when you don't have enough local device memory (VRAM).
> 
> But that meant when that memory is cached that the north bridge always had to snoop the CPU cache over the front side bus for every access the video controller made. This meant a huge performance bottleneck, so the idea was born to access that data uncached.

Ack.

> 
> 
> Well that was nearly 30years ago, PCI, AGP and front side bus are long gone, but the concept of putting video controller (GPU) stuff into uncached system memory has prevailed.
> 
> So for example even modern AMD CPU based laptops need uncached system memory if their local memory is not large enough to contain the picture to display on the monitor. And with modern 8k monitors that can actually happen quite fast...
> 
> What drivers do today is to call vmf_insert_pfn_prot() either with the PFN of their local memory (iomem) or uncached/wc system memory.

That makes perfect sense. I assume we might or might not have "struct 
page" (pfn_valid) for the iomem, depending on where these areas reside, 
correct?

> 
> 
> To summarize that we have an interface to fill in the page tables with either iomem or system memory is actually part of the design. That's how the HW driver is expected to work.
> 
>>> That drivers need to call set_pages_wc/uc() for the linear mapping on x86 manually is correct and checking that is clearly a good idea for debug kernels.
>>
>> I'll have to think about this a bit: assuming only vmf_insert_pfn() calls pfnmap_setup_cachemode_pfn() but vmf_insert_pfn_prot() doesn't, how could we sanity check that somebody is doing something against the will of PAT.
> 
> I think the most defensive approach for a quick fix is this change here:
> 
>   static inline void pgprot_set_cachemode(pgprot_t *prot, enum page_cache_mode pcm)
>   {
> -       *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
> -                        cachemode2protval(pcm));
> +       if (pcm != _PAGE_CACHE_MODE_WB)
> +               *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
> +                                cachemode2protval(pcm));
>   }
> 
> This applies the PAT value if it's anything else than _PAGE_CACHE_MODE_WB but still allows callers to use something different on normal WB system memory.
> 
> What do you think?

This feels like too big of a hammer. In particular, it changes things 
like phys_mem_access_prot_allowed(), which requires more care.

First, I thought we should limit what we do to vmf_insert_pfn_prot() 
only. But then I realized that we have stuff like

	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);

I'm still trying to find out the easy way out that is not a complete hack.

Will the iomem ever be mapped by the driver again with a different cache 
mode? (e.g., WB -> UC -> WB)

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: stupid and complicated PAT :)
  2025-08-28 21:18                     ` stupid and complicated PAT :) David Hildenbrand
@ 2025-08-28 21:28                       ` David Hildenbrand
  2025-08-28 21:32                         ` David Hildenbrand
  0 siblings, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-28 21:28 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 28.08.25 23:18, David Hildenbrand wrote:
> On 26.08.25 18:09, Christian König wrote:
>> On 26.08.25 14:07, David Hildenbrand wrote:
>>>>
>>>>> 2) We add another interface that consumes PFNs, but explicitly states
>>>>>       that it is only for ordinary system RAM, and that the user is
>>>>>       required for updating the direct map.
>>>>>
>>>>>       We could sanity-check the direct map in debug kernels.
>>>>
>>>> I would rather like to see vmf_insert_pfn_prot() fixed instead.
>>>>
>>>> That function was explicitly added to insert the PFN with the given attributes and as far as I can see all users of that function expect exactly that.
>>>
>>> It's all a bit tricky :(
>>
>> I would rather say horrible complicated :(
>>
>>>>>
>>>>> 3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating this
>>>>>       system RAM differently.
>>>>>
>>>>>
>>>>> There is also the option for a mixture between 1 and 2, where we get pages, but we map them non-refcounted in a VM_PFNMAP.
>>>>>
>>>>> In general, having pages makes it easier to assert that they are likely ordinary system ram pages, and that the interface is not getting abused for something else.
>>>>
>>>> Well, exactly that's the use case here and that is not abusive at all as far as I can see.
>>>>
>>>> What drivers want is to insert a PFN with a certain set of caching attributes regardless if it's system memory or iomem. That's why vmf_insert_pfn_prot() was created in the first place.
>>>
>>> I mean, the use case of "allocate pages from the buddy and fixup the linear map" sounds perfectly reasonable to me. Absolutely no reason to get PAT involved. Nobody else should be messing with that memory after all.
>>>
>>> As soon as we are talking about other memory ranges (iomem) that are not from the buddy, it gets weird to bypass PAT, and the question I am asking myself is, when is it okay, and when not.
>>
>> Ok let me try to explain parts of the history and the big picture for at least the graphics use case on x86.
>>
>> In 1996/97 Intel came up with the idea of AGP: https://en.wikipedia.org/wiki/Accelerated_Graphics_Port
>>
>> At that time the CPUs, PCI bus and system memory were all connected together through the north bridge: https://en.wikipedia.org/wiki/Northbridge_(computing)
>>
>> The problem was that AGP also introduced the concept of putting large amounts of data for the video controller (PCI device) into system memory when you don't have enough local device memory (VRAM).
>>
>> But that meant when that memory is cached that the north bridge always had to snoop the CPU cache over the front side bus for every access the video controller made. This meant a huge performance bottleneck, so the idea was born to access that data uncached.
> 
> Ack.
> 
>>
>>
>> Well that was nearly 30years ago, PCI, AGP and front side bus are long gone, but the concept of putting video controller (GPU) stuff into uncached system memory has prevailed.
>>
>> So for example even modern AMD CPU based laptops need uncached system memory if their local memory is not large enough to contain the picture to display on the monitor. And with modern 8k monitors that can actually happen quite fast...
>>
>> What drivers do today is to call vmf_insert_pfn_prot() either with the PFN of their local memory (iomem) or uncached/wc system memory.
> 
> That makes perfect sense. I assume we might or might not have "struct
> page" (pfn_valid) for the iomem, depending on where these areas reside,
> correct?
> 
>>
>>
>> To summarize that we have an interface to fill in the page tables with either iomem or system memory is actually part of the design. That's how the HW driver is expected to work.
>>
>>>> That drivers need to call set_pages_wc/uc() for the linear mapping on x86 manually is correct and checking that is clearly a good idea for debug kernels.
>>>
>>> I'll have to think about this a bit: assuming only vmf_insert_pfn() calls pfnmap_setup_cachemode_pfn() but vmf_insert_pfn_prot() doesn't, how could we sanity check that somebody is doing something against the will of PAT.
>>
>> I think the most defensive approach for a quick fix is this change here:
>>
>>    static inline void pgprot_set_cachemode(pgprot_t *prot, enum page_cache_mode pcm)
>>    {
>> -       *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
>> -                        cachemode2protval(pcm));
>> +       if (pcm != _PAGE_CACHE_MODE_WB)
>> +               *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
>> +                                cachemode2protval(pcm));
>>    }
>>
>> This applies the PAT value if it's anything else than _PAGE_CACHE_MODE_WB but still allows callers to use something different on normal WB system memory.
>>
>> What do you think?
> 
> This feels like too big of a hammer. In particular, it changes things
> like phys_mem_access_prot_allowed(), which requires more care.
> 
> First, I thought we should limit what we do to vmf_insert_pfn_prot()
> only. But then I realized that we have stuff like
> 
> 	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> 
> I'm still trying to find out the easy way out that is not a complete hack.
> 
> Will the iomem ever be mapped by the driver again with a different cache
> mode? (e.g., WB -> UC -> WB)

What I am currently wondering is: assume we get a 
pfnmap_setup_cachemode_pfn() call and we could reliably identify whether 
there was a previous registration, then we could do

(a) No previous registration: don't modify pgprot. Hopefully the driver
     knows what it is doing. Maybe we can add sanity checks that the
     direct map was already updated etc.

(b) A previous registration: modify pgprot like we do today.

System RAM is the problem. I wonder how many of these registrations we 
really get and if we could just store them in the same tree as !system 
RAM instead of abusing page flags.

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: stupid and complicated PAT :)
  2025-08-28 21:28                       ` David Hildenbrand
@ 2025-08-28 21:32                         ` David Hildenbrand
  2025-08-29 10:50                           ` Christian König
  0 siblings, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-28 21:32 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 28.08.25 23:28, David Hildenbrand wrote:
> On 28.08.25 23:18, David Hildenbrand wrote:
>> On 26.08.25 18:09, Christian König wrote:
>>> On 26.08.25 14:07, David Hildenbrand wrote:
>>>>>
>>>>>> 2) We add another interface that consumes PFNs, but explicitly states
>>>>>>        that it is only for ordinary system RAM, and that the user is
>>>>>>        required for updating the direct map.
>>>>>>
>>>>>>        We could sanity-check the direct map in debug kernels.
>>>>>
>>>>> I would rather like to see vmf_insert_pfn_prot() fixed instead.
>>>>>
>>>>> That function was explicitly added to insert the PFN with the given attributes and as far as I can see all users of that function expect exactly that.
>>>>
>>>> It's all a bit tricky :(
>>>
>>> I would rather say horrible complicated :(
>>>
>>>>>>
>>>>>> 3) We teach PAT code in pfnmap_setup_cachemode_pfn() about treating this
>>>>>>        system RAM differently.
>>>>>>
>>>>>>
>>>>>> There is also the option for a mixture between 1 and 2, where we get pages, but we map them non-refcounted in a VM_PFNMAP.
>>>>>>
>>>>>> In general, having pages makes it easier to assert that they are likely ordinary system ram pages, and that the interface is not getting abused for something else.
>>>>>
>>>>> Well, exactly that's the use case here and that is not abusive at all as far as I can see.
>>>>>
>>>>> What drivers want is to insert a PFN with a certain set of caching attributes regardless if it's system memory or iomem. That's why vmf_insert_pfn_prot() was created in the first place.
>>>>
>>>> I mean, the use case of "allocate pages from the buddy and fixup the linear map" sounds perfectly reasonable to me. Absolutely no reason to get PAT involved. Nobody else should be messing with that memory after all.
>>>>
>>>> As soon as we are talking about other memory ranges (iomem) that are not from the buddy, it gets weird to bypass PAT, and the question I am asking myself is, when is it okay, and when not.
>>>
>>> Ok let me try to explain parts of the history and the big picture for at least the graphics use case on x86.
>>>
>>> In 1996/97 Intel came up with the idea of AGP: https://en.wikipedia.org/wiki/Accelerated_Graphics_Port
>>>
>>> At that time the CPUs, PCI bus and system memory were all connected together through the north bridge: https://en.wikipedia.org/wiki/Northbridge_(computing)
>>>
>>> The problem was that AGP also introduced the concept of putting large amounts of data for the video controller (PCI device) into system memory when you don't have enough local device memory (VRAM).
>>>
>>> But that meant when that memory is cached that the north bridge always had to snoop the CPU cache over the front side bus for every access the video controller made. This meant a huge performance bottleneck, so the idea was born to access that data uncached.
>>
>> Ack.
>>
>>>
>>>
>>> Well that was nearly 30years ago, PCI, AGP and front side bus are long gone, but the concept of putting video controller (GPU) stuff into uncached system memory has prevailed.
>>>
>>> So for example even modern AMD CPU based laptops need uncached system memory if their local memory is not large enough to contain the picture to display on the monitor. And with modern 8k monitors that can actually happen quite fast...
>>>
>>> What drivers do today is to call vmf_insert_pfn_prot() either with the PFN of their local memory (iomem) or uncached/wc system memory.
>>
>> That makes perfect sense. I assume we might or might not have "struct
>> page" (pfn_valid) for the iomem, depending on where these areas reside,
>> correct?
>>
>>>
>>>
>>> To summarize that we have an interface to fill in the page tables with either iomem or system memory is actually part of the design. That's how the HW driver is expected to work.
>>>
>>>>> That drivers need to call set_pages_wc/uc() for the linear mapping on x86 manually is correct and checking that is clearly a good idea for debug kernels.
>>>>
>>>> I'll have to think about this a bit: assuming only vmf_insert_pfn() calls pfnmap_setup_cachemode_pfn() but vmf_insert_pfn_prot() doesn't, how could we sanity check that somebody is doing something against the will of PAT.
>>>
>>> I think the most defensive approach for a quick fix is this change here:
>>>
>>>     static inline void pgprot_set_cachemode(pgprot_t *prot, enum page_cache_mode pcm)
>>>     {
>>> -       *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
>>> -                        cachemode2protval(pcm));
>>> +       if (pcm != _PAGE_CACHE_MODE_WB)
>>> +               *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
>>> +                                cachemode2protval(pcm));
>>>     }
>>>
>>> This applies the PAT value if it's anything else than _PAGE_CACHE_MODE_WB but still allows callers to use something different on normal WB system memory.
>>>
>>> What do you think?
>>
>> This feels like too big of a hammer. In particular, it changes things
>> like phys_mem_access_prot_allowed(), which requires more care.
>>
>> First, I thought we should limit what we do to vmf_insert_pfn_prot()
>> only. But then I realized that we have stuff like
>>
>> 	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>>
>> I'm still trying to find out the easy way out that is not a complete hack.
>>
>> Will the iomem ever be mapped by the driver again with a different cache
>> mode? (e.g., WB -> UC -> WB)
> 
> What I am currently wondering is: assume we get a
> pfnmap_setup_cachemode_pfn() call and we could reliably identify whether
> there was a previous registration, then we could do
> 
> (a) No previous registration: don't modify pgprot. Hopefully the driver
>       knows what it is doing. Maybe we can add sanity checks that the
>       direct map was already updated etc.
> 
> (b) A previous registration: modify pgprot like we do today.
> 
> System RAM is the problem. I wonder how many of these registrations we
> really get and if we could just store them in the same tree as !system
> RAM instead of abusing page flags.

commit 9542ada803198e6eba29d3289abb39ea82047b92
Author: Suresh Siddha <suresh.b.siddha@intel.com>
Date:   Wed Sep 24 08:53:33 2008 -0700

     x86: track memtype for RAM in page struct
     
     Track the memtype for RAM pages in page struct instead of using the
     memtype list. This avoids the explosion in the number of entries in
     memtype list (of the order of 20,000 with AGP) and makes the PAT
     tracking simpler.
     
     We are using PG_arch_1 bit in page->flags.
     
     We still use the memtype list for non RAM pages.


I do wonder if that explosion is still an issue today.


-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: stupid and complicated PAT :)
  2025-08-28 21:32                         ` David Hildenbrand
@ 2025-08-29 10:50                           ` Christian König
  2025-08-29 19:52                             ` David Hildenbrand
  0 siblings, 1 reply; 62+ messages in thread
From: Christian König @ 2025-08-29 10:50 UTC (permalink / raw)
  To: David Hildenbrand, intel-xe, intel-gfx, dri-devel, amd-gfx, x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

You write mails faster than I can answer :)

I will try to answer all questions and comment here, here but please ping me if I miss something.

On 28.08.25 23:32, David Hildenbrand wrote:
> On 28.08.25 23:28, David Hildenbrand wrote:
>> On 28.08.25 23:18, David Hildenbrand wrote:
>>> On 26.08.25 18:09, Christian König wrote:
>>>> On 26.08.25 14:07, David Hildenbrand wrote:
[SNIP]
>>>> Well that was nearly 30years ago, PCI, AGP and front side bus are long gone, but the concept of putting video controller (GPU) stuff into uncached system memory has prevailed.
>>>>
>>>> So for example even modern AMD CPU based laptops need uncached system memory if their local memory is not large enough to contain the picture to display on the monitor. And with modern 8k monitors that can actually happen quite fast...
>>>>
>>>> What drivers do today is to call vmf_insert_pfn_prot() either with the PFN of their local memory (iomem) or uncached/wc system memory.
>>>
>>> That makes perfect sense. I assume we might or might not have "struct
>>> page" (pfn_valid) for the iomem, depending on where these areas reside,
>>> correct?

Exactly that, yes.

>>>> To summarize that we have an interface to fill in the page tables with either iomem or system memory is actually part of the design. That's how the HW driver is expected to work.
>>>>
>>>>>> That drivers need to call set_pages_wc/uc() for the linear mapping on x86 manually is correct and checking that is clearly a good idea for debug kernels.
>>>>>
>>>>> I'll have to think about this a bit: assuming only vmf_insert_pfn() calls pfnmap_setup_cachemode_pfn() but vmf_insert_pfn_prot() doesn't, how could we sanity check that somebody is doing something against the will of PAT.
>>>>
>>>> I think the most defensive approach for a quick fix is this change here:
>>>>
>>>>     static inline void pgprot_set_cachemode(pgprot_t *prot, enum page_cache_mode pcm)
>>>>     {
>>>> -       *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
>>>> -                        cachemode2protval(pcm));
>>>> +       if (pcm != _PAGE_CACHE_MODE_WB)
>>>> +               *prot = __pgprot((pgprot_val(*prot) & ~_PAGE_CACHE_MASK) |
>>>> +                                cachemode2protval(pcm));
>>>>     }
>>>>
>>>> This applies the PAT value if it's anything else than _PAGE_CACHE_MODE_WB but still allows callers to use something different on normal WB system memory.
>>>>
>>>> What do you think?
>>>
>>> This feels like too big of a hammer. In particular, it changes things
>>> like phys_mem_access_prot_allowed(), which requires more care.
>>>
>>> First, I thought we should limit what we do to vmf_insert_pfn_prot()
>>> only. But then I realized that we have stuff like
>>>
>>>     vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>>>
>>> I'm still trying to find out the easy way out that is not a complete hack.

Well I think when the change is limited to only to vmf_insert_pfn_prot() for now we can limit the risk quite a bit as well.

Background is that only a handful of callers are using vmf_insert_pfn_prot() and it looks like all of those actually do know what they are doing and using the right flags.

>>> Will the iomem ever be mapped by the driver again with a different cache
>>> mode? (e.g., WB -> UC -> WB)

Yes, that can absolutely happen. But for iomem we would have an explicit call to ioremap(), ioremap_wc(), ioremap_cache() for that before anybody would map anything into userspace page tables.

But thinking more about it I just had an OMFG moment! Is it possible that the PAT currently already has a problem with that?

We had customer projects where BARs of different PCIe devices ended up on different physical addresses after a hot remove/re-add.

Is it possible that the PAT keeps enforcing certain caching attributes for a physical address? E.g. for example because a driver doesn't clean up properly on hot remove?

If yes than that would explain a massive number of problems we had with hot add/remove.

>> What I am currently wondering is: assume we get a
>> pfnmap_setup_cachemode_pfn() call and we could reliably identify whether
>> there was a previous registration, then we could do
>>
>> (a) No previous registration: don't modify pgprot. Hopefully the driver
>>       knows what it is doing. Maybe we can add sanity checks that the
>>       direct map was already updated etc.
>> (b) A previous registration: modify pgprot like we do today.

That would work for me.

>> System RAM is the problem. I wonder how many of these registrations we
>> really get and if we could just store them in the same tree as !system
>> RAM instead of abusing page flags.
> 
> commit 9542ada803198e6eba29d3289abb39ea82047b92
> Author: Suresh Siddha <suresh.b.siddha@intel.com>
> Date:   Wed Sep 24 08:53:33 2008 -0700
> 
>     x86: track memtype for RAM in page struct
>         Track the memtype for RAM pages in page struct instead of using the
>     memtype list. This avoids the explosion in the number of entries in
>     memtype list (of the order of 20,000 with AGP) and makes the PAT
>     tracking simpler.
>         We are using PG_arch_1 bit in page->flags.
>         We still use the memtype list for non RAM pages.
> 
> 
> I do wonder if that explosion is still an issue today.

Yes it is. That is exactly the issue I'm working on here.

It's just that AGP was replaced by internal GPU MMUs over time and so we don't use the old AGP code any more but just call get_free_pages() (or similar) directly.

Thanks a lot for the help,
Christian.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: stupid and complicated PAT :)
  2025-08-29 10:50                           ` Christian König
@ 2025-08-29 19:52                             ` David Hildenbrand
  2025-08-29 19:58                               ` David Hildenbrand
  0 siblings, 1 reply; 62+ messages in thread
From: David Hildenbrand @ 2025-08-29 19:52 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes


> 
> Yes, that can absolutely happen. But for iomem we would have an explicit call to ioremap(), ioremap_wc(), ioremap_cache() for that before anybody would map anything into userspace page tables.
> 
> But thinking more about it I just had an OMFG moment! Is it possible that the PAT currently already has a problem with that?
> 
> We had customer projects where BARs of different PCIe devices ended up on different physical addresses after a hot remove/re-add.
> 
> Is it possible that the PAT keeps enforcing certain caching attributes for a physical address? E.g. for example because a driver doesn't clean up properly on hot remove?
> 
> If yes than that would explain a massive number of problems we had with hot add/remove.

The code is a mess, so if a driver messed up, likely everything is possible.

TBH, the more I look at this all, the more WTF moments I am having.

> 
>>> What I am currently wondering is: assume we get a
>>> pfnmap_setup_cachemode_pfn() call and we could reliably identify whether
>>> there was a previous registration, then we could do
>>>
>>> (a) No previous registration: don't modify pgprot. Hopefully the driver
>>>        knows what it is doing. Maybe we can add sanity checks that the
>>>        direct map was already updated etc.
>>> (b) A previous registration: modify pgprot like we do today.
> 
> That would work for me.
> 
>>> System RAM is the problem. I wonder how many of these registrations we
>>> really get and if we could just store them in the same tree as !system
>>> RAM instead of abusing page flags.
>>
>> commit 9542ada803198e6eba29d3289abb39ea82047b92
>> Author: Suresh Siddha <suresh.b.siddha@intel.com>
>> Date:   Wed Sep 24 08:53:33 2008 -0700
>>
>>      x86: track memtype for RAM in page struct
>>          Track the memtype for RAM pages in page struct instead of using the
>>      memtype list. This avoids the explosion in the number of entries in
>>      memtype list (of the order of 20,000 with AGP) and makes the PAT
>>      tracking simpler.
>>          We are using PG_arch_1 bit in page->flags.
>>          We still use the memtype list for non RAM pages.
>>
>>
>> I do wonder if that explosion is still an issue today.
> 
> Yes it is. That is exactly the issue I'm working on here.
> 
> It's just that AGP was replaced by internal GPU MMUs over time and so we don't use the old AGP code any more but just call get_free_pages() (or similar) directly.

Okay, I thought I slowly understood how it works, then I stumbled over 
the set_memory_uc / set_memory_wc implementation and now I am *all 
confused*.

I mean, that does perform a PAT reservation.

But when is that reservation ever freed again? :/

How can set_memory_wc() followed by set_memory_uc() possibly work? I am 
pretty sure I am missing a piece of the puzzle.

I think you mentioned that set_memory_uc() is avoided by drivers because 
of highmem mess, but what are drivers then using to modify the direct map?

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: stupid and complicated PAT :)
  2025-08-29 19:52                             ` David Hildenbrand
@ 2025-08-29 19:58                               ` David Hildenbrand
  0 siblings, 0 replies; 62+ messages in thread
From: David Hildenbrand @ 2025-08-29 19:58 UTC (permalink / raw)
  To: Christian König, intel-xe, intel-gfx, dri-devel, amd-gfx,
	x86
  Cc: airlied, thomas.hellstrom, matthew.brost, dave.hansen, luto,
	peterz, Lorenzo Stoakes

On 29.08.25 21:52, David Hildenbrand wrote:
> 
>>
>> Yes, that can absolutely happen. But for iomem we would have an explicit call to ioremap(), ioremap_wc(), ioremap_cache() for that before anybody would map anything into userspace page tables.
>>
>> But thinking more about it I just had an OMFG moment! Is it possible that the PAT currently already has a problem with that?
>>
>> We had customer projects where BARs of different PCIe devices ended up on different physical addresses after a hot remove/re-add.
>>
>> Is it possible that the PAT keeps enforcing certain caching attributes for a physical address? E.g. for example because a driver doesn't clean up properly on hot remove?
>>
>> If yes than that would explain a massive number of problems we had with hot add/remove.
> 
> The code is a mess, so if a driver messed up, likely everything is possible.
> 
> TBH, the more I look at this all, the more WTF moments I am having.
> 
>>
>>>> What I am currently wondering is: assume we get a
>>>> pfnmap_setup_cachemode_pfn() call and we could reliably identify whether
>>>> there was a previous registration, then we could do
>>>>
>>>> (a) No previous registration: don't modify pgprot. Hopefully the driver
>>>>         knows what it is doing. Maybe we can add sanity checks that the
>>>>         direct map was already updated etc.
>>>> (b) A previous registration: modify pgprot like we do today.
>>
>> That would work for me.
>>
>>>> System RAM is the problem. I wonder how many of these registrations we
>>>> really get and if we could just store them in the same tree as !system
>>>> RAM instead of abusing page flags.
>>>
>>> commit 9542ada803198e6eba29d3289abb39ea82047b92
>>> Author: Suresh Siddha <suresh.b.siddha@intel.com>
>>> Date:   Wed Sep 24 08:53:33 2008 -0700
>>>
>>>       x86: track memtype for RAM in page struct
>>>           Track the memtype for RAM pages in page struct instead of using the
>>>       memtype list. This avoids the explosion in the number of entries in
>>>       memtype list (of the order of 20,000 with AGP) and makes the PAT
>>>       tracking simpler.
>>>           We are using PG_arch_1 bit in page->flags.
>>>           We still use the memtype list for non RAM pages.
>>>
>>>
>>> I do wonder if that explosion is still an issue today.
>>
>> Yes it is. That is exactly the issue I'm working on here.
>>
>> It's just that AGP was replaced by internal GPU MMUs over time and so we don't use the old AGP code any more but just call get_free_pages() (or similar) directly.
> 
> Okay, I thought I slowly understood how it works, then I stumbled over
> the set_memory_uc / set_memory_wc implementation and now I am *all
> confused*.
> 
> I mean, that does perform a PAT reservation.
> 
> But when is that reservation ever freed again? :/

Ah, set_memory_wb() does that. It just frees stuff. It should have been 
called something like "reset", probably.

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2025-08-29 19:58 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250820143739.3422-1-christian.koenig@amd.com>
2025-08-20 14:33 ` [PATCH 1/3] drm/ttm: use apply_page_range instead of vmf_insert_pfn_prot Christian König
2025-08-20 14:33 ` [PATCH 2/3] drm/ttm: reapply increase ttm pre-fault value to PMD size" Christian König
2025-08-20 14:33 ` [PATCH 3/3] drm/ttm: disable changing the global caching flags on newer AMD CPUs v2 Christian König
2025-08-20 15:12   ` Borislav Petkov
2025-08-20 15:23 ` David Hildenbrand
2025-08-21  8:10   ` Re: Christian König
2025-08-25 19:10     ` Re: David Hildenbrand
2025-08-26  8:38       ` Re: Christian König
2025-08-26  8:46         ` Re: David Hildenbrand
2025-08-26  9:00           ` Re: Christian König
2025-08-26  9:17             ` Re: David Hildenbrand
2025-08-26  9:56               ` Re: Christian König
2025-08-26 12:07                 ` Re: David Hildenbrand
2025-08-26 16:09                   ` Re: Christian König
2025-08-27  9:13                     ` [PATCH 0/3] drm/ttm: Michel Dänzer
2025-08-28 21:18                     ` stupid and complicated PAT :) David Hildenbrand
2025-08-28 21:28                       ` David Hildenbrand
2025-08-28 21:32                         ` David Hildenbrand
2025-08-29 10:50                           ` Christian König
2025-08-29 19:52                             ` David Hildenbrand
2025-08-29 19:58                               ` David Hildenbrand
2025-08-26 14:27                 ` Thomas Hellström
2025-08-28 21:01                   ` stupid PAT :) David Hildenbrand
2025-08-26 12:37         ` David Hildenbrand
2025-08-21  9:16   ` your mail Lorenzo Stoakes
2025-08-21  9:30     ` David Hildenbrand
2025-08-21 10:05       ` Lorenzo Stoakes
2025-08-21 10:16         ` David Hildenbrand
2025-08-25 18:35         ` Christian König
2025-08-25 19:20           ` David Hildenbrand
2023-11-11  4:21 Andrew Worsley
2023-11-11  8:22 ` Javier Martinez Canillas
  -- strict thread matches above, loose matches on Subject: below --
2022-05-19  9:54 Christian König
2022-05-19 10:50 ` Matthew Auld
2022-05-20  7:11   ` Re: Christian König
2022-04-06  7:51 Christian König
2022-04-06 12:59 ` Daniel Vetter
     [not found] <CAGsV3ysM+p_HAq+LgOe4db09e+zRtvELHUQzCjF8FVE2UF+3Ow@mail.gmail.com>
2021-06-29 13:52 ` Re: Alex Deucher
2021-05-15 22:57 Dmitry Baryshkov
2021-06-02 21:45 ` Dmitry Baryshkov
     [not found] <20201008181606.460499-1-sandy.8925@gmail.com>
2020-10-09  6:47 ` Re: Thomas Zimmermann
2020-10-09  7:14   ` Re: Thomas Zimmermann
2020-10-09  7:38     ` Re: Sandeep Raghuraman
2020-10-09  7:51       ` Re: Thomas Zimmermann
2020-10-09 15:48         ` Re: Alex Deucher
2020-09-15  2:40 Dave Airlie
2020-09-15  7:53 ` Christian König
     [not found] <86d0ec$ae4ffc@fmsmga001.fm.intel.com>
2020-02-26 12:08 ` Re: Linus Walleij
2020-02-26 14:34   ` Re: Ville Syrjälä
2020-02-26 14:56     ` Re: Linus Walleij
2020-02-26 15:08       ` Re: Ville Syrjälä
2018-10-21 16:25 (unknown), Michael Tirado
2018-10-22  0:26 ` Dave Airlie
2018-10-21 20:23   ` Re: Michael Tirado
2018-10-22  1:50     ` Re: Dave Airlie
2018-10-21 22:20       ` Re: Michael Tirado
2018-10-23  1:47       ` Re: Michael Tirado
2018-10-23  6:23         ` Re: Dave Airlie
2018-03-05 17:06 (unknown) Meghana Madhyastha
2018-03-05 19:24 ` Noralf Trønnes
2014-08-24 13:14 (unknown), Christian König
2014-08-24 13:34 ` Mike Lothian
2014-08-25  9:10   ` Re: Christian König
2014-09-07 13:24     ` Re: Markus Trippelsdorf
2014-09-08  3:47       ` Re: Alex Deucher
2014-09-08  7:13         ` Re: Markus Trippelsdorf
2013-08-13  9:56 (unknown), Christian König
2013-08-13 14:47 ` Alex Deucher
2012-05-18 12:27 (unknown), Sascha Hauer
2012-05-22 14:06 ` Lars-Peter Clausen
2012-05-23  8:12   ` Re: Sascha Hauer
2012-05-24  6:31   ` Re: Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).