Re: [PATCH/RFT] fbdev driver for HP Visualize FX cards
(cc'ing dri-devel) Hi Am 31.10.21 um 20:53 schrieb Sven Schnelle: Hi List(s), i wrote a fbdev driver for the HP Visualize FX cards used some of the PA-RISC workstations. It utilizes some of the 2D acceleration features present in the card. What is working right now: - modesetting (tested all VESA modes between 640x480 - 1600x1200). - hardware cursor - 2D acceleration like imageblit and fillrect. What is not (fully) working: - X11 with fbdev. xinit + mwm looks almost ok, except some corruption where the text cursor was drawn when it is moved. - more advanced X11 programs. xfce4-session doesn't really show much. I would be most interested if people could test this driver with their FX cards and report. I know that Visualize FXe doesn't work because it uses completely different register addresses. For FX10 i would hope that they share the same register set as they are based on the same architecture. Note that you have to disable the sticon driver, otherwhise that one would hog the PCI card and visualizefx would never be probed. Not sure about FX2/4/6. Might be different, might be not. I obtained all the knowledge about the registers by watching what STI and the HP-UX X Server writes into the card. So the register names might be called different compared to what HP has written in their data books. Thanks for all the work you put into this. We welcome drivers even for older hardware, but not for fbdev. DRM is all the rage now and has been for a while. I'd like to ask you to convert the driver to DRM and resubmit to . I while ago, I made conversion helpers for this. You can look at [1] for a trivial DRM drivers that wraps existing fbdev drivers for use with DRM. Once you have that, it turns into a refactoring job. Best regards Thomas [1] https://gitlab.freedesktop.org/tzimmermann/linux/-/commits/fbconv-plus-drivers Sven. -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[PATCH 0/2] drm: Small CMA cleanups
Remove CMA dependencies from MIPI-DBI and replace the config symbol DRM_KMS_CMA_HELPER with DRM_KMS_HELPER. This allows to link drm_fb_cma_helper.o into a helper library. Thomas Zimmermann (2): drm/mipi-dbi: Remove dependency on GEM CMA helper library drm: Remove CONFIG_DRM_KMS_CMA_HELPER option drivers/gpu/drm/Kconfig | 7 - drivers/gpu/drm/Makefile| 2 +- drivers/gpu/drm/arm/Kconfig | 2 -- drivers/gpu/drm/arm/display/Kconfig | 1 - drivers/gpu/drm/aspeed/Kconfig | 1 - drivers/gpu/drm/atmel-hlcdc/Kconfig | 1 - drivers/gpu/drm/drm_mipi_dbi.c | 34 ++--- drivers/gpu/drm/fsl-dcu/Kconfig | 1 - drivers/gpu/drm/hisilicon/kirin/Kconfig | 1 - drivers/gpu/drm/imx/Kconfig | 2 +- drivers/gpu/drm/imx/dcss/Kconfig| 2 +- drivers/gpu/drm/ingenic/Kconfig | 1 - drivers/gpu/drm/kmb/Kconfig | 1 - drivers/gpu/drm/mcde/Kconfig| 1 - drivers/gpu/drm/meson/Kconfig | 1 - drivers/gpu/drm/mxsfb/Kconfig | 2 +- drivers/gpu/drm/panel/Kconfig | 2 +- drivers/gpu/drm/pl111/Kconfig | 1 - drivers/gpu/drm/rcar-du/Kconfig | 1 - drivers/gpu/drm/shmobile/Kconfig| 1 - drivers/gpu/drm/sti/Kconfig | 1 - drivers/gpu/drm/stm/Kconfig | 1 - drivers/gpu/drm/sun4i/Kconfig | 1 - drivers/gpu/drm/tidss/Kconfig | 1 - drivers/gpu/drm/tilcdc/Kconfig | 1 - drivers/gpu/drm/tiny/Kconfig| 20 +++ drivers/gpu/drm/tve200/Kconfig | 1 - drivers/gpu/drm/vc4/Kconfig | 1 - drivers/gpu/drm/xlnx/Kconfig| 1 - 29 files changed, 40 insertions(+), 53 deletions(-) -- 2.33.1
[PATCH 1/2] drm/mipi-dbi: Remove dependency on GEM CMA helper library
The MIPI DBI helpers access struct drm_gem_cma_object.vaddr in a
few places. Replace all instances with the correct generic GEM
functions. Use drm_gem_fb_vmap() for mapping a framebuffer's GEM
objects and drm_gem_fb_vunmap() for unmapping them. This removes
the dependency on CMA helpers within MIPI DBI.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_mipi_dbi.c | 34 +-
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 71b646c4131f..f80fd6c0ccf8 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -15,9 +15,10 @@
#include
#include
#include
-#include
+#include
#include
#include
+#include
#include
#include
#include
@@ -200,13 +201,19 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer
*fb,
struct drm_rect *clip, bool swap)
{
struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
- struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(gem);
- void *src = cma_obj->vaddr;
+ struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
+ struct dma_buf_map data[DRM_FORMAT_MAX_PLANES];
+ void *src;
int ret;
ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
if (ret)
return ret;
+ src = data[0].vaddr; /* TODO: Use mapping abstraction properly */
+
+ ret = drm_gem_fb_vmap(fb, map, data);
+ if (ret)
+ goto out_drm_gem_fb_end_cpu_access;
switch (fb->format->format) {
case DRM_FORMAT_RGB565:
@@ -221,9 +228,11 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer
*fb,
default:
drm_err_once(fb->dev, "Format is not supported: %p4cc\n",
&fb->format->format);
- return -EINVAL;
+ ret = -EINVAL;
}
+ drm_gem_fb_vunmap(fb, map);
+out_drm_gem_fb_end_cpu_access:
drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
return ret;
@@ -249,8 +258,8 @@ static void mipi_dbi_set_window_address(struct mipi_dbi_dev
*dbidev,
static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect
*rect)
{
- struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
- struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(gem);
+ struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
+ struct dma_buf_map data[DRM_FORMAT_MAX_PLANES];
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(fb->dev);
unsigned int height = rect->y2 - rect->y1;
unsigned int width = rect->x2 - rect->x1;
@@ -266,6 +275,10 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
struct drm_rect *rect)
if (!drm_dev_enter(fb->dev, &idx))
return;
+ ret = drm_gem_fb_vmap(fb, map, data);
+ if (ret)
+ goto err_drm_dev_exit;
+
full = width == fb->width && height == fb->height;
DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id,
DRM_RECT_ARG(rect));
@@ -277,7 +290,7 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
struct drm_rect *rect)
if (ret)
goto err_msg;
} else {
- tr = cma_obj->vaddr;
+ tr = data[0].vaddr; /* TODO: Use mapping abstraction properly */
}
mipi_dbi_set_window_address(dbidev, rect->x1, rect->x2 - 1, rect->y1,
@@ -289,6 +302,9 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
struct drm_rect *rect)
if (ret)
drm_err_once(fb->dev, "Failed to update display %d\n", ret);
+ drm_gem_fb_vunmap(fb, map);
+
+err_drm_dev_exit:
drm_dev_exit(idx);
}
@@ -1117,8 +1133,8 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct
mipi_dbi *dbi,
/*
* Even though it's not the SPI device that does DMA (the master does),
-* the dma mask is necessary for the dma_alloc_wc() in
-* drm_gem_cma_create(). The dma_addr returned will be a physical
+* the dma mask is necessary for the dma_alloc_wc() in the GEM code
+* (e.g., drm_gem_cma_create()). The dma_addr returned will be a
physical
* address which might be different from the bus address, but this is
* not a problem since the address will not be used.
* The virtual address is used in the transfer and the SPI core
--
2.33.1
[PATCH 2/2] drm: Remove CONFIG_DRM_KMS_CMA_HELPER option
Link drm_fb_cma_helper.o into drm_cma_helper.ko if CONFIG_DRM_KMS_HELPER has been set. Remove CONFIG_DRM_KMS_CMA_HELPER config option. Selecting KMS helpers and CMA will now automatically enable CMA KMS helpers. Some drivers' Kconfig files did not correctly select KMS or CMA helpers. Fix this as part of the change. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/Kconfig | 7 --- drivers/gpu/drm/Makefile| 2 +- drivers/gpu/drm/arm/Kconfig | 2 -- drivers/gpu/drm/arm/display/Kconfig | 1 - drivers/gpu/drm/aspeed/Kconfig | 1 - drivers/gpu/drm/atmel-hlcdc/Kconfig | 1 - drivers/gpu/drm/fsl-dcu/Kconfig | 1 - drivers/gpu/drm/hisilicon/kirin/Kconfig | 1 - drivers/gpu/drm/imx/Kconfig | 2 +- drivers/gpu/drm/imx/dcss/Kconfig| 2 +- drivers/gpu/drm/ingenic/Kconfig | 1 - drivers/gpu/drm/kmb/Kconfig | 1 - drivers/gpu/drm/mcde/Kconfig| 1 - drivers/gpu/drm/meson/Kconfig | 1 - drivers/gpu/drm/mxsfb/Kconfig | 2 +- drivers/gpu/drm/panel/Kconfig | 2 +- drivers/gpu/drm/pl111/Kconfig | 1 - drivers/gpu/drm/rcar-du/Kconfig | 1 - drivers/gpu/drm/shmobile/Kconfig| 1 - drivers/gpu/drm/sti/Kconfig | 1 - drivers/gpu/drm/stm/Kconfig | 1 - drivers/gpu/drm/sun4i/Kconfig | 1 - drivers/gpu/drm/tidss/Kconfig | 1 - drivers/gpu/drm/tilcdc/Kconfig | 1 - drivers/gpu/drm/tiny/Kconfig| 20 ++-- drivers/gpu/drm/tve200/Kconfig | 1 - drivers/gpu/drm/vc4/Kconfig | 1 - drivers/gpu/drm/xlnx/Kconfig| 1 - 28 files changed, 15 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index fb144617055b..5e147593bb6c 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -217,13 +217,6 @@ config DRM_GEM_CMA_HELPER help Choose this if you need the GEM CMA helper functions -config DRM_KMS_CMA_HELPER - bool - depends on DRM - select DRM_GEM_CMA_HELPER - help - Choose this if you need the KMS CMA helper functions - config DRM_GEM_SHMEM_HELPER tristate depends on DRM && MMU diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 1c41156deb5f..7125318d6c95 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -34,6 +34,7 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86. obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o drm_cma_helper-y := drm_gem_cma_helper.o +drm_cma_helper-$(CONFIG_DRM_KMS_HELPER) += drm_fb_cma_helper.o obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o drm_shmem_helper-y := drm_gem_shmem_helper.o @@ -58,7 +59,6 @@ drm_kms_helper-y := drm_bridge_connector.o drm_crtc_helper.o drm_dp_helper.o \ drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o -drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig index 3a9e966e0e78..58a242871b28 100644 --- a/drivers/gpu/drm/arm/Kconfig +++ b/drivers/gpu/drm/arm/Kconfig @@ -6,7 +6,6 @@ config DRM_HDLCD depends on DRM && OF && (ARM || ARM64 || COMPILE_TEST) depends on COMMON_CLK select DRM_KMS_HELPER - select DRM_KMS_CMA_HELPER help Choose this option if you have an ARM High Definition Colour LCD controller. @@ -27,7 +26,6 @@ config DRM_MALI_DISPLAY depends on DRM && OF && (ARM || ARM64 || COMPILE_TEST) depends on COMMON_CLK select DRM_KMS_HELPER - select DRM_KMS_CMA_HELPER select DRM_GEM_CMA_HELPER select VIDEOMODE_HELPERS help diff --git a/drivers/gpu/drm/arm/display/Kconfig b/drivers/gpu/drm/arm/display/Kconfig index cec0639e3aa1..e91598b60781 100644 --- a/drivers/gpu/drm/arm/display/Kconfig +++ b/drivers/gpu/drm/arm/display/Kconfig @@ -4,7 +4,6 @@ config DRM_KOMEDA depends on DRM && OF depends on COMMON_CLK select DRM_KMS_HELPER - select DRM_KMS_CMA_HELPER select DRM_GEM_CMA_HELPER select VIDEOMODE_HELPERS help diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig index 5e95bcea43e9..36c4a7e86981 100644 --- a/drivers/gpu/drm/aspeed/Kconfig +++ b/drivers/gpu/drm/aspeed/Kconfig @@ -5,7 +5,6 @@ config DRM_ASPEED_GFX depends on (COMPILE_TEST || ARCH_ASPEED) depends on MMU select DRM_KMS_HELPER - select DRM_KMS_CMA_HELPER select DMA_CMA if HAVE_DMA_CONTIGUOUS select CMA if HAVE_DMA_CONTIGUOUS select MFD_SYSCON diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfi
Re: linux-next: build failure after merge of the drm-misc tree
Hi all,
On Fri, 15 Oct 2021 20:26:48 +1100 Stephen Rothwell
wrote:
>
> After merging the drm-misc tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
>
> drivers/gpu/drm/drm_modeset_lock.c:111:29: error: conflicting types for
> '__stack_depot_save'
> 111 | static depot_stack_handle_t __stack_depot_save(void)
> | ^~
> In file included from include/linux/page_ext.h:7,
> from include/linux/mm.h:25,
> from include/linux/kallsyms.h:13,
> from include/linux/bpf.h:20,
> from include/linux/bpf-cgroup.h:5,
> from include/linux/cgroup-defs.h:22,
> from include/linux/cgroup.h:28,
> from include/linux/memcontrol.h:13,
> from include/linux/swap.h:9,
> from include/linux/suspend.h:5,
> from include/linux/regulator/consumer.h:35,
> from include/linux/i2c.h:18,
> from include/drm/drm_crtc.h:28,
> from include/drm/drm_atomic.h:31,
> from drivers/gpu/drm/drm_modeset_lock.c:24:
> include/linux/stackdepot.h:18:22: note: previous declaration of
> '__stack_depot_save' was here
>18 | depot_stack_handle_t __stack_depot_save(unsigned long *entries,
> | ^~
>
> Caused by commit
>
> cd06ab2fd48f ("drm/locking: add backtrace for locking contended locks
> without backoff")
>
> This may only have been revealed because of another fix I have had to
> apply today.
>
> I have applied the following patch for today.
>
> From: Stephen Rothwell
> Date: Fri, 15 Oct 2021 20:17:52 +1100
> Subject: [PATCH] drm/locking: fix for name conflict
>
> Fixes: cd06ab2fd48f ("drm/locking: add backtrace for locking contended locks
> without backoff")
> Signed-off-by: Stephen Rothwell
> ---
> drivers/gpu/drm/drm_modeset_lock.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c
> b/drivers/gpu/drm/drm_modeset_lock.c
> index 4d32b61fa1fd..ee36dd20900d 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -79,7 +79,7 @@
> static DEFINE_WW_CLASS(crtc_ww_class);
>
> #if IS_ENABLED(CONFIG_DRM_DEBUG_MODESET_LOCK)
> -static noinline depot_stack_handle_t __stack_depot_save(void)
> +static noinline depot_stack_handle_t __drm_stack_depot_save(void)
> {
> unsigned long entries[8];
> unsigned int n;
> @@ -108,7 +108,7 @@ static void __stack_depot_print(depot_stack_handle_t
> stack_depot)
> kfree(buf);
> }
> #else /* CONFIG_DRM_DEBUG_MODESET_LOCK */
> -static depot_stack_handle_t __stack_depot_save(void)
> +static depot_stack_handle_t __drm_stack_depot_save(void)
> {
> return 0;
> }
> @@ -317,7 +317,7 @@ static inline int modeset_lock(struct drm_modeset_lock
> *lock,
> ret = 0;
> } else if (ret == -EDEADLK) {
> ctx->contended = lock;
> - ctx->stack_depot = __stack_depot_save();
> + ctx->stack_depot = __drm_stack_depot_save();
> }
>
> return ret;
> --
> 2.33.0
This has reappeared today. I don't know what happened to the drm-misc
tree over the weeked :-(
I have reapplied the above fix.
--
Cheers,
Stephen Rothwell
pgpSFDmoVhNxy.pgp
Description: OpenPGP digital signature
Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()
Hi
Am 23.10.21 um 09:49 schrieb Sam Ravnborg:
Hi Thomas,
On Fri, Oct 22, 2021 at 03:28:21PM +0200, Thomas Zimmermann wrote:
Provide a function that computes the offset into a blit destination
buffer. This will allow to move destination-buffer clipping into the
format-helper callers.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_format_helper.c | 10 --
include/drm/drm_format_helper.h | 4
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c
b/drivers/gpu/drm/drm_format_helper.c
index 69fde60e36b3..28e9d0d89270 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -17,12 +17,18 @@
#include
#include
-static unsigned int clip_offset(struct drm_rect *clip,
- unsigned int pitch, unsigned int cpp)
+static unsigned int clip_offset(const struct drm_rect *clip, unsigned int
pitch, unsigned int cpp)
{
return clip->y1 * pitch + clip->x1 * cpp;
}
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
+const struct drm_rect *clip)
+{
+ return clip_offset(clip, pitch, format->cpp[0]);
+}
+EXPORT_SYMBOL(drm_fb_clip_offset);
Exported functions are expected to have kernel-doc documentation.
Just copy more or less from the changelog I think.
That's an oversight. Sorry.
Anywhere else (I looked in struct drm_framebuffer) we only need unsigned
int for offsets and width/length - so I cannot see why we do an unsigned
int => unsigned long conversion here.
On ancient platforms, int was 16 bit wide. So for values that are array
indices or buffer indices, I naturally use long, which is 32-bit at
least. Never mind, it's not relevant any longer. I'll convert this code
to unsigned int.
Best regards
Thomas
Sam
+
/**
* drm_fb_memcpy - Copy clip buffer
* @dst: Destination buffer
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index e86925cf07b9..90b9bd9ecb83 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -6,9 +6,13 @@
#ifndef __LINUX_DRM_FORMAT_HELPER_H
#define __LINUX_DRM_FORMAT_HELPER_H
+struct drm_format_info;
struct drm_framebuffer;
struct drm_rect;
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
+const struct drm_rect *clip);
+
void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip);
void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void
*vaddr,
--
2.33.0
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
[Bug 214621] WARNING: CPU: 3 PID: 521 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0xb64/0xe40 [ttm]
https://bugzilla.kernel.org/show_bug.cgi?id=214621 Lang Yu (lang...@amd.com) changed: What|Removed |Added CC||lang...@amd.com --- Comment #5 from Lang Yu (lang...@amd.com) --- Created attachment 299383 --> https://bugzilla.kernel.org/attachment.cgi?id=299383&action=edit fix a potential dma-buf release warning Please have a try with attached patch. Thanks! -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property
Hi Am 24.10.21 um 17:20 schrieb Noralf Trønnes: Den 22.10.2021 15.28, skrev Thomas Zimmermann: Enable the FB_DAMAGE_CLIPS property to reduce display-update overhead. Also fixes a warning in the kernel log. simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called Fix the computation of the blit rectangle. This wasn't an issue so far, as simpledrm always blitted the full framebuffer. The code now supports damage clipping and virtual screen sizes. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tiny/simpledrm.c | 30 ++ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 571f716ff427..e872121e9fb0 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base; - struct drm_rect clip; + struct drm_rect src_clip, dst_clip; int idx; if (!fb) @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, if (!drm_dev_enter(dev, &idx)) return; - drm_rect_init(&clip, 0, 0, fb->width, fb->height); + drm_rect_fp_to_int(&src_clip, &plane_state->src); - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); + dst_clip = plane_state->dst; + if (!drm_rect_intersect(&dst_clip, &src_clip)) + return; + + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip); drm_dev_exit(idx); } @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = plane_state->fb; struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base; - struct drm_rect clip; + struct drm_rect damage_clip, src_clip, dst_clip; int idx; if (!fb) return; - if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip)) + if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip)) + return; + I'm afraid I don't understand what's going on here, but isn't it possible to put this logic into drm_atomic_helper_damage_merged()? The code above gets the damage rectangle (i.e., the plane's area that needs to be updated). The code below get's the framebuffer area. If they don't overlap, return. (I think this can really only fail with the next patch in the series, which adds virtual screens.) Noralf. + drm_rect_fp_to_int(&src_clip, &plane_state->src); + if (!drm_rect_intersect(&src_clip, &damage_clip)) + return; + + dst_clip = plane_state->dst; + if (!drm_rect_intersect(&dst_clip, &src_clip)) return; And here we check if the updated plane/framebuffer area is actually visible on screen; otherwise return. It could be in an area that is off-screen. (Again, this probably only fails with the virtual-screen patch.) I don't think this is all generic enough to be within drm_atomic_helper_damage_merged(). But once we have multiple SHMEM-based drivers with virtual screens, we can move it into a helper for shadow-buffered planes. Your gud driver would be a candidate for this feature as well. Best regards Thomas if (!drm_dev_enter(dev, &idx)) return; - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip); drm_dev_exit(idx); } @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) if (ret) return ret; + drm_plane_enable_fb_damage_clips(&pipe->plane); + drm_mode_config_reset(dev); return 0; -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[Bug 214901] amdgpu freezes HP laptop at start up
https://bugzilla.kernel.org/show_bug.cgi?id=214901 --- Comment #2 from spassw...@web.de --- There is another error message just before the Oops: Nov 1 00:22:49 bart kernel: [2.137397] amdgpu :00:01.0: amdgpu: amdgpu_device_ip_init failed Nov 1 00:22:49 bart kernel: [2.137402] amdgpu :00:01.0: amdgpu: Fatal error during GPU init Nov 1 00:22:49 bart kernel: [2.137406] amdgpu :00:01.0: amdgpu: amdgpu: finishing device. Nov 1 00:22:49 bart kernel: [2.139639] BUG: kernel NULL pointer dereference, address: 01db -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: linux-next: build warning after merge of the drm-misc tree
Hi Stephen,
On Tue, 5 Oct 2021 18:59:40 +1100 Stephen Rothwell
wrote:
>
> Hi all,
>
> After merging the drm-misc tree, today's linux-next build (htmldocs)
> produced this warning:
>
> include/linux/dma-buf.h:456: warning: Function parameter or member 'cb_in'
> not described in 'dma_buf'
> include/linux/dma-buf.h:456: warning: Function parameter or member 'cb_out'
> not described in 'dma_buf'
>
> Introduced by commit
>
> 6b51b02a3a0a ("dma-buf: fix and rework dma_buf_poll v7")
This is back again as well :-(
--
Cheers,
Stephen Rothwell
pgpekVMUfu2Au.pgp
Description: OpenPGP digital signature
Re: [PATCH/RFT] fbdev driver for HP Visualize FX cards
Hi Am 01.11.21 um 09:54 schrieb Sven Schnelle: Hi Thomas, Thomas Zimmermann writes: Am 31.10.21 um 20:53 schrieb Sven Schnelle: Hi List(s), i wrote a fbdev driver for the HP Visualize FX cards used some of the PA-RISC workstations. It utilizes some of the 2D acceleration features present in the card. [..] Thanks for all the work you put into this. We welcome drivers even for older hardware, but not for fbdev. DRM is all the rage now and has been for a while. I'd like to ask you to convert the driver to DRM and resubmit to . I while ago, I made conversion helpers for this. You can look at [1] for a trivial DRM drivers that wraps existing fbdev drivers for use with DRM. Once you have that, it turns into a refactoring job. Thanks, i wasn't aware as i normally don't do any graphics related development. I take a look at dri and port the driver, which is hopefully not too hard. Sounds good. The one big difference when converting is that DRM really wants drivers to support 32-bit XRGB colors. It's not a DRM limitation per se, but a requirement of today's userspace programs. AFAICS your fbdev driver uses a 256-color palette format. So the DRM driver would have to convert XRGB to 8-bit RGB332 and install a corresponding palette. Don't worry, it's easy. Take a look at the cirrus driver for a simple DRM driver. [1] If you need help, there's . Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/tiny/cirrus.c Sven -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[Bug 214901] amdgpu freezes HP laptop at start up
https://bugzilla.kernel.org/show_bug.cgi?id=214901 --- Comment #3 from spassw...@web.de --- Actually the above message is not complete: Nov 1 00:22:49 bart kernel: [2.136382] kfd kfd: amdgpu: Allocated 3969056 bytes on gart Nov 1 00:22:49 bart kernel: [2.136462] kfd kfd: amdgpu: error getting iommu info. is the iommu enabled? Nov 1 00:22:49 bart kernel: [2.136470] kfd kfd: amdgpu: Error initializing iommuv2 Nov 1 00:22:49 bart kernel: [2.137386] kfd kfd: amdgpu: device 1002:9874 NOT added due to errors Nov 1 00:22:49 bart kernel: [2.137393] kfd kfd: amdgpu: Failed to resume IOMMU for device 1002:9874 Nov 1 00:22:49 bart kernel: [2.137397] amdgpu :00:01.0: amdgpu: amdgpu_device_ip_init failed Nov 1 00:22:49 bart kernel: [2.137402] amdgpu :00:01.0: amdgpu: Fatal error during GPU init Nov 1 00:22:49 bart kernel: [2.137406] amdgpu :00:01.0: amdgpu: amdgpu: finishing device. Nov 1 00:22:49 bart kernel: [2.139639] BUG: kernel NULL pointer dereference, address: 01db The messages from kfd have been there with older kernels, too, but were not fatal. They are caused by the HP Laptop 15-bw0xx/8332, not having a iommu or its BIOS not properly initializing it. But linux-5.15 has added the following lines to the amdgpu_device_ip_init: r = amdgpu_amdkfd_resume_iommu(adev); if (r) goto init_failed; which make causes the amdgpu_device_ip_init function to fail when kfd init fails. As a workaround one could remove these. A BIOS update could perhaps also solve the problem but seems to require a Windows running on the Laptop (which was actually sold without Windows) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [Intel-gfx] [PATCH 02/28] drm/i915: use new iterator in i915_gem_object_wait_reservation
On 28/10/2021 16:30, Daniel Vetter wrote: On Thu, Oct 28, 2021 at 10:41:38AM +0200, Christian König wrote: Am 21.10.21 um 13:13 schrieb Tvrtko Ursulin: On 21/10/2021 12:06, Maarten Lankhorst wrote: Op 21-10-2021 om 12:38 schreef Christian König: Am 21.10.21 um 12:35 schrieb Maarten Lankhorst: From: Christian König Simplifying the code a bit. Signed-off-by: Christian König [mlankhorst: Handle timeout = 0 correctly, use new i915_request_wait_timeout.] Signed-off-by: Maarten Lankhorst LGTM, do you want to push it or should I pick it up into drm-misc-next? I think it can be applied to drm-intel-gt-next, after a backmerge. It needs patch 1 too, which fixes i915_request_wait semantics when used in dma-fence. It exports a dma-fence compatible i915_request_wait_timeout function, used in this patch. What about the other i915 patches? I guess you then want to merge them through drm-intel-gt-next as well. I don't think my open has been resolved, at least I haven't seen a reply from Daniel on the topic of potential for infinite waits with untrusted clients after this change. +Daniel Please resolve that internally and let me know the result. I'm fine to use any of the possible approaches, I just need to know which one. I thought I explained this in the patch set from Maarten. This isn't an issue, since the exact same thing can happen if you get interrupts and stuff. Ah were you trying to point out all this time the infinite wait just got moved from inside the "old" dma_resv_get_fences to the new iterator caller? Regards, Tvrtko The only proper fix for bounding the waits is a) compositor grabs a stable set of dma_fence from the dma-buf through the proposed fence export ioctl b) compositor waits on that fence (or drm_syncobj). Everything else is cargo-culted nonsense, and very much includes that igt patch that's floating around internally. I can also whack this into drm-next if this is stuck in this silly bikeshed. -Daniel
Re: [PATCH 3/6] drm/etnaviv: stop getting the excl fence separately here
Am Donnerstag, dem 28.10.2021 um 15:26 +0200 schrieb Christian König:
> Just grab all fences in one go.
>
> Signed-off-by: Christian König
Reviewed-by: Lucas Stach
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 8dc93863bf96..b5e8ce86dbe7 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -189,7 +189,7 @@ static int submit_fence_sync(struct etnaviv_gem_submit
> *submit)
> continue;
>
> if (bo->flags & ETNA_SUBMIT_BO_WRITE) {
> - ret = dma_resv_get_fences(robj, &bo->excl,
> + ret = dma_resv_get_fences(robj, NULL,
> &bo->nr_shared,
> &bo->shared);
> if (ret)
Re: [Intel-gfx] [PATCH] drm/i915/execlists: Weak parallel submission support for execlists
On 27/10/2021 21:10, Matthew Brost wrote:
On Wed, Oct 27, 2021 at 01:04:49PM -0700, John Harrison wrote:
On 10/27/2021 12:17, Matthew Brost wrote:
On Tue, Oct 26, 2021 at 02:58:00PM -0700, John Harrison wrote:
On 10/20/2021 14:47, Matthew Brost wrote:
A weak implementation of parallel submission (multi-bb execbuf IOCTL) for
execlists. Doing as little as possible to support this interface for
execlists - basically just passing submit fences between each request
generated and virtual engines are not allowed. This is on par with what
is there for the existing (hopefully soon deprecated) bonding interface.
We perma-pin these execlists contexts to align with GuC implementation.
v2:
(John Harrison)
- Drop siblings array as num_siblings must be 1
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 10 +++--
drivers/gpu/drm/i915/gt/intel_context.c | 4 +-
.../drm/i915/gt/intel_execlists_submission.c | 44 ++-
drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 -
5 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index fb33d0322960..35e87a7d0ea9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -570,10 +570,6 @@ set_proto_ctx_engines_parallel_submit(struct
i915_user_extension __user *base,
struct intel_engine_cs **siblings = NULL;
intel_engine_mask_t prev_mask;
- /* FIXME: This is NIY for execlists */
- if (!(intel_uc_uses_guc_submission(&i915->gt.uc)))
- return -ENODEV;
-
if (get_user(slot, &ext->engine_index))
return -EFAULT;
@@ -583,6 +579,12 @@ set_proto_ctx_engines_parallel_submit(struct
i915_user_extension __user *base,
if (get_user(num_siblings, &ext->num_siblings))
return -EFAULT;
+ if (!intel_uc_uses_guc_submission(&i915->gt.uc) && num_siblings != 1) {
+ drm_dbg(&i915->drm, "Only 1 sibling (%d) supported in non-GuC
mode\n",
+ num_siblings);
+ return -EINVAL;
+ }
+
if (slot >= set->num_engines) {
drm_dbg(&i915->drm, "Invalid placement value, %d >= %d\n",
slot, set->num_engines);
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index 5634d14052bc..1bec92e1d8e6 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -79,7 +79,8 @@ static int intel_context_active_acquire(struct intel_context
*ce)
__i915_active_acquire(&ce->active);
- if (intel_context_is_barrier(ce) || intel_engine_uses_guc(ce->engine))
+ if (intel_context_is_barrier(ce) || intel_engine_uses_guc(ce->engine) ||
+ intel_context_is_parallel(ce))
return 0;
/* Preallocate tracking nodes */
@@ -563,7 +564,6 @@ void intel_context_bind_parent_child(struct intel_context
*parent,
* Callers responsibility to validate that this function is used
* correctly but we use GEM_BUG_ON here ensure that they do.
*/
- GEM_BUG_ON(!intel_engine_uses_guc(parent->engine));
GEM_BUG_ON(intel_context_is_pinned(parent));
GEM_BUG_ON(intel_context_is_child(parent));
GEM_BUG_ON(intel_context_is_pinned(child));
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index bedb80057046..2865b422300d 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -927,8 +927,7 @@ static void execlists_submit_ports(struct intel_engine_cs
*engine)
static bool ctx_single_port_submission(const struct intel_context *ce)
{
- return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
- intel_context_force_single_submission(ce));
+ return intel_context_force_single_submission(ce);
I think this is actually going to break GVT.
Not so much this change here but the whole use of single submission outside
of GVT. It looks like the GVT driver overloads the single submission flag to
tag requests that it owns. If we start using that flag elsewhere when GVT is
active, I think that will cause much confusion within the GVT code.
The correct fix would be to create a new flag just for GVT usage alongside
the single submission one. GVT would then set both but only check for its
own private flag. The parallel code would obviously only set the existing
single submission flag.
Ok, see below.
}
static bool can_merge_ctx(const struct intel_context *prev,
@@ -2598,6 +2597,46 @@ static void execlists_context_cancel_request(struct
intel_context *ce,
current->comm);
}
+static struct intel_context *
+
[Bug 214901] amdgpu freezes HP laptop at start up
https://bugzilla.kernel.org/show_bug.cgi?id=214901 --- Comment #4 from spassw...@web.de --- Just confirmed that removing the 3 lines r = amdgpu_amdkfd_resume_iommu(adev); if (r) goto init_failed; can be used as a workaround. Removing only the if (r) check is not enough, just calling amdgpu_amdkfd_resume_iommu(adev) leads to freezing. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 0/4] Add Aspeed AST2600 soc display support
The Aspeed AST2600 soc diaplay support is added in this patch. Because some hw designed is changed in this version, add them in this patch. v2: Remove some unnecessary patch. Refine for reviwer request. v1: First add patch. Joel Stanley (2): ARM: dts: aspeed: Add GFX node to AST2600 ARM: dts: aspeed: ast2600-evb: Enable GFX device tommy-huang (2): drm/aspeed: Update INTR_STS handling dt-bindings: gpu: Add ASPEED GFX bindings document .../devicetree/bindings/gpu/aspeed-gfx.txt| 1 + arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 + arch/arm/boot/dts/aspeed-g6.dtsi | 11 drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 ++ drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 26 --- 5 files changed, 55 insertions(+), 3 deletions(-) -- 2.17.1
[PATCH 1/4] ARM: dts: aspeed: Add GFX node to AST2600
From: Joel Stanley
The GFX device is present in the AST2600 SoC.
Signed-off-by: Joel Stanley
Signed-off-by: tommy-huang
---
arch/arm/boot/dts/aspeed-g6.dtsi | 11 +++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index 1b47be1704f8..e38c3742761b 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -351,6 +351,17 @@
quality = <100>;
};
+ gfx: display@1e6e6000 {
+ compatible = "aspeed,ast2600-gfx",
"aspeed,ast2500-gfx", "syscon";
+ reg = <0x1e6e6000 0x1000>;
+ reg-io-width = <4>;
+ clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
+ resets = <&syscon ASPEED_RESET_GRAPHICS>;
+ syscon = <&syscon>;
+ status = "disabled";
+ interrupts = ;
+ };
+
xdma: xdma@1e6e7000 {
compatible = "aspeed,ast2600-xdma";
reg = <0x1e6e7000 0x100>;
--
2.17.1
[PATCH 2/4] ARM: dts: aspeed: ast2600-evb: Enable GFX device
From: Joel Stanley
Enable the GFX device with a framebuffer memory region.
Signed-off-by: Joel Stanley
Signed-off-by: tommy-huang
---
arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index b7eb552640cb..e223dad2abd0 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -23,6 +23,19 @@
reg = <0x8000 0x8000>;
};
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ gfx_memory: framebuffer {
+ size = <0x0100>;
+ alignment = <0x0100>;
+ compatible = "shared-dma-pool";
+ reusable;
+ };
+ };
+
vcc_sdhci0: regulator-vcc-sdhci0 {
compatible = "regulator-fixed";
regulator-name = "SDHCI0 Vcc";
@@ -300,3 +313,8 @@
vqmmc-supply = <&vccq_sdhci1>;
clk-phase-sd-hs = <7>, <200>;
};
+
+&gfx {
+ status = "okay";
+ memory-region = <&gfx_memory>;
+};
--
2.17.1
[PATCH 3/4] drm/aspeed: Update INTR_STS handling
The V-sync INTR_STS is differnet on AST2600.
Change into general rule to handle it.
Signed-off-by: tommy-huang
---
drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 ++
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 26 ++---
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h
b/drivers/gpu/drm/aspeed/aspeed_gfx.h
index 96501152bafa..5eed9275bce7 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx.h
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
@@ -12,6 +12,8 @@ struct aspeed_gfx {
struct regmap *scu;
u32 dac_reg;
+ u32 int_reg;
+ u32 int_clr_reg;
u32 vga_scratch_reg;
u32 throd_val;
u32 scan_line_max;
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
index b53fee6f1c17..1092060cb59c 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -60,6 +60,8 @@
struct aspeed_gfx_config {
u32 dac_reg;/* DAC register in SCU */
+ u32 int_status_reg; /* Interrupt status register */
+ u32 int_clear_reg; /* Interrupt clear register */
u32 vga_scratch_reg;/* VGA scratch register in SCU */
u32 throd_val; /* Default Threshold Seting */
u32 scan_line_max; /* Max memory size of one scan line */
@@ -67,6 +69,8 @@ struct aspeed_gfx_config {
static const struct aspeed_gfx_config ast2400_config = {
.dac_reg = 0x2c,
+ .int_status_reg = 0x60,
+ .int_clear_reg = 0x60,
.vga_scratch_reg = 0x50,
.throd_val = CRT_THROD_LOW(0x1e) | CRT_THROD_HIGH(0x12),
.scan_line_max = 64,
@@ -74,14 +78,26 @@ static const struct aspeed_gfx_config ast2400_config = {
static const struct aspeed_gfx_config ast2500_config = {
.dac_reg = 0x2c,
+ .int_status_reg = 0x60,
+ .int_clear_reg = 0x60,
.vga_scratch_reg = 0x50,
.throd_val = CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3c),
.scan_line_max = 128,
};
+static const struct aspeed_gfx_config ast2600_config = {
+ .dac_reg = 0xc0,
+ .int_status_reg = 0x60,
+ .int_clear_reg = 0x68,
+ .vga_scratch_reg = 0x50,
+ .throd_val = CRT_THROD_LOW(0x50) | CRT_THROD_HIGH(0x70),
+ .scan_line_max = 128,
+};
+
static const struct of_device_id aspeed_gfx_match[] = {
{ .compatible = "aspeed,ast2400-gfx", .data = &ast2400_config },
{ .compatible = "aspeed,ast2500-gfx", .data = &ast2500_config },
+ { .compatible = "aspeed,ast2600-gfx", .data = &ast2600_config },
{ },
};
MODULE_DEVICE_TABLE(of, aspeed_gfx_match);
@@ -113,13 +129,15 @@ static irqreturn_t aspeed_gfx_irq_handler(int irq, void
*data)
{
struct drm_device *drm = data;
struct aspeed_gfx *priv = to_aspeed_gfx(drm);
- u32 reg;
+ u32 reg, clr_reg;
- reg = readl(priv->base + CRT_CTRL1);
+ reg = readl(priv->base + priv->int_reg);
if (reg & CRT_CTRL_VERTICAL_INTR_STS) {
drm_crtc_handle_vblank(&priv->pipe.crtc);
- writel(reg, priv->base + CRT_CTRL1);
+ clr_reg = (readl(priv->base + priv->int_clr_reg) |
+ CRT_CTRL_VERTICAL_INTR_STS);
+ writel(clr_reg, priv->base + priv->int_clr_reg);
return IRQ_HANDLED;
}
@@ -147,6 +165,8 @@ static int aspeed_gfx_load(struct drm_device *drm)
config = match->data;
priv->dac_reg = config->dac_reg;
+ priv->int_reg = config->int_status_reg;
+ priv->int_clr_reg = config->int_clear_reg;
priv->vga_scratch_reg = config->vga_scratch_reg;
priv->throd_val = config->throd_val;
priv->scan_line_max = config->scan_line_max;
--
2.17.1
[PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document
Add ast2600-gfx description for gfx driver. Signed-off-by: tommy-huang --- Documentation/devicetree/bindings/gpu/aspeed-gfx.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt index 958bdf962339..29ecf119cef2 100644 --- a/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt +++ b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt @@ -3,6 +3,7 @@ Device tree configuration for the GFX display device on the ASPEED SoCs Required properties: - compatible * Must be one of the following: + + aspeed,ast2600-gfx + aspeed,ast2500-gfx + aspeed,ast2400-gfx * In addition, the ASPEED pinctrl bindings require the 'syscon' property to -- 2.17.1
Re: [PATCH] drm/rockchip: use generic fbdev setup
On Sun, 31 Oct 2021 19:09:39 +0100
Thomas Zimmermann wrote:
> Am 30.10.21 um 14:05 schrieb John Keeping:
> > On Fri, Oct 29, 2021 at 09:00:08PM +0200, Thomas Zimmermann wrote:
> >> Am 29.10.21 um 13:50 schrieb John Keeping:
> >>> The Rockchip fbdev code does not add anything compared to
> >>> drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the
> >>> same thing as gem_prime_mmap which is called by the helper.
> >>>
> >>> Signed-off-by: John Keeping
> >>> ---
> >>>drivers/gpu/drm/rockchip/Makefile | 1 -
> >>>drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +-
> >>>drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 -
> >>>drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 --
> >>>drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 ---
> >>>5 files changed, 2 insertions(+), 199 deletions(-)
> >>>delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
> >>>delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h
> >>>
> >>> diff --git a/drivers/gpu/drm/rockchip/Makefile
> >>> b/drivers/gpu/drm/rockchip/Makefile
> >>> index 17a9e7eb2130..1a56f696558c 100644
> >>> --- a/drivers/gpu/drm/rockchip/Makefile
> >>> +++ b/drivers/gpu/drm/rockchip/Makefile
> >>> @@ -5,7 +5,6 @@
> >>>rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> >>> rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o
> >>> -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
> >>>rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> >>>rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> >>> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> >>> index 69c699459dce..20d81ae69828 100644
> >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> >>> @@ -26,7 +26,6 @@
> >>>#include "rockchip_drm_drv.h"
> >>>#include "rockchip_drm_fb.h"
> >>> -#include "rockchip_drm_fbdev.h"
> >>>#include "rockchip_drm_gem.h"
> >>>#define DRIVER_NAME"rockchip"
> >>> @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev)
> >>> drm_mode_config_reset(drm_dev);
> >>> - ret = rockchip_drm_fbdev_init(drm_dev);
> >>> - if (ret)
> >>> - goto err_unbind_all;
> >>> -
> >>> /* init kms poll for handling hpd */
> >>> drm_kms_helper_poll_init(drm_dev);
> >>> @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev)
> >>> if (ret)
> >>> goto err_kms_helper_poll_fini;
> >>> + drm_fbdev_generic_setup(drm_dev, 32);
> >>
> >> Please pass 0 for the final argument. The fbdev helpers pick 32 by default.
> >> Maybe leave a comment if you require 32 here.
> >
> > I wanted to minimise the changes introduced here - passing 32 matches
> > the value passed to drm_fb_helper_initial_config() in the deleted code
> > from rockchip_drm_fbdev.c.
>
> In that case
>
> Acked-by: Thomas Zimmermann
Thanks!
> >
> > What do you think about changing this to 0 in a follow-up patch?
> >
>
> Yes. If possible, please provide a follow-up patch for this and set
> modeconfig.prefered_depth to 24.
I'll follow up with a patch setting this to zero, but I'm not convinced
I understand mode_config.prefered_depth well enough to be confident
setting it to 24 is the right thing to do.
Looking at commits 23d4e55f7eeb ("drm/vkms: Unset preferred_depth") and
550f17441f53 ("drm/cirrus: flip default from 24bpp to 16bpp") it seems
that this will have a wider impact beyond fbdev. 32bpp has been used
since the Rockchip driver was added so I don't see any real need to
change to 24 now.
Regards,
John
[PATCH] drm/rockchip: pass 0 to drm_fbdev_generic_setup()
Allow drm_fbdev_generic_setup() to pick the default bpp value for the framebuffer. This has no functional impact because the default is 32, given that mode_config.preferred_depth is not set for Rockchip. Suggested-by: Thomas Zimmermann Signed-off-by: John Keeping --- This needs [1] to be applied first. [1] https://lore.kernel.org/all/20211029115014.264084-1-j...@metanate.com drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 1481181445fd..e37e74998fa9 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -165,7 +165,7 @@ static int rockchip_drm_bind(struct device *dev) if (ret) goto err_kms_helper_poll_fini; - drm_fbdev_generic_setup(drm_dev, 32); + drm_fbdev_generic_setup(drm_dev, 0); return 0; err_kms_helper_poll_fini: -- 2.33.1
Re: [Intel-gfx] [PATCH v4 1/4] drm/i915: Introduce refcounted sg-tables
On Fri, 29 Oct 2021 at 09:22, Thomas Hellström
wrote:
>
> As we start to introduce asynchronous failsafe object migration,
> where we update the object state and then submit asynchronous
> commands we need to record what memory resources are actually used
> by various part of the command stream. Initially for three purposes:
>
> 1) Error capture.
> 2) Asynchronous migration error recovery.
> 3) Asynchronous vma bind.
>
> At the time where these happens, the object state may have been updated
> to be several migrations ahead and object sg-tables discarded.
>
> In order to make it possible to keep sg-tables with memory resource
> information for these operations, introduce refcounted sg-tables that
> aren't freed until the last user is done with them.
>
> The alternative would be to reference information sitting on the
> corresponding ttm_resources which typically have the same lifetime as
> these refcountes sg_tables, but that leads to other awkward constructs:
> Due to the design direction chosen for ttm resource managers that would
> lead to diamond-style inheritance, the LMEM resources may sometimes be
> prematurely freed, and finally the subclassed struct ttm_resource would
> have to bleed into the asynchronous vma bind code.
>
> v3:
> - Address a number of style issues (Matthew Auld)
> v4:
> - Dont check for st->sgl being NULL in i915_ttm_tt__shmem_unpopulate(),
> that should never happen. (Matthew Auld)
>
> Signed-off-by: Thomas Hellström
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +-
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 +-
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 49 +++--
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 186 ++
> drivers/gpu/drm/i915/i915_scatterlist.c | 62 --
> drivers/gpu/drm/i915/i915_scatterlist.h | 76 ++-
> drivers/gpu/drm/i915/intel_region_ttm.c | 15 +-
> drivers/gpu/drm/i915/intel_region_ttm.h | 5 +-
> drivers/gpu/drm/i915/selftests/mock_region.c | 12 +-
> 9 files changed, 276 insertions(+), 144 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index a5479ac7a4ad..ba224598ed69 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -620,12 +620,12 @@ int i915_gem_object_wait_migration(struct
> drm_i915_gem_object *obj,
> bool i915_gem_object_placement_possible(struct drm_i915_gem_object *obj,
> enum intel_memory_type type);
>
> -struct sg_table *shmem_alloc_st(struct drm_i915_private *i915,
> - size_t size, struct intel_memory_region *mr,
> - struct address_space *mapping,
> - unsigned int max_segment);
> -void shmem_free_st(struct sg_table *st, struct address_space *mapping,
> - bool dirty, bool backup);
> +int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
> +size_t size, struct intel_memory_region *mr,
> +struct address_space *mapping,
> +unsigned int max_segment);
> +void shmem_sg_free_table(struct sg_table *st, struct address_space *mapping,
> +bool dirty, bool backup);
> void __shmem_writeback(size_t size, struct address_space *mapping);
>
> #ifdef CONFIG_MMU_NOTIFIER
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> index a4b69a43b898..604ed5ad77f5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -544,6 +544,7 @@ struct drm_i915_gem_object {
> */
> struct list_head region_link;
>
> + struct i915_refct_sgt *rsgt;
> struct sg_table *pages;
> void *mapping;
>
> @@ -597,7 +598,7 @@ struct drm_i915_gem_object {
> } mm;
>
> struct {
> - struct sg_table *cached_io_st;
> + struct i915_refct_sgt *cached_io_rsgt;
> struct i915_gem_object_page_iter get_io_page;
> struct drm_i915_gem_object *backup;
> bool created:1;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 01f332d8dbde..e09141031a5e 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -25,8 +25,8 @@ static void check_release_pagevec(struct pagevec *pvec)
> cond_resched();
> }
>
> -void shmem_free_st(struct sg_table *st, struct address_space *mapping,
> - bool dirty, bool backup)
> +void shmem_sg_free_table(struct sg_table *st, struct address_space *mapping,
> +bool dirty, bool backup)
> {
> struct sgt_iter sgt_iter;
> s
[PATCH v5] drm/i915: Introduce refcounted sg-tables
As we start to introduce asynchronous failsafe object migration,
where we update the object state and then submit asynchronous
commands we need to record what memory resources are actually used
by various part of the command stream. Initially for three purposes:
1) Error capture.
2) Asynchronous migration error recovery.
3) Asynchronous vma bind.
At the time where these happens, the object state may have been updated
to be several migrations ahead and object sg-tables discarded.
In order to make it possible to keep sg-tables with memory resource
information for these operations, introduce refcounted sg-tables that
aren't freed until the last user is done with them.
The alternative would be to reference information sitting on the
corresponding ttm_resources which typically have the same lifetime as
these refcountes sg_tables, but that leads to other awkward constructs:
Due to the design direction chosen for ttm resource managers that would
lead to diamond-style inheritance, the LMEM resources may sometimes be
prematurely freed, and finally the subclassed struct ttm_resource would
have to bleed into the asynchronous vma bind code.
v3:
- Address a number of style issues (Matthew Auld)
v4:
- Dont check for st->sgl being NULL in i915_ttm_tt__shmem_unpopulate(),
that should never happen. (Matthew Auld)
v5:
- Fix a Potential double-free (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 3 +-
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 53 +++--
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 186 ++
drivers/gpu/drm/i915/i915_scatterlist.c | 62 --
drivers/gpu/drm/i915/i915_scatterlist.h | 76 ++-
drivers/gpu/drm/i915/intel_region_ttm.c | 15 +-
drivers/gpu/drm/i915/intel_region_ttm.h | 5 +-
drivers/gpu/drm/i915/selftests/mock_region.c | 12 +-
9 files changed, 277 insertions(+), 147 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index a5479ac7a4ad..ba224598ed69 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -620,12 +620,12 @@ int i915_gem_object_wait_migration(struct
drm_i915_gem_object *obj,
bool i915_gem_object_placement_possible(struct drm_i915_gem_object *obj,
enum intel_memory_type type);
-struct sg_table *shmem_alloc_st(struct drm_i915_private *i915,
- size_t size, struct intel_memory_region *mr,
- struct address_space *mapping,
- unsigned int max_segment);
-void shmem_free_st(struct sg_table *st, struct address_space *mapping,
- bool dirty, bool backup);
+int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
+size_t size, struct intel_memory_region *mr,
+struct address_space *mapping,
+unsigned int max_segment);
+void shmem_sg_free_table(struct sg_table *st, struct address_space *mapping,
+bool dirty, bool backup);
void __shmem_writeback(size_t size, struct address_space *mapping);
#ifdef CONFIG_MMU_NOTIFIER
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index a4b69a43b898..604ed5ad77f5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -544,6 +544,7 @@ struct drm_i915_gem_object {
*/
struct list_head region_link;
+ struct i915_refct_sgt *rsgt;
struct sg_table *pages;
void *mapping;
@@ -597,7 +598,7 @@ struct drm_i915_gem_object {
} mm;
struct {
- struct sg_table *cached_io_st;
+ struct i915_refct_sgt *cached_io_rsgt;
struct i915_gem_object_page_iter get_io_page;
struct drm_i915_gem_object *backup;
bool created:1;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 01f332d8dbde..4a88c89b7a14 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -25,8 +25,8 @@ static void check_release_pagevec(struct pagevec *pvec)
cond_resched();
}
-void shmem_free_st(struct sg_table *st, struct address_space *mapping,
- bool dirty, bool backup)
+void shmem_sg_free_table(struct sg_table *st, struct address_space *mapping,
+bool dirty, bool backup)
{
struct sgt_iter sgt_iter;
struct pagevec pvec;
@@ -49,17 +49,15 @@ void shmem_free_st(struct sg_table *st, struct
address_space *mapping,
check_release_pagevec(&pvec);
sg_fre
Re: [PATCH] drm: Grab reference of connector before return connector from sun4i_tcon_get_connector
On Mon, 01 Nov 2021, He Ying wrote:
> From the comments of drm_for_each_connector_iter(), we know
> that "connector is only valid within the list body, if you
> want to use connector after calling drm_connector_list_iter_end()
> then you need to grab your own reference first using
> drm_connector_get()". So fix the wrong use of connector
> according to the comments and then call drm_connector_put()
> after using connector finishes.
>
> Signed-off-by: He Ying
Please use "drm/sun4i:" subject prefix for sun4i patches.
BR,
Jani.
> ---
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 9f06dec0fc61..24fa6784ee5f 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -47,12 +47,12 @@ static struct drm_connector
> *sun4i_tcon_get_connector(const struct drm_encoder *
> drm_connector_list_iter_begin(encoder->dev, &iter);
> drm_for_each_connector_iter(connector, &iter)
> if (connector->encoder == encoder) {
> - drm_connector_list_iter_end(&iter);
> - return connector;
> + drm_connector_get(connector);
> + break;
> }
> drm_connector_list_iter_end(&iter);
>
> - return NULL;
> + return connector;
> }
>
> static int sun4i_tcon_get_pixel_depth(const struct drm_encoder *encoder)
> @@ -65,6 +65,7 @@ static int sun4i_tcon_get_pixel_depth(const struct
> drm_encoder *encoder)
> return -EINVAL;
>
> info = &connector->display_info;
> + drm_connector_put(connector);
> if (info->num_bus_formats != 1)
> return -EINVAL;
>
> @@ -361,6 +362,7 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon
> *tcon,
> /* TODO support normal CPU interface modes */
> struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
> struct mipi_dsi_device *device = dsi->device;
> + struct drm_connector *connector;
> u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> u8 lanes = device->lanes;
> u32 block_space, start_delay;
> @@ -372,7 +374,9 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon
> *tcon,
> sun4i_tcon0_mode_set_common(tcon, mode);
>
> /* Set dithering if needed */
> - sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> + connector = sun4i_tcon_get_connector(encoder);
> + sun4i_tcon0_mode_set_dithering(tcon, connector);
> + drm_connector_put(connector);
>
> regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
> SUN4I_TCON0_CTL_IF_MASK,
> @@ -430,6 +434,7 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon
> *tcon,
> const struct drm_display_mode *mode)
> {
> unsigned int bp;
> + struct drm_connector *connector;
> u8 clk_delay;
> u32 reg, val = 0;
>
> @@ -440,7 +445,9 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon
> *tcon,
> sun4i_tcon0_mode_set_common(tcon, mode);
>
> /* Set dithering if needed */
> - sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> + connector = sun4i_tcon_get_connector(encoder);
> + sun4i_tcon0_mode_set_dithering(tcon, connector);
> + drm_connector_put(connector);
>
> /* Adjust clock delay */
> clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
> @@ -518,6 +525,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon
> *tcon,
>
> /* Set dithering if needed */
> sun4i_tcon0_mode_set_dithering(tcon, connector);
> + drm_connector_put(connector);
>
> /* Adjust clock delay */
> clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v2] drm/dp: Fix aux->transfer NULL pointer dereference on drm_dp_dpcd_access
On Mon, 01 Nov 2021, Perry Yuan wrote: > Fix below crash by adding a check in the drm_dp_dpcd_access which > ensures that aux->transfer was actually initialized earlier. Gut feeling says this is papering over a real usage issue somewhere else. Why is the aux being used for transfers before ->transfer has been set? Why should the dp helper be defensive against all kinds of misprogramming? BR, Jani. > > BUG: kernel NULL pointer dereference, address: > PGD 0 P4D 0 > Oops: 0010 [#1] SMP NOPTI > RIP: 0010:0x0 > Code: Unable to access opcode bytes at RIP 0xffd6. > RSP: 0018:a8d64225bab8 EFLAGS: 00010246 > RAX: RBX: 0020 RCX: a8d64225bb5e > RDX: 93151d921880 RSI: a8d64225bac8 RDI: 931511a1a9d8 > RBP: a8d64225bb10 R08: 0001 R09: a8d64225ba60 > R10: 0002 R11: 000d R12: 0001 > R13: R14: a8d64225bb5e R15: 931511a1a9d8 > FS: 7ff8ea7fa9c0() GS:9317fe6c() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: ffd6 CR3: 00010d5a4000 CR4: 00750ee0 > PKRU: 5554 > Call Trace: > drm_dp_dpcd_access+0x72/0x110 [drm_kms_helper] > drm_dp_dpcd_read+0xb7/0xf0 [drm_kms_helper] > drm_dp_start_crc+0x38/0xb0 [drm_kms_helper] > amdgpu_dm_crtc_set_crc_source+0x1ae/0x3e0 [amdgpu] > crtc_crc_open+0x174/0x220 [drm] > full_proxy_open+0x168/0x1f0 > ? open_proxy_open+0x100/0x100 > do_dentry_open+0x156/0x370 > vfs_open+0x2d/0x30 > > v2: fix some typo > > Signed-off-by: Perry Yuan > --- > drivers/gpu/drm/drm_dp_helper.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 6d0f2c447f3b..76b28396001a 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -260,6 +260,10 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 > request, > msg.buffer = buffer; > msg.size = size; > > + /* No transfer function is set, so not an available DP connector */ > + if (!aux->transfer) > + return -EINVAL; > + > mutex_lock(&aux->hw_mutex); > > /* -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v5] drm/i915: Introduce refcounted sg-tables
On 01/11/2021 12:24, Thomas Hellström wrote:
As we start to introduce asynchronous failsafe object migration,
where we update the object state and then submit asynchronous
commands we need to record what memory resources are actually used
by various part of the command stream. Initially for three purposes:
1) Error capture.
2) Asynchronous migration error recovery.
3) Asynchronous vma bind.
FWIW something like this may be interesting to me as well, although I
haven't looked much into details yet, for the purpose of allowing
delayed "put pages" via decoupling from the GEM bo.
Two questions after glancing over:
1)
I do wonder if abstracting "sgt" away from the name would make sense?
Like perhaps obj->mm.pages being the location of the new abstraction so
naming it along the lines of i915_obj_pages or something.
2)
And how come obj->mm.pages remains? Does it go away later in follow up work?
Regards,
Tvrtko
At the time where these happens, the object state may have been updated
to be several migrations ahead and object sg-tables discarded.
In order to make it possible to keep sg-tables with memory resource
information for these operations, introduce refcounted sg-tables that
aren't freed until the last user is done with them.
The alternative would be to reference information sitting on the
corresponding ttm_resources which typically have the same lifetime as
these refcountes sg_tables, but that leads to other awkward constructs:
Due to the design direction chosen for ttm resource managers that would
lead to diamond-style inheritance, the LMEM resources may sometimes be
prematurely freed, and finally the subclassed struct ttm_resource would
have to bleed into the asynchronous vma bind code.
v3:
- Address a number of style issues (Matthew Auld)
v4:
- Dont check for st->sgl being NULL in i915_ttm_tt__shmem_unpopulate(),
that should never happen. (Matthew Auld)
v5:
- Fix a Potential double-free (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 3 +-
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 53 +++--
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 186 ++
drivers/gpu/drm/i915/i915_scatterlist.c | 62 --
drivers/gpu/drm/i915/i915_scatterlist.h | 76 ++-
drivers/gpu/drm/i915/intel_region_ttm.c | 15 +-
drivers/gpu/drm/i915/intel_region_ttm.h | 5 +-
drivers/gpu/drm/i915/selftests/mock_region.c | 12 +-
9 files changed, 277 insertions(+), 147 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index a5479ac7a4ad..ba224598ed69 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -620,12 +620,12 @@ int i915_gem_object_wait_migration(struct
drm_i915_gem_object *obj,
bool i915_gem_object_placement_possible(struct drm_i915_gem_object *obj,
enum intel_memory_type type);
-struct sg_table *shmem_alloc_st(struct drm_i915_private *i915,
- size_t size, struct intel_memory_region *mr,
- struct address_space *mapping,
- unsigned int max_segment);
-void shmem_free_st(struct sg_table *st, struct address_space *mapping,
- bool dirty, bool backup);
+int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
+size_t size, struct intel_memory_region *mr,
+struct address_space *mapping,
+unsigned int max_segment);
+void shmem_sg_free_table(struct sg_table *st, struct address_space *mapping,
+bool dirty, bool backup);
void __shmem_writeback(size_t size, struct address_space *mapping);
#ifdef CONFIG_MMU_NOTIFIER
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index a4b69a43b898..604ed5ad77f5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -544,6 +544,7 @@ struct drm_i915_gem_object {
*/
struct list_head region_link;
+ struct i915_refct_sgt *rsgt;
struct sg_table *pages;
void *mapping;
@@ -597,7 +598,7 @@ struct drm_i915_gem_object {
} mm;
struct {
- struct sg_table *cached_io_st;
+ struct i915_refct_sgt *cached_io_rsgt;
struct i915_gem_object_page_iter get_io_page;
struct drm_i915_gem_object *backup;
bool created:1;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 01f332d8dbde..4a88c89b7a14 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i91
[PATCH v4] backlight: lp855x: Switch to atomic PWM API
Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and
replace it for the atomic PWM API.
Signed-off-by: Maíra Canal
---
V1 -> V2: Initialize variable and simply conditional loop
V2 -> V3: Fix assignment of NULL variable
V3 -> V4: Replace division for pwm_set_relative_duty_cycle
---
drivers/video/backlight/lp855x_bl.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c
b/drivers/video/backlight/lp855x_bl.c
index e94932c69f54..bbf24564082a 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)
static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
{
- unsigned int period = lp->pdata->period_ns;
- unsigned int duty = br * period / max_br;
struct pwm_device *pwm;
+ struct pwm_state state;
/* request pwm device with the consumer name */
if (!lp->pwm) {
@@ -245,18 +244,15 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br,
int max_br)
lp->pwm = pwm;
- /*
-* FIXME: pwm_apply_args() should be removed when switching to
-* the atomic PWM API.
-*/
- pwm_apply_args(pwm);
+ pwm_init_state(lp->pwm, &state);
+ state.period = lp->pdata->period_ns;
}
- pwm_config(lp->pwm, duty, period);
- if (duty)
- pwm_enable(lp->pwm);
- else
- pwm_disable(lp->pwm);
+ pwm_get_state(lp->pwm, &state);
+ pwm_set_relative_duty_cycle(&state, br, max_br);
+ state.enabled = state.duty_cycle;
+
+ pwm_apply_state(lp->pwm, &state);
}
static int lp855x_bl_update_status(struct backlight_device *bl)
--
2.31.1
Re: [PATCH 4/9] drm/format-helper: Rework format-helper conversion functions
Hi
Am 24.10.21 um 13:32 schrieb Noralf Trønnes:
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Move destination-buffer clipping from all format-helper conversion
functions into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.
Simply harmonize the interface and semantics of the existing code.
Not all conversion helpers support all combinations of parameters.
We have to add additional features when we need them.
Signed-off-by: Thomas Zimmermann
---
/**
* drm_fb_xrgb_to_gray8 - Convert XRGB to grayscale
* @dst: 8-bit grayscale destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
* @vaddr: XRGB source buffer
* @fb: DRM framebuffer
* @clip: Clip rectangle area to copy
@@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb_to_rgb888_dstclip);
*
* ITU BT.601 is used for the RGB -> luma (brightness) conversion.
*/
-void drm_fb_xrgb_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
- struct drm_rect *clip)
+void drm_fb_xrgb_to_gray8(void *dst, unsigned int dst_pitch, const void
*vaddr,
+ const struct drm_framebuffer *fb, const struct
drm_rect *clip)
{
unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
unsigned int x, y;
void *buf;
- u32 *src;
+ u8 *dst8;
+ u32 *src32;
if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB))
return;
+
+ if (!dst_pitch)
len is source length (should really have been named src_len) which
results in a kernel crash:
+ dst_pitch = len;
This works:
dst_pitch = drm_rect_width(clip);
Fixed in the next revision. Thank you so much!
Best regards
Thomas
With that fixed:
Tested-by: Noralf Trønnes
Reviewed-by: Noralf Trønnes
+
/*
* The cma memory is write-combined so reads are uncached.
* Speed up by fetching one line at a time.
@@ -433,20 +440,22 @@ void drm_fb_xrgb_to_gray8(u8 *dst, void *vaddr,
struct drm_framebuffer *fb,
if (!buf)
return;
+ vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
for (y = clip->y1; y < clip->y2; y++) {
- src = vaddr + (y * fb->pitches[0]);
- src += clip->x1;
- memcpy(buf, src, len);
- src = buf;
+ dst8 = dst;
+ src32 = memcpy(buf, vaddr, len);
for (x = clip->x1; x < clip->x2; x++) {
- u8 r = (*src & 0x00ff) >> 16;
- u8 g = (*src & 0xff00) >> 8;
- u8 b = *src & 0x00ff;
+ u8 r = (*src32 & 0x00ff) >> 16;
+ u8 g = (*src32 & 0xff00) >> 8;
+ u8 b = *src32 & 0x00ff;
/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
- *dst++ = (3 * r + 6 * g + b) / 10;
- src++;
+ *dst8++ = (3 * r + 6 * g + b) / 10;
+ src32++;
}
+
+ vaddr += fb->pitches[0];
+ dst += dst_pitch;
}
kfree(buf);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: linux-next: Tree for Nov 1 [drivers/gpu/drm/msm/msm.ko]
On 11/1/21 2:44 AM, Stephen Rothwell wrote: Hi all, Changes since 20211029: On i386, when # CONFIG_COMMON_CLK is not set ERROR: modpost: "msm_hdmi_phy_8996_cfg" [drivers/gpu/drm/msm/msm.ko] undefined! Full randconfig file is attached. -- ~Randy config-r1095b.gz Description: application/gzip
Re: [Intel-gfx] [PATCH v5] drm/i915: Introduce refcounted sg-tables
Hi, Tvrtko On Mon, 2021-11-01 at 13:14 +, Tvrtko Ursulin wrote: > > On 01/11/2021 12:24, Thomas Hellström wrote: > > As we start to introduce asynchronous failsafe object migration, > > where we update the object state and then submit asynchronous > > commands we need to record what memory resources are actually used > > by various part of the command stream. Initially for three > > purposes: > > > > 1) Error capture. > > 2) Asynchronous migration error recovery. > > 3) Asynchronous vma bind. > > FWIW something like this may be interesting to me as well, although I > haven't looked much into details yet, for the purpose of allowing > delayed "put pages" via decoupling from the GEM bo. > Two questions after glancing over: > > 1) > I do wonder if abstracting "sgt" away from the name would make sense? > Like perhaps obj->mm.pages being the location of the new abstraction > so > naming it along the lines of i915_obj_pages or something. Well it's not yet clear how this will end up. Really this should develop into something along the lines of "struct i915_async_obj", on which the sg-list is a member only. Depending on how this turns out and if it remains an sg-list I think your suggestion makes sense, but is it something we can postpone for now? > > 2) > And how come obj->mm.pages remains? Does it go away later in follow > up work? For the non-ttm backends, it's not yet implemented, so once they are either moved to TTM or updated, we can completely replace obj- >mm.pages. /Thomas > > Regards, > > Tvrtko > > > At the time where these happens, the object state may have been > > updated > > to be several migrations ahead and object sg-tables discarded. > > > > In order to make it possible to keep sg-tables with memory resource > > information for these operations, introduce refcounted sg-tables > > that > > aren't freed until the last user is done with them. > > > > The alternative would be to reference information sitting on the > > corresponding ttm_resources which typically have the same lifetime > > as > > these refcountes sg_tables, but that leads to other awkward > > constructs: > > Due to the design direction chosen for ttm resource managers that > > would > > lead to diamond-style inheritance, the LMEM resources may sometimes > > be > > prematurely freed, and finally the subclassed struct ttm_resource > > would > > have to bleed into the asynchronous vma bind code. > > > > v3: > > - Address a number of style issues (Matthew Auld) > > v4: > > - Dont check for st->sgl being NULL in > > i915_ttm_tt__shmem_unpopulate(), > > that should never happen. (Matthew Auld) > > v5: > > - Fix a Potential double-free (Matthew Auld) > > > > Signed-off-by: Thomas Hellström > > Reviewed-by: Matthew Auld > > --- > > drivers/gpu/drm/i915/gem/i915_gem_object.h | 12 +- > > .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 +- > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 53 +++-- > > drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 186 ++--- > > - > > drivers/gpu/drm/i915/i915_scatterlist.c | 62 -- > > drivers/gpu/drm/i915/i915_scatterlist.h | 76 ++- > > drivers/gpu/drm/i915/intel_region_ttm.c | 15 +- > > drivers/gpu/drm/i915/intel_region_ttm.h | 5 +- > > drivers/gpu/drm/i915/selftests/mock_region.c | 12 +- > > 9 files changed, 277 insertions(+), 147 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h > > b/drivers/gpu/drm/i915/gem/i915_gem_object.h > > index a5479ac7a4ad..ba224598ed69 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > > @@ -620,12 +620,12 @@ int i915_gem_object_wait_migration(struct > > drm_i915_gem_object *obj, > > bool i915_gem_object_placement_possible(struct > > drm_i915_gem_object *obj, > > enum intel_memory_type > > type); > > > > -struct sg_table *shmem_alloc_st(struct drm_i915_private *i915, > > - size_t size, struct > > intel_memory_region *mr, > > - struct address_space *mapping, > > - unsigned int max_segment); > > -void shmem_free_st(struct sg_table *st, struct address_space > > *mapping, > > - bool dirty, bool backup); > > +int shmem_sg_alloc_table(struct drm_i915_private *i915, struct > > sg_table *st, > > + size_t size, struct intel_memory_region > > *mr, > > + struct address_space *mapping, > > + unsigned int max_segment); > > +void shmem_sg_free_table(struct sg_table *st, struct address_space > > *mapping, > > + bool dirty, bool backup); > > void __shmem_writeback(size_t size, struct address_space > > *mapping); > > > > #ifdef CONFIG_MMU_NOTIFIER > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > > b/drivers/gpu/drm/i915/gem/i915_gem
[PATCH v2 0/9] drm/simpledrm: Enable damage clips and virtual screens
Enable FB_DAMAGE_CLIPS with simpledrm for improved performance and/or
less overhead. With this in place, add support for virtual screens
(i.e., framebuffers that are larger than the display resolution). This
also enables fbdev panning and page flipping.
After the discussion and bug fixing wrt to fbdev overallocation, I
decided to add full support for this to simpledrm. Patches #1 to #5
change the format-helper functions accordingly. Destination buffers
are now clipped by the caller and all functions support a similar
feature set. This has some fallout in various drivers.
Patch #6 change fbdev emulation to support overallocation with
shadow buffers, even if the hardware buffer would be too small.
Patch #7 and #8 update simpledrm to enable damage clipping and virtual
screen sizes. Both features go hand in hand, sort of. For shadow-
buffered planes, the DRM framebuffer lives in system memory. So the
maximum size of the virtual screen is somewhat arbitrary. We add two
constants for resonable maximum width and height of 4096 each.
Patch #9 adds documentation and a TODO item.
Tested with simpledrm. I also ran the recently posted fbdev panning
tests to make sure that the fbdev overallocation works correctly. [1]
v2:
* add missing docs (Sam)
* return unsigned int from drm_fb_clip_offset() (Sam, Noralf)
* fix OOB access in drm_fb_xrgb_to_gray8() (Noralf)
[1] https://lists.freedesktop.org/archives/igt-dev/2021-October/036642.html
Thomas Zimmermann (9):
drm/format-helper: Export drm_fb_clip_offset()
drm/format-helper: Rework format-helper memcpy functions
drm/format-helper: Add destination-buffer pitch to drm_fb_swab()
drm/format-helper: Rework format-helper conversion functions
drm/format-helper: Streamline blit-helper interface
drm/fb-helper: Allocate shadow buffer of surface height
drm/simpledrm: Enable FB_DAMAGE_CLIPS property
drm/simpledrm: Support virtual screen sizes
drm: Clarify semantics of struct
drm_mode_config.{min,max}_{width,height}
Documentation/gpu/todo.rst | 15 ++
drivers/gpu/drm/drm_fb_helper.c | 2 +-
drivers/gpu/drm/drm_format_helper.c | 247 ++--
drivers/gpu/drm/drm_mipi_dbi.c | 6 +-
drivers/gpu/drm/gud/gud_pipe.c | 14 +-
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 5 +-
drivers/gpu/drm/mgag200/mgag200_mode.c | 4 +-
drivers/gpu/drm/tiny/cirrus.c | 24 +-
drivers/gpu/drm/tiny/repaper.c | 2 +-
drivers/gpu/drm/tiny/simpledrm.c| 41 +++-
drivers/gpu/drm/tiny/st7586.c | 2 +-
include/drm/drm_format_helper.h | 58 ++---
include/drm/drm_gem_atomic_helper.h | 18 ++
include/drm/drm_mode_config.h | 13 ++
14 files changed, 264 insertions(+), 187 deletions(-)
--
2.33.1
[PATCH v2 3/9] drm/format-helper: Add destination-buffer pitch to drm_fb_swab()
Add destination-buffer pitch as argument to drm_fb_swab(). Done for
consistency with the rest of the interface.
v2:
* update documentation (Noralf)
Signed-off-by: Thomas Zimmermann
Tested-by: Noralf Trønnes
Reviewed-by: Noralf Trønnes
---
drivers/gpu/drm/drm_format_helper.c | 21 -
drivers/gpu/drm/drm_mipi_dbi.c | 2 +-
drivers/gpu/drm/gud/gud_pipe.c | 2 +-
include/drm/drm_format_helper.h | 5 +++--
4 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c
b/drivers/gpu/drm/drm_format_helper.c
index fac37c697d8b..dac355320287 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -101,6 +101,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
/**
* drm_fb_swab - Swap bytes into clip buffer
* @dst: Destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
* @src: Source buffer
* @fb: DRM framebuffer
* @clip: Clip rectangle area to copy
@@ -110,21 +111,27 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
* time to speed up slow uncached reads.
*
* This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
+ * is at the top-left corner.
*/
-void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
-struct drm_rect *clip, bool cached)
+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
+const struct drm_framebuffer *fb, const struct drm_rect *clip,
+bool cached)
{
u8 cpp = fb->format->cpp[0];
size_t len = drm_rect_width(clip) * cpp;
- u16 *src16, *dst16 = dst;
- u32 *src32, *dst32 = dst;
+ const u16 *src16;
+ const u32 *src32;
+ u16 *dst16;
+ u32 *dst32;
unsigned int x, y;
void *buf = NULL;
if (WARN_ON_ONCE(cpp != 2 && cpp != 4))
return;
+ if (!dst_pitch)
+ dst_pitch = len;
+
if (!cached)
buf = kmalloc(len, GFP_KERNEL);
@@ -140,6 +147,9 @@ void drm_fb_swab(void *dst, void *src, struct
drm_framebuffer *fb,
src32 = src;
}
+ dst16 = dst;
+ dst32 = dst;
+
for (x = clip->x1; x < clip->x2; x++) {
if (cpp == 4)
*dst32++ = swab32(*src32++);
@@ -148,6 +158,7 @@ void drm_fb_swab(void *dst, void *src, struct
drm_framebuffer *fb,
}
src += fb->pitches[0];
+ dst += dst_pitch;
}
kfree(buf);
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index c09df8b06c7a..7ce89917d761 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -211,7 +211,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
switch (fb->format->format) {
case DRM_FORMAT_RGB565:
if (swap)
- drm_fb_swab(dst, src, fb, clip, !gem->import_attach);
+ drm_fb_swab(dst, 0, src, fb, clip, !gem->import_attach);
else
drm_fb_memcpy(dst, 0, src, fb, clip);
break;
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index a92112ffd37a..e0b117b2559f 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -201,7 +201,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct
drm_framebuffer *fb,
len = gud_xrgb_to_color(buf, format, vaddr, fb,
rect);
}
} else if (gud_is_big_endian() && format->cpp[0] > 1) {
- drm_fb_swab(buf, vaddr, fb, rect, !import_attach);
+ drm_fb_swab(buf, 0, vaddr, fb, rect, !import_attach);
} else if (compression && !import_attach && pitch == fb->pitches[0]) {
/* can compress directly from the framebuffer */
buf = vaddr + rect->y1 * pitch;
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 1fc3ba7b6060..ddcba5abe306 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -17,8 +17,9 @@ void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const
void *vaddr,
const struct drm_framebuffer *fb, const struct drm_rect
*clip);
void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void
*vaddr,
const struct drm_framebuffer *fb, const struct drm_rect
*clip);
-void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
-struct drm_rect *clip, bool cached);
+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
+const struct drm_framebuffer *fb, const struct drm_rect *clip,
+bool cached);
void drm_fb_xrgb_to_rgb332(void *dst, void *vaddr, struct drm
[PATCH v2 1/9] drm/format-helper: Export drm_fb_clip_offset()
Provide a function that computes the offset into a blit destination
buffer. This will allow to move destination-buffer clipping into the
format-helper callers.
v2:
* provide documentation (Sam)
* return 'unsigned int' (Sam, Noralf)
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_format_helper.c | 19 +--
include/drm/drm_format_helper.h | 4
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c
b/drivers/gpu/drm/drm_format_helper.c
index 69fde60e36b3..677e62e39f72 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -17,12 +17,27 @@
#include
#include
-static unsigned int clip_offset(struct drm_rect *clip,
- unsigned int pitch, unsigned int cpp)
+static unsigned int clip_offset(const struct drm_rect *clip, unsigned int
pitch, unsigned int cpp)
{
return clip->y1 * pitch + clip->x1 * cpp;
}
+/**
+ * drm_fb_clip_offset - Returns the clipping rectangles byte-offset in a
frambuffer
+ * pitch: Frambuffer line pitch in byte
+ * format: Frambuffer format
+ * clip: Clip rectangle
+ *
+ * Returns:
+ * The byte offset of the clip rectangle's top-left corner within the
framebuffer.
+ */
+unsigned int drm_fb_clip_offset(unsigned int pitch, const struct
drm_format_info *format,
+ const struct drm_rect *clip)
+{
+ return clip_offset(clip, pitch, format->cpp[0]);
+}
+EXPORT_SYMBOL(drm_fb_clip_offset);
+
/**
* drm_fb_memcpy - Copy clip buffer
* @dst: Destination buffer
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index e86925cf07b9..f5a8b334b18d 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -6,9 +6,13 @@
#ifndef __LINUX_DRM_FORMAT_HELPER_H
#define __LINUX_DRM_FORMAT_HELPER_H
+struct drm_format_info;
struct drm_framebuffer;
struct drm_rect;
+unsigned int drm_fb_clip_offset(unsigned int pitch, const struct
drm_format_info *format,
+ const struct drm_rect *clip);
+
void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip);
void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void
*vaddr,
--
2.33.1
[PATCH v2 4/9] drm/format-helper: Rework format-helper conversion functions
Move destination-buffer clipping from all format-helper conversion
functions into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.
Simply harmonize the interface and semantics of the existing code.
Not all conversion helpers support all combinations of parameters.
We have to add additional features when we need them.
v2:
* fix default destination pitch in drm_fb_xrgb_to_gray8()
(Noralf)
Signed-off-by: Thomas Zimmermann
Tested-by: Noralf Trønnes
Reviewed-by: Noralf Trønnes
---
drivers/gpu/drm/drm_format_helper.c | 131 +++-
drivers/gpu/drm/drm_mipi_dbi.c | 2 +-
drivers/gpu/drm/gud/gud_pipe.c | 10 +--
drivers/gpu/drm/tiny/cirrus.c | 10 +--
drivers/gpu/drm/tiny/repaper.c | 2 +-
drivers/gpu/drm/tiny/st7586.c | 2 +-
include/drm/drm_format_helper.h | 30 +++
7 files changed, 97 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c
b/drivers/gpu/drm/drm_format_helper.c
index dac355320287..0c8933390fdd 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -165,7 +165,7 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const
void *src,
}
EXPORT_SYMBOL(drm_fb_swab);
-static void drm_fb_xrgb_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned
int pixels)
+static void drm_fb_xrgb_to_rgb332_line(u8 *dbuf, const __le32 *sbuf,
unsigned int pixels)
{
unsigned int x;
u32 pix;
@@ -181,23 +181,24 @@ static void drm_fb_xrgb_to_rgb332_line(u8 *dbuf,
__le32 *sbuf, unsigned int
/**
* drm_fb_xrgb_to_rgb332 - Convert XRGB to RGB332 clip buffer
* @dst: RGB332 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
* @src: XRGB source buffer
* @fb: DRM framebuffer
* @clip: Clip rectangle area to copy
*
* Drivers can use this function for RGB332 devices that don't natively
support XRGB.
- *
- * This function does not apply clipping on dst, i.e. the destination is a
small buffer
- * containing the clip rect only.
*/
-void drm_fb_xrgb_to_rgb332(void *dst, void *src, struct drm_framebuffer
*fb,
- struct drm_rect *clip)
+void drm_fb_xrgb_to_rgb332(void *dst, unsigned int dst_pitch, const void
*src,
+ const struct drm_framebuffer *fb, const struct
drm_rect *clip)
{
size_t width = drm_rect_width(clip);
size_t src_len = width * sizeof(u32);
unsigned int y;
void *sbuf;
+ if (!dst_pitch)
+ dst_pitch = width;
+
/* Use a buffer to speed up access on buffers with uncached read
mapping (i.e. WC) */
sbuf = kmalloc(src_len, GFP_KERNEL);
if (!sbuf)
@@ -208,14 +209,14 @@ void drm_fb_xrgb_to_rgb332(void *dst, void *src,
struct drm_framebuffer *fb,
memcpy(sbuf, src, src_len);
drm_fb_xrgb_to_rgb332_line(dst, sbuf, width);
src += fb->pitches[0];
- dst += width;
+ dst += dst_pitch;
}
kfree(sbuf);
}
EXPORT_SYMBOL(drm_fb_xrgb_to_rgb332);
-static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32 *sbuf,
+static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, const u32 *sbuf,
unsigned int pixels,
bool swab)
{
@@ -236,6 +237,7 @@ static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32
*sbuf,
/**
* drm_fb_xrgb_to_rgb565 - Convert XRGB to RGB565 clip buffer
* @dst: RGB565 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
* @vaddr: XRGB source buffer
* @fb: DRM framebuffer
* @clip: Clip rectangle area to copy
@@ -243,13 +245,10 @@ static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32
*sbuf,
*
* Drivers can use this function for RGB565 devices that don't natively
* support XRGB.
- *
- * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
*/
-void drm_fb_xrgb_to_rgb565(void *dst, void *vaddr,
- struct drm_framebuffer *fb,
- struct drm_rect *clip, bool swab)
+void drm_fb_xrgb_to_rgb565(void *dst, unsigned int dst_pitch, const void
*vaddr,
+ const struct drm_framebuffer *fb, const struct
drm_rect *clip,
+ bool swab)
{
size_t linepixels = clip->x2 - clip->x1;
size_t src_len = linepixels * sizeof(u32);
@@ -257,6 +256,9 @@ void drm_fb_xrgb_to_rgb565(void *dst, void *vaddr,
unsigned y, lines = clip->y2 - clip->y1;
void *sbuf;
+ if (!dst_pitch)
+ dst_pitch = dst_len;
+
/*
* The cma memory is write-combined so reads are uncached.
[PATCH v2 5/9] drm/format-helper: Streamline blit-helper interface
Move destination-buffer clipping from format-helper blit function into
caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for
consistency with the rest of the interface. Remove drm_fb_blit_dstclip(),
which isn't required.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Noralf Trønnes
---
drivers/gpu/drm/drm_format_helper.c | 51 -
drivers/gpu/drm/tiny/simpledrm.c| 14 +---
include/drm/drm_format_helper.h | 10 ++
3 files changed, 19 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c
b/drivers/gpu/drm/drm_format_helper.c
index 0c8933390fdd..2fd5098d5b55 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -472,7 +472,7 @@ void drm_fb_xrgb_to_gray8(void *dst, unsigned int
dst_pitch, const void *vad
EXPORT_SYMBOL(drm_fb_xrgb_to_gray8);
/**
- * drm_fb_blit_rect_dstclip - Copy parts of a framebuffer to display memory
+ * drm_fb_blit_toio - Copy parts of a framebuffer to display memory
* @dst: The display memory to copy to
* @dst_pitch: Number of bytes between two consecutive scanlines within dst
* @dst_format:FOURCC code of the display's color format
@@ -484,17 +484,14 @@ EXPORT_SYMBOL(drm_fb_xrgb_to_gray8);
* formats of the display and the framebuffer mismatch, the blit function
* will attempt to convert between them.
*
- * Use drm_fb_blit_dstclip() to copy the full framebuffer.
- *
* Returns:
* 0 on success, or
* -EINVAL if the color-format conversion failed, or
* a negative error code otherwise.
*/
-int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
-uint32_t dst_format, void *vmap,
-struct drm_framebuffer *fb,
-struct drm_rect *clip)
+int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t
dst_format,
+const void *vmap, const struct drm_framebuffer *fb,
+const struct drm_rect *clip)
{
uint32_t fb_format = fb->format->format;
@@ -505,20 +502,16 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned
int dst_pitch,
dst_format = DRM_FORMAT_XRGB;
if (dst_format == fb_format) {
- dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip);
return 0;
} else if (dst_format == DRM_FORMAT_RGB565) {
if (fb_format == DRM_FORMAT_XRGB) {
- dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
- drm_fb_xrgb_to_rgb565_toio(dst, dst_pitch, vmap,
fb, clip,
- false);
+ drm_fb_xrgb_to_rgb565_toio(dst, dst_pitch, vmap,
fb, clip, false);
return 0;
}
} else if (dst_format == DRM_FORMAT_RGB888) {
if (fb_format == DRM_FORMAT_XRGB) {
- dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
drm_fb_xrgb_to_rgb888_toio(dst, dst_pitch, vmap,
fb, clip);
return 0;
}
@@ -526,36 +519,4 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned
int dst_pitch,
return -EINVAL;
}
-EXPORT_SYMBOL(drm_fb_blit_rect_dstclip);
-
-/**
- * drm_fb_blit_dstclip - Copy framebuffer to display memory
- * @dst: The display memory to copy to
- * @dst_pitch: Number of bytes between two consecutive scanlines within dst
- * @dst_format:FOURCC code of the display's color format
- * @vmap: The framebuffer memory to copy from
- * @fb:The framebuffer to copy from
- *
- * This function copies a full framebuffer to display memory. If the formats
- * of the display and the framebuffer mismatch, the copy function will
- * attempt to convert between them.
- *
- * See drm_fb_blit_rect_dstclip() for more information.
- *
- * Returns:
- * 0 on success, or a negative error code otherwise.
- */
-int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch,
- uint32_t dst_format, void *vmap,
- struct drm_framebuffer *fb)
-{
- struct drm_rect fullscreen = {
- .x1 = 0,
- .x2 = fb->width,
- .y1 = 0,
- .y2 = fb->height,
- };
- return drm_fb_blit_rect_dstclip(dst, dst_pitch, dst_format, vmap, fb,
- &fullscreen);
-}
-EXPORT_SYMBOL(drm_fb_blit_dstclip);
+EXPORT_SYMBOL(drm_fb_blit_toio);
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 481b48bde047..571f716ff427 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -641,6 +641,8 @@ simpledrm_simple_display_pipe_enable(struct
drm_simple_display_pipe *pipe,
[PATCH v2 2/9] drm/format-helper: Rework format-helper memcpy functions
Move destination-buffer clipping from all format-helper memcpy
function into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.
Signed-off-by: Thomas Zimmermann
Tested-by: Noralf Trønnes
Reviewed-by: Noralf Trønnes
---
drivers/gpu/drm/drm_format_helper.c | 35 -
drivers/gpu/drm/drm_mipi_dbi.c | 2 +-
drivers/gpu/drm/gud/gud_pipe.c | 2 +-
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 5 ++-
drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++-
drivers/gpu/drm/tiny/cirrus.c | 14 +
include/drm/drm_format_helper.h | 9 +++---
7 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c
b/drivers/gpu/drm/drm_format_helper.c
index 677e62e39f72..fac37c697d8b 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -41,58 +41,62 @@ EXPORT_SYMBOL(drm_fb_clip_offset);
/**
* drm_fb_memcpy - Copy clip buffer
* @dst: Destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
* @vaddr: Source buffer
* @fb: DRM framebuffer
* @clip: Clip rectangle area to copy
*
* This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
+ * is at the top-left corner.
*/
-void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
- struct drm_rect *clip)
+void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr,
+ const struct drm_framebuffer *fb, const struct drm_rect
*clip)
{
unsigned int cpp = fb->format->cpp[0];
size_t len = (clip->x2 - clip->x1) * cpp;
unsigned int y, lines = clip->y2 - clip->y1;
+ if (!dst_pitch)
+ dst_pitch = len;
+
vaddr += clip_offset(clip, fb->pitches[0], cpp);
for (y = 0; y < lines; y++) {
memcpy(dst, vaddr, len);
vaddr += fb->pitches[0];
- dst += len;
+ dst += dst_pitch;
}
}
EXPORT_SYMBOL(drm_fb_memcpy);
/**
- * drm_fb_memcpy_dstclip - Copy clip buffer
+ * drm_fb_memcpy_toio - Copy clip buffer
* @dst: Destination buffer (iomem)
* @dst_pitch: Number of bytes between two consecutive scanlines within dst
* @vaddr: Source buffer
* @fb: DRM framebuffer
* @clip: Clip rectangle area to copy
*
- * This function applies clipping on dst, i.e. the destination is a
- * full (iomem) framebuffer but only the clip rect content is copied over.
+ * This function does not apply clipping on dst, i.e. the destination
+ * is at the top-left corner.
*/
-void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch,
- void *vaddr, struct drm_framebuffer *fb,
- struct drm_rect *clip)
+void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void
*vaddr,
+ const struct drm_framebuffer *fb, const struct drm_rect
*clip)
{
unsigned int cpp = fb->format->cpp[0];
- unsigned int offset = clip_offset(clip, dst_pitch, cpp);
size_t len = (clip->x2 - clip->x1) * cpp;
unsigned int y, lines = clip->y2 - clip->y1;
- vaddr += offset;
- dst += offset;
+ if (!dst_pitch)
+ dst_pitch = len;
+
+ vaddr += clip_offset(clip, fb->pitches[0], cpp);
for (y = 0; y < lines; y++) {
memcpy_toio(dst, vaddr, len);
vaddr += fb->pitches[0];
dst += dst_pitch;
}
}
-EXPORT_SYMBOL(drm_fb_memcpy_dstclip);
+EXPORT_SYMBOL(drm_fb_memcpy_toio);
/**
* drm_fb_swab - Swap bytes into clip buffer
@@ -481,7 +485,8 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned
int dst_pitch,
dst_format = DRM_FORMAT_XRGB;
if (dst_format == fb_format) {
- drm_fb_memcpy_dstclip(dst, dst_pitch, vmap, fb, clip);
+ dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
+ drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip);
return 0;
} else if (dst_format == DRM_FORMAT_RGB565) {
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 71b646c4131f..c09df8b06c7a 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -213,7 +213,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
if (swap)
drm_fb_swab(dst, src, fb, clip, !gem->import_attach);
else
- drm_fb_memcpy(dst, src, fb, clip);
+ drm_fb_memcpy(dst, 0, src, fb, clip);
break;
case DRM_FORMAT_XRGB:
drm_fb_xrgb_to_rgb565(dst, src, fb, clip, swap);
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index daf7
[PATCH v2 6/9] drm/fb-helper: Allocate shadow buffer of surface height
Allocating a shadow buffer of the height of the buffer object does not support fbdev overallocation. Use surface height instead. Signed-off-by: Thomas Zimmermann Reviewed-by: Noralf Trønnes --- drivers/gpu/drm/drm_fb_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 8e7a124d6c5a..9727a59d35fd 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2338,7 +2338,7 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, return PTR_ERR(fbi); fbi->fbops = &drm_fbdev_fb_ops; - fbi->screen_size = fb->height * fb->pitches[0]; + fbi->screen_size = sizes->surface_height * fb->pitches[0]; fbi->fix.smem_len = fbi->screen_size; drm_fb_helper_fill_info(fbi, fb_helper, sizes); -- 2.33.1
[PATCH v2 8/9] drm/simpledrm: Support virtual screen sizes
Add constants for the maximum size of the shadow-plane surface size. Useful for shadow planes with virtual screen sizes. The current sizes are 4096 scanlines with 4096 pixels each. This seems reasonable for current hardware, but can be increased as necessary. In simpledrm, set the maximum framebuffer size from the constants for shadow planes. Implements support for virtual screen sizes and page flipping on the fbdev console. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tiny/simpledrm.c| 9 +++-- include/drm/drm_gem_atomic_helper.h | 18 ++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index e872121e9fb0..e42ae1c6ebcd 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -776,6 +777,7 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) struct drm_display_mode *mode = &sdev->mode; struct drm_connector *connector = &sdev->connector; struct drm_simple_display_pipe *pipe = &sdev->pipe; + unsigned long max_width, max_height; const uint32_t *formats; size_t nformats; int ret; @@ -784,10 +786,13 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) if (ret) return ret; + max_width = max_t(unsigned long, mode->hdisplay, DRM_SHADOW_PLANE_MAX_WIDTH); + max_height = max_t(unsigned long, mode->vdisplay, DRM_SHADOW_PLANE_MAX_HEIGHT); + dev->mode_config.min_width = mode->hdisplay; - dev->mode_config.max_width = mode->hdisplay; + dev->mode_config.max_width = max_width; dev->mode_config.min_height = mode->vdisplay; - dev->mode_config.max_height = mode->vdisplay; + dev->mode_config.max_height = max_height; dev->mode_config.prefer_shadow_fbdev = true; dev->mode_config.preferred_depth = sdev->format->cpp[0] * 8; dev->mode_config.funcs = &simpledrm_mode_config_funcs; diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h index 48222a107873..54983ecf641a 100644 --- a/include/drm/drm_gem_atomic_helper.h +++ b/include/drm/drm_gem_atomic_helper.h @@ -22,6 +22,24 @@ int drm_gem_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe, * Helpers for planes with shadow buffers */ +/** + * DRM_SHADOW_PLANE_MAX_WIDTH - Maximum width of a plane's shadow buffer in pixels + * + * For drivers with shadow planes, the maximum width of the framebuffer is + * usually independent from hardware limitations. Drivers can initialize struct + * drm_mode_config.max_width from DRM_SHADOW_PLANE_MAX_WIDTH. + */ +#define DRM_SHADOW_PLANE_MAX_WIDTH (1ul << 12) + +/** + * DRM_SHADOW_PLANE_MAX_HEIGHT - Maximum height of a plane's shadow buffer in scanlines + * + * For drivers with shadow planes, the maximum height of the framebuffer is + * usually independent from hardware limitations. Drivers can initialize struct + * drm_mode_config.max_height from DRM_SHADOW_PLANE_MAX_HEIGHT. + */ +#define DRM_SHADOW_PLANE_MAX_HEIGHT(1ul << 12) + /** * struct drm_shadow_plane_state - plane state for planes with shadow buffers * -- 2.33.1
[PATCH v2 9/9] drm: Clarify semantics of struct drm_mode_config.{min, max}_{width, height}
Add additional information on the semantics of the size fields in
struct drm_mode_config. Also add a TODO to review all driver for
correct usage of these fields.
Signed-off-by: Thomas Zimmermann
---
Documentation/gpu/todo.rst| 15 +++
include/drm/drm_mode_config.h | 13 +
2 files changed, 28 insertions(+)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 60d1d7ee0719..f4e1d72149f7 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -463,6 +463,21 @@ Contact: Thomas Zimmermann ,
Christian König, Daniel Vette
Level: Intermediate
+Review all drivers for setting struct drm_mode_config.{max_width,max_height}
correctly
+--
+
+The values in struct drm_mode_config.{max_width,max_height} describe the
+maximum supported framebuffer size. It's the virtual screen size, but many
+drivers treat it like limitations of the physical resolution.
+
+The maximum width depends on the hardware's maximum scanline pitch. The
+maximum height depends on the amount of addressable video memory. Review all
+drivers to initialize the fields to the correct values.
+
+Contact: Thomas Zimmermann
+
+Level: Intermediate
+
Core refactorings
=
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..91ca575a78de 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -359,6 +359,19 @@ struct drm_mode_config_funcs {
* Core mode resource tracking structure. All CRTC, encoders, and connectors
* enumerated by the driver are added here, as are global properties. Some
* global restrictions are also here, e.g. dimension restrictions.
+ *
+ * Framebuffer sizes refer to the virtual screen that can be displayed by
+ * the CRTC. This can be different from the physical resolution programmed.
+ * The minimum width and height, stored in @min_width and @min_height,
+ * describe the smallest size of the framebuffer. It correlates to the
+ * minimum programmable resolution.
+ * The maximum width, stored in @max_width, is typically limited by the
+ * maximum pitch between two adjacent scanlines. The maximum height, stored
+ * in @max_height, is usually only limited by the amount of addressable video
+ * memory. For hardware that has no real maximum, drivers should pick a
+ * reasonable default.
+ *
+ * See also @DRM_SHADOW_PLANE_MAX_WIDTH and @DRM_SHADOW_PLANE_MAX_HEIGHT.
*/
struct drm_mode_config {
/**
--
2.33.1
[PATCH v2 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property
Enable the FB_DAMAGE_CLIPS property to reduce display-update overhead. Also fixes a warning in the kernel log. simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called Fix the computation of the blit rectangle. This wasn't an issue so far, as simpledrm always blitted the full framebuffer. The code now supports damage clipping and virtual screen sizes. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tiny/simpledrm.c | 30 ++ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 571f716ff427..e872121e9fb0 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base; - struct drm_rect clip; + struct drm_rect src_clip, dst_clip; int idx; if (!fb) @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, if (!drm_dev_enter(dev, &idx)) return; - drm_rect_init(&clip, 0, 0, fb->width, fb->height); + drm_rect_fp_to_int(&src_clip, &plane_state->src); - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); + dst_clip = plane_state->dst; + if (!drm_rect_intersect(&dst_clip, &src_clip)) + return; + + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip); drm_dev_exit(idx); } @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = plane_state->fb; struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base; - struct drm_rect clip; + struct drm_rect damage_clip, src_clip, dst_clip; int idx; if (!fb) return; - if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip)) + if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip)) + return; + + drm_rect_fp_to_int(&src_clip, &plane_state->src); + if (!drm_rect_intersect(&src_clip, &damage_clip)) + return; + + dst_clip = plane_state->dst; + if (!drm_rect_intersect(&dst_clip, &src_clip)) return; if (!drm_dev_enter(dev, &idx)) return; - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip); drm_dev_exit(idx); } @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) if (ret) return ret; + drm_plane_enable_fb_damage_clips(&pipe->plane); + drm_mode_config_reset(dev); return 0; -- 2.33.1
Re: [Intel-gfx] [PATCH v5] drm/i915: Introduce refcounted sg-tables
On 01/11/2021 13:51, Thomas Hellström wrote: Hi, Tvrtko On Mon, 2021-11-01 at 13:14 +, Tvrtko Ursulin wrote: On 01/11/2021 12:24, Thomas Hellström wrote: As we start to introduce asynchronous failsafe object migration, where we update the object state and then submit asynchronous commands we need to record what memory resources are actually used by various part of the command stream. Initially for three purposes: 1) Error capture. 2) Asynchronous migration error recovery. 3) Asynchronous vma bind. FWIW something like this may be interesting to me as well, although I haven't looked much into details yet, for the purpose of allowing delayed "put pages" via decoupling from the GEM bo. Two questions after glancing over: 1) I do wonder if abstracting "sgt" away from the name would make sense? Like perhaps obj->mm.pages being the location of the new abstraction so naming it along the lines of i915_obj_pages or something. Well it's not yet clear how this will end up. Really this should develop into something along the lines of "struct i915_async_obj", on Whole gigantic object struct will be needed for async free or for something more than that? which the sg-list is a member only. Depending on how this turns out and if it remains an sg-list I think your suggestion makes sense, but is it something we can postpone for now? ... 2) And how come obj->mm.pages remains? Does it go away later in follow up work? For the non-ttm backends, it's not yet implemented, so once they are either moved to TTM or updated, we can completely replace obj- mm.pages. ... sure, it's your project. I assume there is some time pressure then. I was just asking since it looked a bit outside of the usual patterns on a glance. Oh one more question, how will it work for objects which migrate between system and local memory? Depending on current placement either obj->mm.pages or obj->mm.rsgt will be valid? Regards, Tvrtko
Re: [PATCH/RFT] fbdev driver for HP Visualize FX cards
Hi Thomas, Thomas Zimmermann writes: > Am 01.11.21 um 09:54 schrieb Sven Schnelle: >> Thomas Zimmermann writes: >> Thanks, i wasn't aware as i normally don't do any graphics related >> development. I take a look at dri and port the driver, which is >> hopefully not too hard. > > Sounds good. > > The one big difference when converting is that DRM really wants > drivers to support 32-bit XRGB colors. It's not a DRM limitation per > se, but a requirement of today's userspace programs. AFAICS your fbdev > driver uses a 256-color palette format. So the DRM driver would have > to convert > XRGB to 8-bit RGB332 and install a corresponding palette. Right now the driver only supports 8 bit pseudocolor, because i wanted to start with something easy to get the kernel fbcon running. I have no idea (yet) how to switch the card into other color formats. And neither how to do pseudo color with drm. But i'll figure it out i guess. > Don't worry, it's easy. Take a look at the cirrus driver for a simple DRM > driver. [1] Great, i also picked that driver as a template. :-) Thanks for your help and pointers, much appreciated! Sven
Re: Lockdep spalt on killing a processes
Pushed to drm-misc-next Andrey On 2021-10-29 3:07 a.m., Christian König wrote: Attached a patch. Give it a try please, I tested it on my side and tried to generate the right conditions to trigger this code path by repeatedly submitting commands while issuing GPU reset to stop the scheduler and then killing command submissions process in the middle. But for some reason looks like the job_queue was always empty already at the time of entity kill. It was trivial to trigger with the stress utility I've hacked together: amdgpu_stress -b v 1g -b g 1g -c 1 2 1g 1k Then while it is copying just cntrl+c to kill it. The patch itself is: Tested-by: Christian König Reviewed-by: Christian König Thanks, Christian.
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/uapi: Add query for hwconfig table
writes:
> From: Rodrigo Vivi
>
> GuC contains a consolidated table with a bunch of information about the
> current device.
>
> Previously, this information was spread and hardcoded to all the components
> including GuC, i915 and various UMDs. The goal here is to consolidate
> the data into GuC in a way that all interested components can grab the
> very latest and synchronized information using a simple query.
>
> As per most of the other queries, this one can be called twice.
> Once with item.length=0 to determine the exact buffer size, then
> allocate the user memory and call it again for to retrieve the
> table data. For example:
> struct drm_i915_query_item item = {
> .query_id = DRM_I915_QUERY_HWCONCFIG_TABLE;
> };
> query.items_ptr = (int64_t) &item;
> query.num_items = 1;
>
> ioctl(fd, DRM_IOCTL_I915_QUERY, query, sizeof(query));
>
> if (item.length <= 0)
> return -ENOENT;
>
> data = malloc(item.length);
> item.data_ptr = (int64_t) &data;
> ioctl(fd, DRM_IOCTL_I915_QUERY, query, sizeof(query));
>
> // Parse the data as appropriate...
>
> The returned array is a simple and flexible KLV (Key/Length/Value)
> formatted table. For example, it could be just:
> enum device_attr {
> ATTR_SOME_VALUE = 0,
> ATTR_SOME_MASK = 1,
> };
>
> static const u32 hwconfig[] = {
> ATTR_SOME_VALUE,
> 1, // Value Length in DWords
> 8, // Value
>
> ATTR_SOME_MASK,
> 3,
> 0x00, 0x, 0xFF00,
> };
Seems simple enough, so why doesn't i915 define the format of the
returned hwconfig blob in i915_drm.h?
struct drm_i915_hwconfig {
uint32_t key;
uint32_t length;
uint32_t values[];
};
It sounds like the kernel depends on the closed source guc being loaded
to return this information. Is that right? Will i915 also become
dependent on some of this data such that it won't be able to initialize
without the firmware being loaded?
> The attribute ids are defined in a hardware spec.
Which spec?
-Jordan
Re: [Intel-gfx] [PATCH v5] drm/i915: Introduce refcounted sg-tables
Hi, On 11/1/21 15:50, Tvrtko Ursulin wrote: On 01/11/2021 13:51, Thomas Hellström wrote: Hi, Tvrtko On Mon, 2021-11-01 at 13:14 +, Tvrtko Ursulin wrote: On 01/11/2021 12:24, Thomas Hellström wrote: As we start to introduce asynchronous failsafe object migration, where we update the object state and then submit asynchronous commands we need to record what memory resources are actually used by various part of the command stream. Initially for three purposes: 1) Error capture. 2) Asynchronous migration error recovery. 3) Asynchronous vma bind. FWIW something like this may be interesting to me as well, although I haven't looked much into details yet, for the purpose of allowing delayed "put pages" via decoupling from the GEM bo. Two questions after glancing over: 1) I do wonder if abstracting "sgt" away from the name would make sense? Like perhaps obj->mm.pages being the location of the new abstraction so naming it along the lines of i915_obj_pages or something. Well it's not yet clear how this will end up. Really this should develop into something along the lines of "struct i915_async_obj", on Whole gigantic object struct will be needed for async free or for something more than that? I guess it depends on how an async free is supposed to work. For the async migration, the plan is that when you migrate, for example between LMEM and sys, we first unbind async and get a fence that signals when unbinding is complete. The pages sg list will then be updated immediately to point to sys, then the old memory in the form of a struct ttm_resource will be freed when fences expire. It's on that ttm resource we ideally would want the sg-table to sit, but we avoid that ATM due to the awkward way those ttm resources were designed. But it's not a super-huge object. which the sg-list is a member only. Depending on how this turns out and if it remains an sg-list I think your suggestion makes sense, but is it something we can postpone for now? ... 2) And how come obj->mm.pages remains? Does it go away later in follow up work? For the non-ttm backends, it's not yet implemented, so once they are either moved to TTM or updated, we can completely replace obj- mm.pages. ... sure, it's your project. I assume there is some time pressure then. Yes, initially. I was just asking since it looked a bit outside of the usual patterns on a glance. Oh one more question, how will it work for objects which migrate between system and local memory? Depending on current placement either obj->mm.pages or obj->mm.rsgt will be valid? The contract currently is that obj->mm.pages is *always* valid. Sometimes it points to the sg_table embedded in obj->mm.rsgt. For anything that requires awareness of async migration, like upcoming vma resources and error capture, they also need to be aware of obj->mm.rsgt and handle refcounting accordingly. If it's NULL they can safely assume async migration is not happening. /Thomas Regards, Tvrtko
Re: [PATCH/RFT] fbdev driver for HP Visualize FX cards
Hi Thomas, Thomas Zimmermann writes: > Am 31.10.21 um 20:53 schrieb Sven Schnelle: >> Hi List(s), >> i wrote a fbdev driver for the HP Visualize FX cards used some of >> the >> PA-RISC workstations. It utilizes some of the 2D acceleration features >> present in the card. >> [..] > > Thanks for all the work you put into this. We welcome drivers even for > older hardware, but not for fbdev. DRM is all the rage now and has > been for a while. I'd like to ask you to convert the driver to DRM and > resubmit to . > > I while ago, I made conversion helpers for this. You can look at [1] > for a trivial DRM drivers that wraps existing fbdev drivers for use > with DRM. Once you have that, it turns into a refactoring job. Thanks, i wasn't aware as i normally don't do any graphics related development. I take a look at dri and port the driver, which is hopefully not too hard. Sven
Re: [PATCH v7 03/20] dt-bindings: mediatek: add ethdr definition for mt8195
Hi, Nancy:
Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道：
>
> Add vdosys1 ETHDR definition.
Reviewed-by: Chun-Kuang Hu
>
> Signed-off-by: Nancy.Lin
> ---
> .../display/mediatek/mediatek,ethdr.yaml | 147 ++
> 1 file changed, 147 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
> new file mode 100644
> index ..131eed5eeeb7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
> @@ -0,0 +1,147 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/mediatek/mediatek,ethdr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mediatek Ethdr Device Tree Bindings
> +
> +maintainers:
> + - Chun-Kuang Hu
> + - Philipp Zabel
> +
> +description: |
> + ETHDR is designed for HDR video and graphics conversion in the external
> display path.
> + It handles multiple HDR input types and performs tone mapping, color
> space/color
> + format conversion, and then combine different layers, output the required
> HDR or
> + SDR signal to the subsequent display path. This engine is composed of two
> video
> + frontends, two graphic frontends, one video backend and a mixer. ETHDR has
> two
> + DMA function blocks, DS and ADL. These two function blocks read the
> pre-programmed
> + registers from DRAM and set them to HW in the v-blanking period.
> +
> +properties:
> + compatible:
> +items:
> + - const: mediatek,mt8195-disp-ethdr
> + reg:
> +maxItems: 7
> + reg-names:
> +items:
> + - const: mixer
> + - const: vdo_fe0
> + - const: vdo_fe1
> + - const: gfx_fe0
> + - const: gfx_fe1
> + - const: vdo_be
> + - const: adl_ds
> + interrupts:
> +minItems: 1
> + iommus:
> +description: The compatible property is DMA function blocks.
> + Should point to the respective IOMMU block with master port as
> argument,
> + see Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for
> + details.
> +minItems: 1
> +maxItems: 2
> + clocks:
> +items:
> + - description: mixer clock
> + - description: video frontend 0 clock
> + - description: video frontend 1 clock
> + - description: graphic frontend 0 clock
> + - description: graphic frontend 1 clock
> + - description: video backend clock
> + - description: autodownload and menuload clock
> + - description: video frontend 0 async clock
> + - description: video frontend 1 async clock
> + - description: graphic frontend 0 async clock
> + - description: graphic frontend 1 async clock
> + - description: video backend async clock
> + - description: ethdr top clock
> + clock-names:
> +items:
> + - const: mixer
> + - const: vdo_fe0
> + - const: vdo_fe1
> + - const: gfx_fe0
> + - const: gfx_fe1
> + - const: vdo_be
> + - const: adl_ds
> + - const: vdo_fe0_async
> + - const: vdo_fe1_async
> + - const: gfx_fe0_async
> + - const: gfx_fe1_async
> + - const: vdo_be_async
> + - const: ethdr_top
> + power-domains:
> +maxItems: 1
> + resets:
> +maxItems: 5
> + mediatek,gce-client-reg:
> +$ref: /schemas/types.yaml#/definitions/phandle-array
> +description: The register of display function block to be set by gce.
> + There are 4 arguments in this property, gce node, subsys id, offset and
> + register size. The subsys id is defined in the gce header of each chips
> + include/include/dt-bindings/gce/-gce.h, mapping to the register
> of
> + display function block.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - interrupts
> + - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +
> +disp_ethdr@1c114000 {
> +compatible = "mediatek,mt8195-disp-ethdr";
> +reg = <0 0x1c114000 0 0x1000>,
> + <0 0x1c115000 0 0x1000>,
> + <0 0x1c117000 0 0x1000>,
> + <0 0x1c119000 0 0x1000>,
> + <0 0x1c11A000 0 0x1000>,
> + <0 0x1c11B000 0 0x1000>,
> + <0 0x1c11C000 0 0x1000>;
> +reg-names = "mixer", "vdo_fe0", "vdo_fe1", "gfx_fe0", "gfx_fe1",
> +"vdo_be", "adl_ds";
> +mediatek,gce-client-reg = <&gce0 SUBSYS_1c11 0x4000 0x1000>,
> + <&gce0 SUBSYS_1c11 0x5000 0x1000>,
> + <&gce0 SUBSYS_1c11 0x7000 0x1000>,
> + <&gce0 SUBSYS_1c11 0x9000 0x1000>,
> + <&gce0 SUBSYS_1c11 0xA
Re: [PATCH 2/2] drm: Remove CONFIG_DRM_KMS_CMA_HELPER option
Hi Thomas, I love your patch! Yet something to improve: [auto build test ERROR on next-20211029] [cannot apply to drm/drm-next shawnguo/for-next pinchartl-media/drm/du/next v5.15 v5.15-rc7 v5.15-rc6 v5.15] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Small-CMA-cleanups/20211101-161911 base:bdcc9f6a568275aed4cc32fd2312432d2ff1b704 config: x86_64-randconfig-a004-20211101 (attached as .config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 82ed106567063ea269c6d5669278b733e173a42f) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/c3c7ec5f9ccd90e78f0f2d3143505db4060bbf17 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Thomas-Zimmermann/drm-Small-CMA-cleanups/20211101-161911 git checkout c3c7ec5f9ccd90e78f0f2d3143505db4060bbf17 # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vmap from >> namespace DMA_BUF, but does not import it. >> ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vunmap from >> namespace DMA_BUF, but does not import it. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH] drm/msm/dsi: set default num_data_lanes
Hi, On Sat, Oct 30, 2021 at 10:08 AM Philip Chen wrote: > > If "data_lanes" property of the dsi output endpoint is missing in > the DT, num_data_lanes would be 0 by default, which could cause > dsi_host_attach() to fail if dsi->lanes is set to a non-zero value > by the bridge driver. > > According to the binding document of msm dsi controller, the > input/output endpoint of the controller is expected to have 4 lanes. > So let's set num_data_lanes to 4 by default. > > Signed-off-by: Philip Chen > --- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Douglas Anderson
Re: [PATCH] drm/msm/dsi: set default num_data_lanes
Quoting Philip Chen (2021-10-30 10:08:50) > If "data_lanes" property of the dsi output endpoint is missing in > the DT, num_data_lanes would be 0 by default, which could cause > dsi_host_attach() to fail if dsi->lanes is set to a non-zero value > by the bridge driver. > > According to the binding document of msm dsi controller, the > input/output endpoint of the controller is expected to have 4 lanes. > So let's set num_data_lanes to 4 by default. > > Signed-off-by: Philip Chen > --- Reviewed-by: Stephen Boyd
[PATCH 1/3] backlight: lp855x: Move device_config setting out of lp855x_configure()
Move the setting of the lp->cfg pointer to the chip specific
lp855x_device_config struct from lp855x_configure() to
lp855x_probe(), before calling lp855x_parse_dt().
This is a preperation patch for adding ACPI enumeration support.
Signed-off-by: Hans de Goede
---
drivers/video/backlight/lp855x_bl.c | 32 ++---
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c
b/drivers/video/backlight/lp855x_bl.c
index e94932c69f54..808ff00b2003 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -170,22 +170,6 @@ static int lp855x_configure(struct lp855x *lp)
int i, ret;
struct lp855x_platform_data *pd = lp->pdata;
- switch (lp->chip_id) {
- case LP8550:
- case LP8551:
- case LP8552:
- case LP8553:
- case LP8556:
- lp->cfg = &lp855x_dev_cfg;
- break;
- case LP8555:
- case LP8557:
- lp->cfg = &lp8557_dev_cfg;
- break;
- default:
- return -EINVAL;
- }
-
if (lp->cfg->pre_init_device) {
ret = lp->cfg->pre_init_device(lp);
if (ret) {
@@ -413,6 +397,22 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
lp->chip_id = id->driver_data;
lp->pdata = dev_get_platdata(&cl->dev);
+ switch (lp->chip_id) {
+ case LP8550:
+ case LP8551:
+ case LP8552:
+ case LP8553:
+ case LP8556:
+ lp->cfg = &lp855x_dev_cfg;
+ break;
+ case LP8555:
+ case LP8557:
+ lp->cfg = &lp8557_dev_cfg;
+ break;
+ default:
+ return -EINVAL;
+ }
+
if (!lp->pdata) {
ret = lp855x_parse_dt(lp);
if (ret < 0)
--
2.31.1
[PATCH 3/3] backlight: lp855x: Add support ACPI enumeration
The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight
controller for its LCD-panel, with a Xiaomi specific ACPI HID of
"XMCC0001", add support for this.
Note the new "if (id)" check also fixes a NULL pointer deref when a user
tries to manually bind the driver from sysfs.
When CONFIG_ACPI is disabled acpi_match_device() will always return NULL,
so the lp855x_parse_acpi() call will get optimized away.
Signed-off-by: Hans de Goede
---
drivers/video/backlight/lp855x_bl.c | 70 -
1 file changed, 60 insertions(+), 10 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c
b/drivers/video/backlight/lp855x_bl.c
index d1d27d5eb0f2..f075ec84acfb 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -5,6 +5,7 @@
* Copyright (C) 2011 Texas Instruments
*/
+#include
#include
#include
#include
@@ -330,7 +331,7 @@ static int lp855x_parse_dt(struct lp855x *lp)
{
struct device *dev = lp->dev;
struct device_node *node = dev->of_node;
- struct lp855x_platform_data *pdata;
+ struct lp855x_platform_data *pdata = lp->pdata;
int rom_length;
if (!node) {
@@ -338,10 +339,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
return -EINVAL;
}
- pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata)
- return -ENOMEM;
-
of_property_read_string(node, "bl-name", &pdata->name);
of_property_read_u8(node, "dev-ctrl", &pdata->device_control);
of_property_read_u8(node, "init-brt", &pdata->initial_brightness);
@@ -379,8 +376,31 @@ static int lp855x_parse_dt(struct lp855x *lp)
}
#endif
+static int lp855x_parse_acpi(struct lp855x *lp)
+{
+ int ret;
+
+ /*
+* On ACPI the device has already been initialized by the firmware
+* so we read back the settings from the registers.
+*/
+ ret = i2c_smbus_read_byte_data(lp->client, lp->cfg->reg_brightness);
+ if (ret < 0)
+ return ret;
+
+ lp->pdata->initial_brightness = ret;
+
+ ret = i2c_smbus_read_byte_data(lp->client, lp->cfg->reg_devicectrl);
+ if (ret < 0)
+ return ret;
+
+ lp->pdata->device_control = ret;
+ return 0;
+}
+
static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
{
+ const struct acpi_device_id *acpi_id = NULL;
struct device *dev = &cl->dev;
struct lp855x *lp;
int ret;
@@ -394,10 +414,20 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
lp->client = cl;
lp->dev = dev;
- lp->chipname = id->name;
- lp->chip_id = id->driver_data;
lp->pdata = dev_get_platdata(dev);
+ if (id) {
+ lp->chipname = id->name;
+ lp->chip_id = id->driver_data;
+ } else {
+ acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
+ if (!acpi_id)
+ return -ENODEV;
+
+ lp->chipname = acpi_id->id;
+ lp->chip_id = acpi_id->driver_data;
+ }
+
switch (lp->chip_id) {
case LP8550:
case LP8551:
@@ -415,9 +445,19 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
}
if (!lp->pdata) {
- ret = lp855x_parse_dt(lp);
- if (ret < 0)
- return ret;
+ lp->pdata = devm_kzalloc(dev, sizeof(*lp->pdata), GFP_KERNEL);
+ if (!lp->pdata)
+ return -ENOMEM;
+
+ if (id) {
+ ret = lp855x_parse_dt(lp);
+ if (ret < 0)
+ return ret;
+ } else {
+ ret = lp855x_parse_acpi(lp);
+ if (ret < 0)
+ return ret;
+ }
}
if (lp->pdata->period_ns > 0)
@@ -537,10 +577,20 @@ static const struct i2c_device_id lp855x_ids[] = {
};
MODULE_DEVICE_TABLE(i2c, lp855x_ids);
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id lp855x_acpi_match[] = {
+ /* Xiaomi specific HID used for the LP8556 on the Mi Pad 2 */
+ { "XMCC0001", LP8556 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, lp855x_acpi_match);
+#endif
+
static struct i2c_driver lp855x_driver = {
.driver = {
.name = "lp855x",
.of_match_table = of_match_ptr(lp855x_dt_ids),
+ .acpi_match_table = ACPI_PTR(lp855x_acpi_match),
},
.probe = lp855x_probe,
.remove = lp855x_remove,
--
2.31.1
[PATCH 2/3] backlight: lp855x: Add dev helper variable to lp855x_probe()
Add a dev local variable to the lp855x_probe(), to replace "&cl->dev"
and "lp->dev" in various places.
Also switch to dev_err_probe() in one case which takes care of not
printing -EPROBE_DEFER errors for us.
This is mostly a preparation for adding ACPI enumeration support which
will use the new "dev" variable more.
Signed-off-by: Hans de Goede
---
drivers/video/backlight/lp855x_bl.c | 29 +
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c
b/drivers/video/backlight/lp855x_bl.c
index 808ff00b2003..d1d27d5eb0f2 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -381,21 +381,22 @@ static int lp855x_parse_dt(struct lp855x *lp)
static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
{
+ struct device *dev = &cl->dev;
struct lp855x *lp;
int ret;
if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
return -EIO;
- lp = devm_kzalloc(&cl->dev, sizeof(struct lp855x), GFP_KERNEL);
+ lp = devm_kzalloc(dev, sizeof(struct lp855x), GFP_KERNEL);
if (!lp)
return -ENOMEM;
lp->client = cl;
- lp->dev = &cl->dev;
+ lp->dev = dev;
lp->chipname = id->name;
lp->chip_id = id->driver_data;
- lp->pdata = dev_get_platdata(&cl->dev);
+ lp->pdata = dev_get_platdata(dev);
switch (lp->chip_id) {
case LP8550:
@@ -424,30 +425,27 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
else
lp->mode = REGISTER_BASED;
- lp->supply = devm_regulator_get(lp->dev, "power");
+ lp->supply = devm_regulator_get(dev, "power");
if (IS_ERR(lp->supply)) {
if (PTR_ERR(lp->supply) == -EPROBE_DEFER)
return -EPROBE_DEFER;
lp->supply = NULL;
}
- lp->enable = devm_regulator_get_optional(lp->dev, "enable");
+ lp->enable = devm_regulator_get_optional(dev, "enable");
if (IS_ERR(lp->enable)) {
ret = PTR_ERR(lp->enable);
if (ret == -ENODEV) {
lp->enable = NULL;
} else {
- if (ret != -EPROBE_DEFER)
- dev_err(lp->dev, "error getting enable
regulator: %d\n",
- ret);
- return ret;
+ return dev_err_probe(dev, ret, "getting enable
regulator\n");
}
}
if (lp->supply) {
ret = regulator_enable(lp->supply);
if (ret < 0) {
- dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
+ dev_err(dev, "failed to enable supply: %d\n", ret);
return ret;
}
}
@@ -455,7 +453,7 @@ static int lp855x_probe(struct i2c_client *cl, const struct
i2c_device_id *id)
if (lp->enable) {
ret = regulator_enable(lp->enable);
if (ret < 0) {
- dev_err(lp->dev, "failed to enable vddio: %d\n", ret);
+ dev_err(dev, "failed to enable vddio: %d\n", ret);
goto disable_supply;
}
@@ -470,20 +468,19 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
ret = lp855x_configure(lp);
if (ret) {
- dev_err(lp->dev, "device config err: %d", ret);
+ dev_err(dev, "device config err: %d", ret);
goto disable_vddio;
}
ret = lp855x_backlight_register(lp);
if (ret) {
- dev_err(lp->dev,
- "failed to register backlight. err: %d\n", ret);
+ dev_err(dev, "failed to register backlight. err: %d\n", ret);
goto disable_vddio;
}
- ret = sysfs_create_group(&lp->dev->kobj, &lp855x_attr_group);
+ ret = sysfs_create_group(&dev->kobj, &lp855x_attr_group);
if (ret) {
- dev_err(lp->dev, "failed to register sysfs. err: %d\n", ret);
+ dev_err(dev, "failed to register sysfs. err: %d\n", ret);
goto disable_vddio;
}
--
2.31.1
[PATCH v2 0/2] drm/i915: Failsafe migration blits
This patch series introduces failsafe migration blits. The reason for this seemingly strange concept is that if the initial clearing or readback of LMEM fails for some reason[1], and we then set up either GPU- or CPU ptes to the allocated LMEM, we can expose old contents from other clients. So after each migration blit to LMEM, attach a dma-fence callback that checks the migration fence error value and if it's an error, performs a memcpy blit, instead. Patch 1 splits out the TTM move code into separate files Patch 2 implements the failsafe blits and related self-tests [1] There are at least two ways we could trigger exposure of uninitialized LMEM assuming the migration blits themselves never trigger a gpu hang. a) A gpu operation preceding a pipelined eviction blit resets and sets the error fence to -EIO, and the error is propagated across the TTM manager to the clear / swapin blit of a newly allocated TTM resource. It aborts and leaves the memory uninitialized. b) Something wedges the GT while a migration blit is submitted. It ends up never executed and TTM can fault user-space cpu-ptes into uninitialized memory. Thomas Hellström (2): drm/i915/ttm: Reorganize the ttm move code drm/i915/ttm: Failsafe migration blits drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 328 ++- drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 35 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 520 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 43 ++ .../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +- 6 files changed, 670 insertions(+), 281 deletions(-) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h -- 2.31.1
[PATCH v2 2/2] drm/i915/ttm: Failsafe migration blits
If the initial fill blit or copy blit of an object fails, the old
content of the data might be exposed and read as soon as either CPU- or
GPU PTEs are set up to point at the pages.
Intercept the blit fence with an async callback that checks the
blit fence for errors and if there are errors performs an async cpu blit
instead. If there is a failure to allocate the async dma_fence_work,
allocate it on the stack and sync wait for the blit to complete.
Add selftests that simulate gpu blit failures and failure to allocate
the async dma_fence_work.
A previous version of this pach used dma_fence_work, now that's
opencoded which adds more code but might lower the latency
somewhat in the common non-error case.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
3 files changed, 295 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 0ed6b7f2b95f..2e3328e2b48c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -18,6 +18,29 @@
#include "gt/intel_gt.h"
#include "gt/intel_migrate.h"
+/**
+ * DOC: Selftest failure modes for failsafe migration:
+ *
+ * For fail_gpu_migration, the gpu blit scheduled is always a clear blit
+ * rather than a copy blit, and then we force the failure paths as if
+ * the blit fence returned an error.
+ *
+ * For fail_work_allocation we fail the kmalloc of the async worker, we
+ * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
+ * true, then a memcpy operation is performed sync.
+ */
+I915_SELFTEST_DECLARE(static bool fail_gpu_migration;)
+I915_SELFTEST_DECLARE(static bool fail_work_allocation;)
+
+#ifdef CONFIG_DRM_I915_SELFTEST
+void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
+ bool work_allocation)
+{
+ fail_gpu_migration = gpu_migration;
+ fail_work_allocation = work_allocation;
+}
+#endif
+
static enum i915_cache_level
i915_ttm_cache_level(struct drm_i915_private *i915, struct ttm_resource *res,
struct ttm_tt *ttm)
@@ -129,11 +152,11 @@ int i915_ttm_move_notify(struct ttm_buffer_object *bo)
return 0;
}
-static int i915_ttm_accel_move(struct ttm_buffer_object *bo,
- bool clear,
- struct ttm_resource *dst_mem,
- struct ttm_tt *dst_ttm,
- struct sg_table *dst_st)
+static struct dma_fence *i915_ttm_accel_move(struct ttm_buffer_object *bo,
+bool clear,
+struct ttm_resource *dst_mem,
+struct ttm_tt *dst_ttm,
+struct sg_table *dst_st)
{
struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915),
bdev);
@@ -144,30 +167,29 @@ static int i915_ttm_accel_move(struct ttm_buffer_object
*bo,
int ret;
if (!i915->gt.migrate.context || intel_gt_is_wedged(&i915->gt))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+
+ /* With fail_gpu_migration, we always perform a GPU clear. */
+ if (I915_SELFTEST_ONLY(fail_gpu_migration))
+ clear = true;
dst_level = i915_ttm_cache_level(i915, dst_mem, dst_ttm);
if (clear) {
- if (bo->type == ttm_bo_type_kernel)
- return -EINVAL;
+ if (bo->type == ttm_bo_type_kernel &&
+ !I915_SELFTEST_ONLY(fail_gpu_migration))
+ return ERR_PTR(-EINVAL);
intel_engine_pm_get(i915->gt.migrate.context->engine);
ret = intel_context_migrate_clear(i915->gt.migrate.context,
NULL,
dst_st->sgl, dst_level,
i915_ttm_gtt_binds_lmem(dst_mem),
0, &rq);
-
- if (!ret && rq) {
- i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
- i915_request_put(rq);
- }
- intel_engine_pm_put(i915->gt.migrate.context->engine);
} else {
struct i915_refct_sgt *src_rsgt =
i915_ttm_resource_get_st(obj, bo->resource);
if (IS_ERR(src_rsgt))
- return PTR_ERR(src_rsgt);
+ return ERR_CAST(src_rsgt);
src_level = i915_ttm_cache_level(i915, bo->resource, src_ttm);
intel_engine_pm_get(i915->gt.migrate.context->engine);
@@ -178,15 +200,183 @@ static int i915_tt
[PATCH v2 1/2] drm/i915/ttm: Reorganize the ttm move code
We are about to introduce failsafe- and asynchronous migration and
ttm moves.
This will add complexity and code to the TTM move code so it makes sense
to split it out to a separate file to make the i915 TTM code easer to
digest.
Split the i915 TTM move code out and since we will have to change the name
of the gpu_binds_iomem() and cpu_maps_iomem() functions anyway,
we alter the name of gpu_binds_iomem() to i915_ttm_gtt_binds_lmem() which
is more reflecting what it is used for.
With this we also add some more documentation. Otherwise there should be
no functional change.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/Makefile| 1 +
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 328 +++
drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 35 ++
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 308 +
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 38 +++
5 files changed, 430 insertions(+), 280 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 467872cca027..7d0d0b814670 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -154,6 +154,7 @@ gem-y += \
gem/i915_gem_throttle.o \
gem/i915_gem_tiling.o \
gem/i915_gem_ttm.o \
+ gem/i915_gem_ttm_move.o \
gem/i915_gem_ttm_pm.o \
gem/i915_gem_userptr.o \
gem/i915_gem_wait.o \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 6a05369e2705..6369fb9b2455 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -14,13 +14,9 @@
#include "gem/i915_gem_object.h"
#include "gem/i915_gem_region.h"
#include "gem/i915_gem_ttm.h"
+#include "gem/i915_gem_ttm_move.h"
#include "gem/i915_gem_ttm_pm.h"
-
-#include "gt/intel_engine_pm.h"
-#include "gt/intel_gt.h"
-#include "gt/intel_migrate.h"
-
#define I915_TTM_PRIO_PURGE 0
#define I915_TTM_PRIO_NO_PAGES 1
#define I915_TTM_PRIO_HAS_PAGES 2
@@ -108,28 +104,6 @@ static int i915_ttm_err_to_gem(int err)
return err;
}
-static bool gpu_binds_iomem(struct ttm_resource *mem)
-{
- return mem->mem_type != TTM_PL_SYSTEM;
-}
-
-static bool cpu_maps_iomem(struct ttm_resource *mem)
-{
- /* Once / if we support GGTT, this is also false for cached ttm_tts */
- return mem->mem_type != TTM_PL_SYSTEM;
-}
-
-static enum i915_cache_level
-i915_ttm_cache_level(struct drm_i915_private *i915, struct ttm_resource *res,
-struct ttm_tt *ttm)
-{
- return ((HAS_LLC(i915) || HAS_SNOOP(i915)) && !gpu_binds_iomem(res) &&
- ttm->caching == ttm_cached) ? I915_CACHE_LLC :
- I915_CACHE_NONE;
-}
-
-static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj);
-
static enum ttm_caching
i915_ttm_select_tt_caching(const struct drm_i915_gem_object *obj)
{
@@ -370,23 +344,14 @@ static void i915_ttm_evict_flags(struct ttm_buffer_object
*bo,
*placement = i915_sys_placement;
}
-static int i915_ttm_move_notify(struct ttm_buffer_object *bo)
-{
- struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
- int ret;
-
- ret = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE);
- if (ret)
- return ret;
-
- ret = __i915_gem_object_put_pages(obj);
- if (ret)
- return ret;
-
- return 0;
-}
-
-static void i915_ttm_free_cached_io_rsgt(struct drm_i915_gem_object *obj)
+/**
+ * i915_ttm_free_cached_io_rsgt - Free object cached LMEM information
+ * @obj: The GEM object
+ * This function frees any LMEM-related information that is cached on
+ * the object. For example the radix tree for fast page lookup and the
+ * cached refcounted sg-table
+ */
+void i915_ttm_free_cached_io_rsgt(struct drm_i915_gem_object *obj)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -403,56 +368,16 @@ static void i915_ttm_free_cached_io_rsgt(struct
drm_i915_gem_object *obj)
obj->ttm.cached_io_rsgt = NULL;
}
-static void
-i915_ttm_adjust_domains_after_move(struct drm_i915_gem_object *obj)
-{
- struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
-
- if (cpu_maps_iomem(bo->resource) || bo->ttm->caching != ttm_cached) {
- obj->write_domain = I915_GEM_DOMAIN_WC;
- obj->read_domains = I915_GEM_DOMAIN_WC;
- } else {
- obj->write_domain = I915_GEM_DOMAIN_CPU;
- obj->read_domains = I915_GEM_DOMAIN_CPU;
- }
-}
-
-static void i915_ttm_adjust_gem_after_move(struct drm_i915_gem_object *obj)
-{
- struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
- unsigned int cache_level;
- unsigned int i;
-
- /*
-* If object was moved to an allowable region, update the object
-* region to consider it
Re: [PATCH v3 24/34] drm/tilcdc: Migrate to aggregate driver
On 2021-10-26 3:00, Stephen Boyd wrote:
Use an aggregate driver instead of component ops so that we can get
proper driver probe ordering of the aggregate device with respect to
all
the component devices that make up the aggregate device.
Cc: Jyri Sarha
Cc: Tomi Valkeinen
Cc: Daniel Vetter
Cc: "Rafael J. Wysocki"
Cc: Rob Clark
Cc: Russell King
Cc: Saravana Kannan
Signed-off-by: Stephen Boyd
---
Tested-by: Jyri Sarha
Thanks,
Jyri
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 6b03f89a98d4..d5c6567eec8d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -531,13 +531,16 @@ static const struct dev_pm_ops tilcdc_pm_ops = {
/*
* Platform driver:
*/
-static int tilcdc_bind(struct device *dev)
+static int tilcdc_bind(struct aggregate_device *adev)
{
+ struct device *dev = adev->parent;
+
return tilcdc_init(&tilcdc_driver, dev);
}
-static void tilcdc_unbind(struct device *dev)
+static void tilcdc_unbind(struct aggregate_device *adev)
{
+ struct device *dev = adev->parent;
struct drm_device *ddev = dev_get_drvdata(dev);
/* Check if a subcomponent has already triggered the unloading. */
@@ -547,9 +550,13 @@ static void tilcdc_unbind(struct device *dev)
tilcdc_fini(dev_get_drvdata(dev));
}
-static const struct component_master_ops tilcdc_comp_ops = {
- .bind = tilcdc_bind,
- .unbind = tilcdc_unbind,
+static struct aggregate_driver tilcdc_aggregate_driver = {
+ .probe = tilcdc_bind,
+ .remove = tilcdc_unbind,
+ .driver = {
+ .name = "tilcdc_drm",
+ .owner = THIS_MODULE,
+ },
};
static int tilcdc_pdev_probe(struct platform_device *pdev)
@@ -566,12 +573,9 @@ static int tilcdc_pdev_probe(struct
platform_device *pdev)
ret = tilcdc_get_external_components(&pdev->dev, &match);
if (ret < 0)
return ret;
- else if (ret == 0)
+ if (ret == 0)
return tilcdc_init(&tilcdc_driver, &pdev->dev);
- else
- return component_master_add_with_match(&pdev->dev,
- &tilcdc_comp_ops,
- match);
+ return component_aggregate_register(&pdev->dev,
&tilcdc_aggregate_driver, match);
}
static int tilcdc_pdev_remove(struct platform_device *pdev)
@@ -581,10 +585,10 @@ static int tilcdc_pdev_remove(struct
platform_device *pdev)
ret = tilcdc_get_external_components(&pdev->dev, NULL);
if (ret < 0)
return ret;
- else if (ret == 0)
+ if (ret == 0)
tilcdc_fini(platform_get_drvdata(pdev));
else
- component_master_del(&pdev->dev, &tilcdc_comp_ops);
+ component_aggregate_unregister(&pdev->dev,
&tilcdc_aggregate_driver);
return 0;
}
[Bug 214859] drm-amdgpu-init-iommu~fd-device-init.patch introduce bug
https://bugzilla.kernel.org/show_bug.cgi?id=214859 --- Comment #2 from t...@siduction.org --- The relevant commit is 714d9e4574d54596973ee3b0624ee4a16264d700 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 214859] drm-amdgpu-init-iommu~fd-device-init.patch introduce bug
https://bugzilla.kernel.org/show_bug.cgi?id=214859 t...@siduction.org changed: What|Removed |Added Kernel Version|5.14.15 |5.14.15, 5.15.0 Regression|No |Yes -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document
On Mon, 01 Nov 2021 19:01:07 +0800, tommy-huang wrote: > Add ast2600-gfx description for gfx driver. > > Signed-off-by: tommy-huang > --- > Documentation/devicetree/bindings/gpu/aspeed-gfx.txt | 1 + > 1 file changed, 1 insertion(+) > Please add Acked-by/Reviewed-by tags when posting new versions. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for acks received on the version they apply. If a tag was not added on purpose, please state why and what changed.
[Bug 214859] drm-amdgpu-init-iommu~fd-device-init.patch introduce bug
https://bugzilla.kernel.org/show_bug.cgi?id=214859 --- Comment #3 from t...@siduction.org --- Additional info, after installing the kernel from a working system, 1st boot with that kernel is working flawless. Rebooting with that kernel and the boot is hanging a long time, then the desktop starts but the system is not really usuable. All the problems do not happen after reverting 714d9e4574d54596973ee3b0624ee4a16264d700. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v2 0/3] drm/i915/guc/slpc: Implement waitboost for SLPC
On Sun, 31 Oct 2021 21:39:34 -0700, Belgaumkar, Vinay wrote: > > Waitboost is a legacy feature implemented in the Host Turbo algorithm. This > patch set implements it for the SLPC path. A "boost" happens when user > calls gem_wait ioctl on a submission that has not landed on HW yet. Afaiu user doesn't have to call gem_wait, the boost will happen whenever a request waits to be submitted to GuC because of an unmet depedency. This has to be done from i915 because GuC has not yet seen the request. Rest of the cover letter is fine.
Re: [PATCH 1/3] drm/i915/guc/slpc: Define and initialize boost frequency
On Sun, 31 Oct 2021 21:39:35 -0700, Belgaumkar, Vinay wrote:
>
> Define helpers and struct members required to record boost info.
> Boost frequency is initialized to RP0 at SLPC init. Also define num_waiters
> which can track the pending boost requests.
>
> Boost will be done by scheduling a worker thread. This will allow
> us to make H2G calls inside an interrupt context. Initialize the
"to not make H2G calls from interrupt context" is probably better.
> +static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq)
> +{
> + struct drm_i915_private *i915 = slpc_to_i915(slpc);
> + intel_wakeref_t wakeref;
> + int ret = 0;
> +
> + lockdep_assert_held(&slpc->lock);
> +
> + /**
nit: this I believe should just be
/*
/** I believe shows up in kerneldoc so shouldn't be used unless we want
something in kerneldoc.
> + * This function is a little different as compared to
> + * intel_guc_slpc_set_min_freq(). Softlimit will not be updated
> + * here since this is used to temporarily change min freq,
> + * for example, during a waitboost. Caller is responsible for
> + * checking bounds.
> + */
> +
> + with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
> + ret = slpc_set_param(slpc,
> + SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> + freq);
> + if (ret)
> + drm_err(&i915->drm, "Unable to force min freq to %u:
> %d",
Probably drm_err_ratelimited since it's called at run time not only at
init? Not sure if drm_err_once suffizes, probably not.
> + freq, ret);
> + }
> +
> + return ret;
> +}
> +
> +static void slpc_boost_work(struct work_struct *work)
> +{
> + struct intel_guc_slpc *slpc = container_of(work, typeof(*slpc),
> boost_work);
> +
> + /* Raise min freq to boost. It's possible that
> + * this is greater than current max. But it will
> + * certainly be limited by RP0. An error setting
> + * the min param is not fatal.
> + */
nit: do we follow the following format for multi-line comments,
Documentation/process/coding-style.rst mentions this:
/*
* Line 1
* Line 2
*/
Re: [PATCH 2/3] drm/i915/guc/slpc: Add waitboost functionality for SLPC
On Sun, 31 Oct 2021 21:39:36 -0700, Belgaumkar, Vinay wrote:
>
> @@ -945,6 +960,17 @@ void intel_rps_boost(struct i915_request *rq)
> if (!test_and_set_bit(I915_FENCE_FLAG_BOOST, &rq->fence.flags)) {
> struct intel_rps *rps = &READ_ONCE(rq->engine)->gt->rps;
>
> + if (rps_uses_slpc(rps)) {
> + slpc = rps_to_slpc(rps);
> +
> + /* Return if old value is non zero */
> + if (atomic_fetch_inc(&slpc->num_waiters))
> + return;
> +
> + if (intel_rps_get_requested_frequency(rps) <
> slpc->boost_freq)
I think this check is not needed because:
a. The waitboost code only changes min_freq. i915 code should not depend on
how GuC changes requested_freq in response to change in min_freq.
b. What is more worrisome is that when we "de-boost" we set min_freq to
min_freq_softlimit. If GuC e.g. has a delay in bringing requested_freq
down and intel_rps_boost() gets called meanwhile we will miss the one
opportunity we have to boost the freq (when num_waiters goes from 0 to
1. Asking GuC to boost when actual_freq is already boost_freq is
harmless in comparison). So to avoid this risk of missing the chance to
boost I think we should delete this check and replace the code above
with something like:
if (rps_uses_slpc(rps)) {
struct intel_guc_slpc *slpc = rps_to_slpc(rps);
if (slpc->boost_freq <= slpc->min_freq_softlimit)
return;
if (!atomic_fetch_inc(&slpc->num_waiters))
schedule_work(&slpc->boost_work);
return;
}
Note that this check:
if (slpc->boost_freq <= slpc->min_freq_softlimit)
return;
(which is basically a degenerate case in which we don't have to do
anything), can be probably be implemented when boost_freq is set in sysfs,
or may already be encompassed in "val < slpc->min_freq" in
intel_guc_slpc_set_boost_freq() in which case this check can also be
skipped from this function.
> +void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc)
> +{
> + /* Return min back to the softlimit.
> + * This is called during request retire,
> + * so we don't need to fail that if the
> + * set_param fails.
> + */
nit: maybe follow kernel multi-line comment format.
Re: [PATCH 3/3] drm/i915/guc/slpc: Update boost sysfs hooks for SLPC
On Sun, 31 Oct 2021 21:39:37 -0700, Belgaumkar, Vinay wrote: > > +static int set_boost_freq(struct intel_rps *rps, u32 val) Since this is legacy rps code path maybe change function name to rps_set_boost_freq?
Re: [PATCH v3 03/10] drm/i915: Restructure probe to handle multi-tile platforms
Hi Matt, On Thu, Oct 28, 2021 at 08:28:10PM -0700, Matt Roper wrote: > On a multi-tile platform, each tile has its own registers + GGTT space, > and BAR 0 is extended to cover all of them. Upcoming patches will start > exposing the tiles as multiple GTs within a single PCI device. In > preparation for supporting such setups, restructure the driver's probe > code a bit. > > Only the primary/root tile is initialized for now; the other tiles will > be detected and plugged in by future patches once the necessary > infrastructure is in place to handle them. > > v2: > - Rename for naming prefix consistency. (Jani, Lucas) > > Original-author: Abdiel Janulgue > Cc: Daniele Ceraolo Spurio > Cc: Matthew Auld > Cc: Joonas Lahtinen > Cc: Lucas De Marchi > Cc: Jani Nikula > Signed-off-by: Daniele Ceraolo Spurio > Signed-off-by: Tvrtko Ursulin > Signed-off-by: Matt Roper Looks correct to me: Reviewed-by: Andi Shyti Andi
Re: [PATCH v3 05/10] drm/i915: Prepare for multiple gts
Hi Matt and Tvrtko,
[...]
> static int
> intel_gt_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t
> phys_addr)
we don't actually need 'id', it's gt->info.id. It's introduced in
patch 3 with the value '0' but it's not needed.
> {
> + struct drm_i915_private *i915 = gt->i915;
> + struct intel_uncore *uncore;
> + struct intel_uncore_mmio_debug *mmio_debug;
> int ret;
>
> - intel_uncore_init_early(gt->uncore, gt);
> + if (id) {
if (gt->info.id) ?
Andi
> + uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);
> + if (!uncore)
> + return -ENOMEM;
> +
> + mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL);
> + if (!mmio_debug) {
> + kfree(uncore);
> + return -ENOMEM;
> + }
> +
> + __intel_gt_init_early(gt, uncore, i915);
> + } else {
> + uncore = &i915->uncore;
> + mmio_debug = &i915->mmio_debug;
> + }
> +
> + intel_uncore_init_early(uncore, gt);
>
> ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
Re: [PATCH v3 03/10] drm/i915: Restructure probe to handle multi-tile platforms
Hi Matt,
> +static int
> +intel_gt_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t
> phys_addr)
I have already r-b this, but, as I commented in patch 5, 'id' is
redundant. Can we remove it?
Andi
> +{
> + int ret;
> +
> + intel_uncore_init_early(gt->uncore, gt->i915);
> +
> + ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
> + if (ret)
> + return ret;
> +
> + gt->phys_addr = phys_addr;
> +
> + return 0;
> +}
[...]
> + /* We always have at least one primary GT on any device */
> + ret = intel_gt_tile_setup(&i915->gt, 0, phys_addr);
> + if (ret)
> + return ret;
> +
> + /* TODO: add more tiles */
> + return 0;
> +}
Re: [PATCH v3 07/10] drm/i915/xehp: Determine which tile raised an interrupt
Hi Matt and Paulo,
> @@ -2771,40 +2771,45 @@ static inline void dg1_master_intr_enable(void
> __iomem * const regs)
> static irqreturn_t dg1_irq_handler(int irq, void *arg)
> {
> struct drm_i915_private * const i915 = arg;
> + void __iomem * const t0_regs = i915->gt.uncore->regs;
> struct intel_gt *gt = &i915->gt;
> - void __iomem * const regs = gt->uncore->regs;
> u32 master_tile_ctl, master_ctl;
> - u32 gu_misc_iir;
> + u32 gu_misc_iir = 0;
just a nitpick, this doesn't need to be initialize and you could
also insert it inside the for_each_gt()
Reviewed-by: Andi Shyti
Thanks,
Andi
> + unsigned int i;
>
> if (!intel_irqs_enabled(i915))
> return IRQ_NONE;
>
> - master_tile_ctl = dg1_master_intr_disable(regs);
> + master_tile_ctl = dg1_master_intr_disable(t0_regs);
> if (!master_tile_ctl) {
> - dg1_master_intr_enable(regs);
> + dg1_master_intr_enable(t0_regs);
> return IRQ_NONE;
> }
>
> - /* FIXME: we only support tile 0 for now. */
> - if (master_tile_ctl & DG1_MSTR_TILE(0)) {
> + for_each_gt(i915, i, gt) {
> + void __iomem *const regs = gt->uncore->regs;
> +
> + if ((master_tile_ctl & DG1_MSTR_TILE(i)) == 0)
> + continue;
> +
> master_ctl = raw_reg_read(regs, GEN11_GFX_MSTR_IRQ);
> raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, master_ctl);
> - } else {
> - DRM_ERROR("Tile not supported: 0x%08x\n", master_tile_ctl);
> - dg1_master_intr_enable(regs);
> - return IRQ_NONE;
> - }
>
> - gen11_gt_irq_handler(gt, master_ctl);
> + gen11_gt_irq_handler(gt, master_ctl);
>
> - if (master_ctl & GEN11_DISPLAY_IRQ)
> - gen11_display_irq_handler(i915);
> -
> - gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
> + /*
> + * In practice we'll only get display and gu_misc interrupts
> + * for the GSE on tile0, but it's still simplest to process
> + * them inside the loop.
> + */
> + if (master_ctl & GEN11_DISPLAY_IRQ)
> + gen11_display_irq_handler(i915);
>
> - dg1_master_intr_enable(regs);
> + gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
> + gen11_gu_misc_irq_handler(gt, gu_misc_iir);
> + }
>
> - gen11_gu_misc_irq_handler(gt, gu_misc_iir);
> + dg1_master_intr_enable(t0_regs);
>
> pmu_irq_stats(i915, IRQ_HANDLED);
>
> --
> 2.33.0
Re: [PATCH v3 09/10] drm/i915/guc: Update CT debug macro for multi-tile
Hi Matt and Michal, On Thu, Oct 28, 2021 at 08:28:16PM -0700, Matt Roper wrote: > From: Michal Wajdeczko > > Update CT debug macros by including tile ID in all messages. > > Cc: Michał Winiarski > Signed-off-by: Michal Wajdeczko > Signed-off-by: Matt Roper Reviewed-by: Andi Shyti Andi
Re: [PATCH v2 0/3] drm/i915/guc/slpc: Implement waitboost for SLPC
On 11/1/2021 1:24 PM, Dixit, Ashutosh wrote: On Sun, 31 Oct 2021 21:39:34 -0700, Belgaumkar, Vinay wrote: Waitboost is a legacy feature implemented in the Host Turbo algorithm. This patch set implements it for the SLPC path. A "boost" happens when user calls gem_wait ioctl on a submission that has not landed on HW yet. Afaiu user doesn't have to call gem_wait, the boost will happen whenever a request waits to be submitted to GuC because of an unmet depedency. This has to be done from i915 because GuC has not yet seen the request. Rest of the cover letter is fine. Ok, thanks, Vinay.
Re: [PATCH 2/3] drm/i915/guc/slpc: Add waitboost functionality for SLPC
On 11/1/2021 1:28 PM, Dixit, Ashutosh wrote:
On Sun, 31 Oct 2021 21:39:36 -0700, Belgaumkar, Vinay wrote:
@@ -945,6 +960,17 @@ void intel_rps_boost(struct i915_request *rq)
if (!test_and_set_bit(I915_FENCE_FLAG_BOOST, &rq->fence.flags)) {
struct intel_rps *rps = &READ_ONCE(rq->engine)->gt->rps;
+ if (rps_uses_slpc(rps)) {
+ slpc = rps_to_slpc(rps);
+
+ /* Return if old value is non zero */
+ if (atomic_fetch_inc(&slpc->num_waiters))
+ return;
+
+ if (intel_rps_get_requested_frequency(rps) <
slpc->boost_freq)
I think this check is not needed because:
a. The waitboost code only changes min_freq. i915 code should not depend on
how GuC changes requested_freq in response to change in min_freq.
b. What is more worrisome is that when we "de-boost" we set min_freq to
min_freq_softlimit. If GuC e.g. has a delay in bringing requested_freq
down and intel_rps_boost() gets called meanwhile we will miss the one
opportunity we have to boost the freq (when num_waiters goes from 0 to
1. Asking GuC to boost when actual_freq is already boost_freq is
harmless in comparison). So to avoid this risk of missing the chance to
boost I think we should delete this check and replace the code above
with something like:
if (rps_uses_slpc(rps)) {
struct intel_guc_slpc *slpc = rps_to_slpc(rps);
if (slpc->boost_freq <= slpc->min_freq_softlimit)
return;
if (!atomic_fetch_inc(&slpc->num_waiters))
schedule_work(&slpc->boost_work);
return;
}
Note that this check:
if (slpc->boost_freq <= slpc->min_freq_softlimit)
return;
(which is basically a degenerate case in which we don't have to do
anything), can be probably be implemented when boost_freq is set in sysfs,
or may already be encompassed in "val < slpc->min_freq" in
intel_guc_slpc_set_boost_freq() in which case this check can also be
skipped from this function.
We already have that check in set_boost_freq function. So, just adding
the atomic_fetch_inc check.
+void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc)
+{
+ /* Return min back to the softlimit.
+* This is called during request retire,
+* so we don't need to fail that if the
+* set_param fails.
+*/
nit: maybe follow kernel multi-line comment format.
Ok.
Thanks,
Vinay.
RE: [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document
Hi Rob, Thanks for your comments. I got it. By Tommy > -Original Message- > From: Rob Herring > Sent: Tuesday, November 2, 2021 4:02 AM > To: Tommy Huang > Cc: dri-devel@lists.freedesktop.org; linux-arm-ker...@lists.infradead.org; > linux-asp...@lists.ozlabs.org; dan...@ffwll.ch; BMC-SW > ; devicet...@vger.kernel.org; > linux-ker...@vger.kernel.org; robh...@kernel.org; j...@jms.id.au; > and...@aj.id.au; airl...@linux.ie > Subject: Re: [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document > > On Mon, 01 Nov 2021 19:01:07 +0800, tommy-huang wrote: > > Add ast2600-gfx description for gfx driver. > > > > Signed-off-by: tommy-huang > > --- > > Documentation/devicetree/bindings/gpu/aspeed-gfx.txt | 1 + > > 1 file changed, 1 insertion(+) > > > > > Please add Acked-by/Reviewed-by tags when posting new versions. However, > there's no need to repost patches *only* to add the tags. The upstream > maintainer will do that for acks received on the version they apply. > > If a tag was not added on purpose, please state why and what changed.
Re: [PATCH 1/3] drm/i915/guc/slpc: Define and initialize boost frequency
On 11/1/2021 1:26 PM, Dixit, Ashutosh wrote:
On Sun, 31 Oct 2021 21:39:35 -0700, Belgaumkar, Vinay wrote:
Define helpers and struct members required to record boost info.
Boost frequency is initialized to RP0 at SLPC init. Also define num_waiters
which can track the pending boost requests.
Boost will be done by scheduling a worker thread. This will allow
us to make H2G calls inside an interrupt context. Initialize the
"to not make H2G calls from interrupt context" is probably better.
+static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq)
+{
+ struct drm_i915_private *i915 = slpc_to_i915(slpc);
+ intel_wakeref_t wakeref;
+ int ret = 0;
+
+ lockdep_assert_held(&slpc->lock);
+
+ /**
nit: this I believe should just be
/*
ok.
/** I believe shows up in kerneldoc so shouldn't be used unless we want
something in kerneldoc.
+* This function is a little different as compared to
+* intel_guc_slpc_set_min_freq(). Softlimit will not be updated
+* here since this is used to temporarily change min freq,
+* for example, during a waitboost. Caller is responsible for
+* checking bounds.
+*/
+
+ with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
+ ret = slpc_set_param(slpc,
+SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+freq);
+ if (ret)
+ drm_err(&i915->drm, "Unable to force min freq to %u:
%d",
Probably drm_err_ratelimited since it's called at run time not only at
init? Not sure if drm_err_once suffizes, probably not.
Keeping it drm_err as discussed offline.
+ freq, ret);
+ }
+
+ return ret;
+}
+
+static void slpc_boost_work(struct work_struct *work)
+{
+ struct intel_guc_slpc *slpc = container_of(work, typeof(*slpc),
boost_work);
+
+ /* Raise min freq to boost. It's possible that
+* this is greater than current max. But it will
+* certainly be limited by RP0. An error setting
+* the min param is not fatal.
+*/
nit: do we follow the following format for multi-line comments,
Documentation/process/coding-style.rst mentions this:
/*
* Line 1
* Line 2
*/
Ok.
Thanks,
Vinay.
[PATCH v3 0/3] drm/i915/guc/slpc: Implement waitboost for SLPC
Waitboost is a legacy feature implemented in the Host Turbo algorithm. This patch set implements it for the SLPC path. A boost can happen when a request is waiting for an unmet dependency. GT frequency gets temporarily bumped to boost freq to allow the previous request to finish quickly. We achieve this on SLPC by setting the min frequency, SLPC will set that as the requested frequency. The boost will occur through a worker thread that will be scheduled when the required conditions are met. Like before, boost frequency is configurable through sysfs, so we can adjust it to any specific value as long as it is between [min, RP0]. v2: Add a worker thread to perform freq boost. v3: Address comments (Ashutosh) Cc: Ashutosh Dixit Signed-off-by: Vinay Belgaumkar Vinay Belgaumkar (3): drm/i915/guc/slpc: Define and initialize boost frequency drm/i915/guc/slpc: Add waitboost functionality for SLPC drm/i915/guc/slpc: Update boost sysfs hooks for SLPC drivers/gpu/drm/i915/gt/intel_rps.c | 72 + drivers/gpu/drm/i915/gt/intel_rps.h | 3 + drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 151 +++--- drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 3 + .../gpu/drm/i915/gt/uc/intel_guc_slpc_types.h | 13 ++ drivers/gpu/drm/i915/i915_request.c | 2 +- drivers/gpu/drm/i915/i915_sysfs.c | 19 +-- 7 files changed, 223 insertions(+), 40 deletions(-) -- 2.25.0
[PATCH 1/3] drm/i915/guc/slpc: Define and initialize boost frequency
Define helpers and struct members required to record boost info.
Boost frequency is initialized to RP0 at SLPC init. Also define num_waiters
which can track the pending boost requests.
Boost will be done by scheduling a worker thread. This will avoid
the need to make H2G calls inside an interrupt context. Initialize the
worker function during SLPC init as well. Had to move intel_guc_slpc_init
a few lines below to accomodate this.
v2: Add a workqueue to handle waitboost
v3: Code review comments (Ashutosh)
Cc: Ashutosh Dixit
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 102 ++
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 1 +
.../gpu/drm/i915/gt/uc/intel_guc_slpc_types.h | 13 +++
3 files changed, 93 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index 65a3e7fdb2b2..3a9750af0bdf 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -79,29 +79,6 @@ static void slpc_mem_set_disabled(struct slpc_shared_data
*data,
slpc_mem_set_param(data, enable_id, 0);
}
-int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
-{
- struct intel_guc *guc = slpc_to_guc(slpc);
- struct drm_i915_private *i915 = slpc_to_i915(slpc);
- u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data));
- int err;
-
- GEM_BUG_ON(slpc->vma);
-
- err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma, (void
**)&slpc->vaddr);
- if (unlikely(err)) {
- drm_err(&i915->drm,
- "Failed to allocate SLPC struct (err=%pe)\n",
- ERR_PTR(err));
- return err;
- }
-
- slpc->max_freq_softlimit = 0;
- slpc->min_freq_softlimit = 0;
-
- return err;
-}
-
static u32 slpc_get_state(struct intel_guc_slpc *slpc)
{
struct slpc_shared_data *data;
@@ -203,6 +180,82 @@ static int slpc_unset_param(struct intel_guc_slpc *slpc,
return guc_action_slpc_unset_param(guc, id);
}
+static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq)
+{
+ struct drm_i915_private *i915 = slpc_to_i915(slpc);
+ intel_wakeref_t wakeref;
+ int ret = 0;
+
+ lockdep_assert_held(&slpc->lock);
+
+ /*
+* This function is a little different as compared to
+* intel_guc_slpc_set_min_freq(). Softlimit will not be updated
+* here since this is used to temporarily change min freq,
+* for example, during a waitboost. Caller is responsible for
+* checking bounds.
+*/
+
+ with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
+ ret = slpc_set_param(slpc,
+SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+freq);
+ if (ret)
+ drm_err(&i915->drm, "Unable to force min freq to %u:
%d",
+ freq, ret);
+ }
+
+ return ret;
+}
+
+static void slpc_boost_work(struct work_struct *work)
+{
+ struct intel_guc_slpc *slpc = container_of(work, typeof(*slpc),
boost_work);
+
+ /*
+* Raise min freq to boost. It's possible that
+* this is greater than current max. But it will
+* certainly be limited by RP0. An error setting
+* the min param is not fatal.
+*/
+ mutex_lock(&slpc->lock);
+ if (atomic_read(&slpc->num_waiters)) {
+ slpc_force_min_freq(slpc, slpc->boost_freq);
+ slpc->num_boosts++;
+ }
+ mutex_unlock(&slpc->lock);
+}
+
+int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
+{
+ struct intel_guc *guc = slpc_to_guc(slpc);
+ struct drm_i915_private *i915 = slpc_to_i915(slpc);
+ u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data));
+ int err;
+
+ GEM_BUG_ON(slpc->vma);
+
+ err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma, (void
**)&slpc->vaddr);
+ if (unlikely(err)) {
+ drm_err(&i915->drm,
+ "Failed to allocate SLPC struct (err=%pe)\n",
+ ERR_PTR(err));
+ return err;
+ }
+
+ slpc->max_freq_softlimit = 0;
+ slpc->min_freq_softlimit = 0;
+
+ slpc->boost_freq = 0;
+ atomic_set(&slpc->num_waiters, 0);
+ slpc->num_boosts = 0;
+
+ mutex_init(&slpc->lock);
+ INIT_WORK(&slpc->boost_work, slpc_boost_work);
+
+ return err;
+}
+
static const char *slpc_global_state_to_string(enum slpc_global_state state)
{
switch (state) {
@@ -522,6 +575,9 @@ static void slpc_get_rp_values(struct intel_guc_slpc *slpc)
GT_FREQUENCY_MULTIPLIER;
slpc->min_freq = REG_FIELD_GET(RPN_CAP_MASK, rp_state_cap) *
GT_FREQUENCY_MULTIPLIER;
+
+ if (!slpc->boost_fre
[PATCH 2/3] drm/i915/guc/slpc: Add waitboost functionality for SLPC
Add helper in RPS code for handling SLPC and non-SLPC paths.
When boost is requested in the SLPC path, we can ask GuC to ramp
up the frequency req by setting the minimum frequency to boost freq.
Reset freq back to the min softlimit when there are no more waiters.
v2: Schedule a worker thread which can boost freq from within
an interrupt context as well.
v3: No need to check against requested freq before scheduling boost
work (Ashutosh)
Cc: Ashutosh Dixit
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/intel_rps.c | 25 +
drivers/gpu/drm/i915/gt/intel_rps.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 20 +
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 1 +
drivers/gpu/drm/i915/i915_request.c | 2 +-
5 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 5e275f8dda8c..6f310c9d9765 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -936,8 +936,23 @@ void intel_rps_park(struct intel_rps *rps)
GT_TRACE(rps_to_gt(rps), "park:%x\n", rps->cur_freq);
}
+void intel_rps_dec_waiters(struct intel_rps *rps)
+{
+ struct intel_guc_slpc *slpc;
+
+ if (rps_uses_slpc(rps)) {
+ slpc = rps_to_slpc(rps);
+
+ intel_guc_slpc_dec_waiters(slpc);
+ } else {
+ atomic_dec(&rps->num_waiters);
+ }
+}
+
void intel_rps_boost(struct i915_request *rq)
{
+ struct intel_guc_slpc *slpc;
+
if (i915_request_signaled(rq) || i915_request_has_waitboost(rq))
return;
@@ -945,6 +960,16 @@ void intel_rps_boost(struct i915_request *rq)
if (!test_and_set_bit(I915_FENCE_FLAG_BOOST, &rq->fence.flags)) {
struct intel_rps *rps = &READ_ONCE(rq->engine)->gt->rps;
+ if (rps_uses_slpc(rps)) {
+ slpc = rps_to_slpc(rps);
+
+ /* Return if old value is non zero */
+ if (!atomic_fetch_inc(&slpc->num_waiters))
+ schedule_work(&slpc->boost_work);
+
+ return;
+ }
+
if (atomic_fetch_inc(&rps->num_waiters))
return;
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index 11960d64ca82..407e878d5006 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -23,6 +23,7 @@ void intel_rps_disable(struct intel_rps *rps);
void intel_rps_park(struct intel_rps *rps);
void intel_rps_unpark(struct intel_rps *rps);
void intel_rps_boost(struct i915_request *rq);
+void intel_rps_dec_waiters(struct intel_rps *rps);
int intel_rps_set(struct intel_rps *rps, u8 val);
void intel_rps_mark_interactive(struct intel_rps *rps, bool interactive);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index 3a9750af0bdf..db418396a145 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -446,7 +446,11 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
*slpc, u32 val)
val > slpc->max_freq_softlimit)
return -EINVAL;
+ /* Need a lock now since waitboost can be modifying min as well */
+ mutex_lock(&slpc->lock);
+
with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
+
ret = slpc_set_param(slpc,
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
val);
@@ -459,6 +463,8 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
*slpc, u32 val)
if (!ret)
slpc->min_freq_softlimit = val;
+ mutex_unlock(&slpc->lock);
+
return ret;
}
@@ -644,6 +650,20 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
return 0;
}
+void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc)
+{
+ /*
+* Return min back to the softlimit.
+* This is called during request retire,
+* so we don't need to fail that if the
+* set_param fails.
+*/
+ mutex_lock(&slpc->lock);
+ if (atomic_dec_and_test(&slpc->num_waiters))
+ slpc_force_min_freq(slpc, slpc->min_freq_softlimit);
+ mutex_unlock(&slpc->lock);
+}
+
int intel_guc_slpc_print_info(struct intel_guc_slpc *slpc, struct drm_printer
*p)
{
struct drm_i915_private *i915 = slpc_to_i915(slpc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
index b62528647770..d74d6d749bdc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
@@ -39,5 +39,6 @@ int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc,
u32 *val);
int intel_guc_slpc_print_info(struct intel_guc_slpc *slpc, struct d
[PATCH 3/3] drm/i915/guc/slpc: Update boost sysfs hooks for SLPC
Add a helper to sort through the SLPC/RPS paths of get/set methods.
Boost frequency will be modified as long as it is within the constraints
of RP0 and if it is different from the existing one. We will set min
freq to boost only if there is at least one active waiter.
v2: Add num_boosts to guc_slpc_info and changes for worker function
v3: Review comments (Ashutosh)
Cc: Ashutosh Dixit
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/intel_rps.c | 47 +
drivers/gpu/drm/i915/gt/intel_rps.h | 2 +
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 29 +
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 1 +
drivers/gpu/drm/i915/i915_sysfs.c | 19 ++---
5 files changed, 82 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 6f310c9d9765..07ff7ba7b2b7 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -936,6 +936,53 @@ void intel_rps_park(struct intel_rps *rps)
GT_TRACE(rps_to_gt(rps), "park:%x\n", rps->cur_freq);
}
+u32 intel_rps_get_boost_frequency(struct intel_rps *rps)
+{
+ struct intel_guc_slpc *slpc;
+
+ if (rps_uses_slpc(rps)) {
+ slpc = rps_to_slpc(rps);
+
+ return slpc->boost_freq;
+ } else {
+ return intel_gpu_freq(rps, rps->boost_freq);
+ }
+}
+
+static int rps_set_boost_freq(struct intel_rps *rps, u32 val)
+{
+ bool boost = false;
+
+ /* Validate against (static) hardware limits */
+ val = intel_freq_opcode(rps, val);
+ if (val < rps->min_freq || val > rps->max_freq)
+ return -EINVAL;
+
+ mutex_lock(&rps->lock);
+ if (val != rps->boost_freq) {
+ rps->boost_freq = val;
+ boost = atomic_read(&rps->num_waiters);
+ }
+ mutex_unlock(&rps->lock);
+ if (boost)
+ schedule_work(&rps->work);
+
+ return 0;
+}
+
+int intel_rps_set_boost_frequency(struct intel_rps *rps, u32 freq)
+{
+ struct intel_guc_slpc *slpc;
+
+ if (rps_uses_slpc(rps)) {
+ slpc = rps_to_slpc(rps);
+
+ return intel_guc_slpc_set_boost_freq(slpc, freq);
+ } else {
+ return rps_set_boost_freq(rps, freq);
+ }
+}
+
void intel_rps_dec_waiters(struct intel_rps *rps)
{
struct intel_guc_slpc *slpc;
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index 407e878d5006..aee12f37d38a 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -24,6 +24,8 @@ void intel_rps_park(struct intel_rps *rps);
void intel_rps_unpark(struct intel_rps *rps);
void intel_rps_boost(struct i915_request *rq);
void intel_rps_dec_waiters(struct intel_rps *rps);
+u32 intel_rps_get_boost_frequency(struct intel_rps *rps);
+int intel_rps_set_boost_frequency(struct intel_rps *rps, u32 freq);
int intel_rps_set(struct intel_rps *rps, u8 val);
void intel_rps_mark_interactive(struct intel_rps *rps, bool interactive);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index db418396a145..4e1d3cd29164 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -650,6 +650,33 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
return 0;
}
+int intel_guc_slpc_set_boost_freq(struct intel_guc_slpc *slpc, u32 val)
+{
+ int ret = 0;
+
+ if (val < slpc->min_freq || val > slpc->rp0_freq)
+ return -EINVAL;
+
+ mutex_lock(&slpc->lock);
+
+ if (slpc->boost_freq != val) {
+ /* Apply only if there are active waiters */
+ if (atomic_read(&slpc->num_waiters)) {
+ ret = slpc_force_min_freq(slpc, val);
+ if (ret) {
+ ret = -EIO;
+ goto done;
+ }
+ }
+
+ slpc->boost_freq = val;
+ }
+
+done:
+ mutex_unlock(&slpc->lock);
+ return ret;
+}
+
void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc)
{
/*
@@ -687,6 +714,8 @@ int intel_guc_slpc_print_info(struct intel_guc_slpc *slpc,
struct drm_printer *p
slpc_decode_max_freq(slpc));
drm_printf(p, "\tMin freq: %u MHz\n",
slpc_decode_min_freq(slpc));
+ drm_printf(p, "\twaitboosts: %u\n",
+ slpc->num_boosts);
}
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
index d74d6d749bdc..0caa8fee3c04 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
@@ -34,6 +34,7 @@ int intel_guc_slpc_enable(stru
Re: [PATCH v3 0/3] drm/i915/guc/slpc: Implement waitboost for SLPC
On Mon, 01 Nov 2021 18:26:05 -0700, Vinay Belgaumkar wrote: > > Waitboost is a legacy feature implemented in the Host Turbo algorithm. This > patch set implements it for the SLPC path. A boost can happen when a request > is waiting for an unmet dependency. GT frequency gets temporarily bumped to > boost freq to allow the previous request to finish quickly. We achieve this > on SLPC by setting the min frequency, SLPC will set that as the requested > frequency. > > The boost will occur through a worker thread that will be scheduled > when the required conditions are met. > > Like before, boost frequency is configurable through sysfs, so we can > adjust it to any specific value as long as it is between [min, RP0]. > > v2: Add a worker thread to perform freq boost. > v3: Address comments (Ashutosh) For the series: Reviewed-by: Ashutosh Dixit > Cc: Ashutosh Dixit > Signed-off-by: Vinay Belgaumkar > > Vinay Belgaumkar (3): > drm/i915/guc/slpc: Define and initialize boost frequency > drm/i915/guc/slpc: Add waitboost functionality for SLPC > drm/i915/guc/slpc: Update boost sysfs hooks for SLPC > > drivers/gpu/drm/i915/gt/intel_rps.c | 72 + > drivers/gpu/drm/i915/gt/intel_rps.h | 3 + > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 151 +++--- > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 3 + > .../gpu/drm/i915/gt/uc/intel_guc_slpc_types.h | 13 ++ > drivers/gpu/drm/i915/i915_request.c | 2 +- > drivers/gpu/drm/i915/i915_sysfs.c | 19 +-- > 7 files changed, 223 insertions(+), 40 deletions(-) > > -- > 2.25.0 >
Re: [PATCH v7 3/3] drm/bridge: anx7625: config hs packets end aligned to avoid screen shift
On Mon, Nov 01, 2021 at 11:16:15AM +0800, Jitao Shi wrote:
> Hi Xin,
>
> Please help to review the changes in anx7625.c
>
> On Thu, 2021-09-16 at 06:31 +0800, Jitao Shi wrote:
> > This device requires the packets on lanes aligned at the end to fix
> > screen shift or scroll.
> >
> > Signed-off-by: Jitao Shi
> > ---
> > drivers/gpu/drm/bridge/analogix/anx7625.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index 14d73fb1dd15..d76fb63fa9f7 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > @@ -1327,6 +1327,7 @@ static int anx7625_attach_dsi(struct
> > anx7625_data *ctx)
> > MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> > MIPI_DSI_MODE_NO_EOT_PACKET |
> > MIPI_DSI_MODE_VIDEO_HSE;
> > + dsi->hs_packet_end_aligned = true;
Looks good, it's OK for me.
Reviewed-by: Xin Ji
> >
> > if (mipi_dsi_attach(dsi) < 0) {
> > DRM_DEV_ERROR(dev, "fail to attach dsi to host.\n");
RE: [PATCH v2] drm/dp: Fix aux->transfer NULL pointer dereference on drm_dp_dpcd_access
[AMD Official Use Only] Hi Jani: Thanks for your comments. > -Original Message- > From: Jani Nikula > Sent: Monday, November 1, 2021 9:07 PM > To: Yuan, Perry ; Maarten Lankhorst > ; Maxime Ripard ; > Thomas Zimmermann ; David Airlie ; > Daniel Vetter > Cc: Yuan, Perry ; dri-devel@lists.freedesktop.org; linux- > ker...@vger.kernel.org; Huang, Shimmer ; Huang, > Ray > Subject: Re: [PATCH v2] drm/dp: Fix aux->transfer NULL pointer dereference on > drm_dp_dpcd_access > > [CAUTION: External Email] > > On Mon, 01 Nov 2021, Perry Yuan wrote: > > Fix below crash by adding a check in the drm_dp_dpcd_access which > > ensures that aux->transfer was actually initialized earlier. > > Gut feeling says this is papering over a real usage issue somewhere else. Why > is > the aux being used for transfers before ->transfer has been set? Why should > the > dp helper be defensive against all kinds of misprogramming? > > > BR, > Jani. > The issue was found by Intel IGT test suite, graphic by pass test case. https://gitlab.freedesktop.org/drm/igt-gpu-tools normally use case will not see the issue. To avoid this issue happy again when we run the test case , it will be nice to add a check before the transfer is called. And we can see that it really needs to have a check here to make ITG &kernel happy. Perry. > > > > > BUG: kernel NULL pointer dereference, address: PGD 0 > > P4D 0 > > Oops: 0010 [#1] SMP NOPTI > > RIP: 0010:0x0 > > Code: Unable to access opcode bytes at RIP 0xffd6. > > RSP: 0018:a8d64225bab8 EFLAGS: 00010246 > > RAX: RBX: 0020 RCX: a8d64225bb5e > > RDX: 93151d921880 RSI: a8d64225bac8 RDI: 931511a1a9d8 > > RBP: a8d64225bb10 R08: 0001 R09: a8d64225ba60 > > R10: 0002 R11: 000d R12: 0001 > > R13: R14: a8d64225bb5e R15: 931511a1a9d8 > > FS: 7ff8ea7fa9c0() GS:9317fe6c() > > knlGS: > > CS: 0010 DS: ES: CR0: 80050033 > > CR2: ffd6 CR3: 00010d5a4000 CR4: 00750ee0 > > PKRU: 5554 > > Call Trace: > > drm_dp_dpcd_access+0x72/0x110 [drm_kms_helper] > > drm_dp_dpcd_read+0xb7/0xf0 [drm_kms_helper] > > drm_dp_start_crc+0x38/0xb0 [drm_kms_helper] > > amdgpu_dm_crtc_set_crc_source+0x1ae/0x3e0 [amdgpu] > > crtc_crc_open+0x174/0x220 [drm] > > full_proxy_open+0x168/0x1f0 > > ? open_proxy_open+0x100/0x100 > > do_dentry_open+0x156/0x370 > > vfs_open+0x2d/0x30 > > > > v2: fix some typo > > > > Signed-off-by: Perry Yuan > > --- > > drivers/gpu/drm/drm_dp_helper.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > > b/drivers/gpu/drm/drm_dp_helper.c index 6d0f2c447f3b..76b28396001a > > 100644 > > --- a/drivers/gpu/drm/drm_dp_helper.c > > +++ b/drivers/gpu/drm/drm_dp_helper.c > > @@ -260,6 +260,10 @@ static int drm_dp_dpcd_access(struct drm_dp_aux > *aux, u8 request, > > msg.buffer = buffer; > > msg.size = size; > > > > + /* No transfer function is set, so not an available DP connector */ > > + if (!aux->transfer) > > + return -EINVAL; > > + > > mutex_lock(&aux->hw_mutex); > > > > /* > > -- > Jani Nikula, Intel Open Source Graphics Center
[PATCH] drm/exynos: Replace legacy gpio interface for gpiod interface
Considering the current transition of the GPIO subsystem, remove all
dependencies of the legacy GPIO interface (linux/gpio.h and linux
/of_gpio.h) and replace it with the descriptor-based GPIO approach.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 42 +
1 file changed, 14 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 8d137857818c..b0b1acb7e712 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -13,7 +13,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -265,7 +264,7 @@ struct exynos_dsi {
struct clk **clks;
struct regulator_bulk_data supplies[2];
int irq;
- int te_gpio;
+ struct gpio_desc *te_gpio;
u32 pll_clk_rate;
u32 burst_clk_rate;
@@ -1298,14 +1297,14 @@ static void exynos_dsi_enable_irq(struct exynos_dsi
*dsi)
{
enable_irq(dsi->irq);
- if (gpio_is_valid(dsi->te_gpio))
- enable_irq(gpio_to_irq(dsi->te_gpio));
+ if (dsi->te_gpio)
+ enable_irq(gpiod_to_irq(dsi->te_gpio));
}
static void exynos_dsi_disable_irq(struct exynos_dsi *dsi)
{
- if (gpio_is_valid(dsi->te_gpio))
- disable_irq(gpio_to_irq(dsi->te_gpio));
+ if (dsi->te_gpio)
+ disable_irq(gpiod_to_irq(dsi->te_gpio));
disable_irq(dsi->irq);
}
@@ -1335,29 +1334,20 @@ static int exynos_dsi_register_te_irq(struct exynos_dsi
*dsi,
int ret;
int te_gpio_irq;
- dsi->te_gpio = of_get_named_gpio(panel->of_node, "te-gpios", 0);
- if (dsi->te_gpio == -ENOENT)
- return 0;
-
- if (!gpio_is_valid(dsi->te_gpio)) {
- ret = dsi->te_gpio;
- dev_err(dsi->dev, "cannot get te-gpios, %d\n", ret);
+ dsi->te_gpio = devm_gpiod_get_optional(dsi->dev, "te", GPIOD_IN);
+ if (IS_ERR(dsi->te_gpio)) {
+ dev_err(dsi->dev, "gpio request failed with %ld\n",
+ PTR_ERR(dsi->te_gpio));
goto out;
}
- ret = gpio_request(dsi->te_gpio, "te_gpio");
- if (ret) {
- dev_err(dsi->dev, "gpio request failed with %d\n", ret);
- goto out;
- }
-
- te_gpio_irq = gpio_to_irq(dsi->te_gpio);
+ te_gpio_irq = gpiod_to_irq(dsi->te_gpio);
ret = request_threaded_irq(te_gpio_irq, exynos_dsi_te_irq_handler, NULL,
IRQF_TRIGGER_RISING | IRQF_NO_AUTOEN, "TE",
dsi);
if (ret) {
dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
- gpio_free(dsi->te_gpio);
+ gpiod_put(dsi->te_gpio);
goto out;
}
@@ -1367,10 +1357,9 @@ static int exynos_dsi_register_te_irq(struct exynos_dsi
*dsi,
static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
{
- if (gpio_is_valid(dsi->te_gpio)) {
- free_irq(gpio_to_irq(dsi->te_gpio), dsi);
- gpio_free(dsi->te_gpio);
- dsi->te_gpio = -ENOENT;
+ if (dsi->te_gpio) {
+ free_irq(gpiod_to_irq(dsi->te_gpio), dsi);
+ gpiod_put(dsi->te_gpio);
}
}
@@ -1745,9 +1734,6 @@ static int exynos_dsi_probe(struct platform_device *pdev)
if (!dsi)
return -ENOMEM;
- /* To be checked as invalid one */
- dsi->te_gpio = -ENOENT;
-
init_completion(&dsi->completed);
spin_lock_init(&dsi->transfer_lock);
INIT_LIST_HEAD(&dsi->transfer_list);
--
2.31.1
Re: [PATCH] drm: Grab reference of connector before return connector from sun4i_tcon_get_connector
在 2021/11/1 21:02, Jani Nikula 写道: On Mon, 01 Nov 2021, He Ying wrote: From the comments of drm_for_each_connector_iter(), we know that "connector is only valid within the list body, if you want to use connector after calling drm_connector_list_iter_end() then you need to grab your own reference first using drm_connector_get()". So fix the wrong use of connector according to the comments and then call drm_connector_put() after using connector finishes. Signed-off-by: He Ying Please use "drm/sun4i:" subject prefix for sun4i patches. OK. I'll send a V2 later. By the way, may I have your suggestions about this patch's content.
