Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

16 participants
3178 discussions

[PATCH v2 0/6] DRM driver for verisilicon

by Keith Zhao

This patch is a drm driver for Starfive Soc JH7110, I am sending Drm driver part and HDMI driver part. We used GEM framework for buffer management , and for buffer allocation,we use DMA APIs. the Starfive HDMI servers as interface between a LCD Controller and a HDMI bus. A HDMI TX consists of one HDMI transmitter controller and one HDMI transmitter PHY. (Sound support is not include in this patch) This patchset should be applied after the patchset: https://patchwork.kernel.org/project/linux-clk/cover/20230713113902.56519-1… V1: Changes since v1: - Further standardize the yaml file. - Dts naming convention improved. - Fix the problem of compiling and loading ko files. - Use drm new api to automatically manage resources. - Drop struct vs_crtc_funcs&vs_plane_funcs��subdivide the plane's help interface - Reduce the modifiers unused. - Optimize the hdmi driver code V2: Changes since v2: - fix the error about checking the yaml file. - match drm driver GEM DMA API. - Delete the custom crtc property . - hdmi use drmm_ new api to automatically manage resources. - update the modifiers comments. - enabling KASAN, fix the error during removing module Keith Zhao (6): dt-bindings: display: Add yamls for JH7110 display system riscv: dts: starfive: jh7110: add dc controller and hdmi node drm/fourcc: Add drm/vs tiled modifiers drm/vs: Register DRM device drm/vs: Add KMS crtc&plane drm/vs: Add hdmi driver .../starfive/starfive,display-subsystem.yaml | 41 + .../starfive/starfive,jh7110-dc8200.yaml | 109 + .../starfive/starfive,jh7110-inno-hdmi.yaml | 85 + MAINTAINERS | 7 + .../jh7110-starfive-visionfive-2.dtsi | 91 + arch/riscv/boot/dts/starfive/jh7110.dtsi | 41 + drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/verisilicon/Kconfig | 21 + drivers/gpu/drm/verisilicon/Makefile | 12 + drivers/gpu/drm/verisilicon/starfive_hdmi.c | 949 ++++++++ drivers/gpu/drm/verisilicon/starfive_hdmi.h | 295 +++ drivers/gpu/drm/verisilicon/vs_crtc.c | 257 +++ drivers/gpu/drm/verisilicon/vs_crtc.h | 43 + drivers/gpu/drm/verisilicon/vs_dc.c | 1002 +++++++++ drivers/gpu/drm/verisilicon/vs_dc.h | 80 + drivers/gpu/drm/verisilicon/vs_dc_hw.c | 1959 +++++++++++++++++ drivers/gpu/drm/verisilicon/vs_dc_hw.h | 492 +++++ drivers/gpu/drm/verisilicon/vs_drv.c | 234 ++ drivers/gpu/drm/verisilicon/vs_drv.h | 31 + drivers/gpu/drm/verisilicon/vs_modeset.c | 36 + drivers/gpu/drm/verisilicon/vs_modeset.h | 10 + drivers/gpu/drm/verisilicon/vs_plane.c | 526 +++++ drivers/gpu/drm/verisilicon/vs_plane.h | 58 + drivers/gpu/drm/verisilicon/vs_type.h | 69 + include/uapi/drm/drm_fourcc.h | 57 + include/uapi/drm/vs_drm.h | 50 + 27 files changed, 6558 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml create mode 100644 drivers/gpu/drm/verisilicon/Kconfig create mode 100644 drivers/gpu/drm/verisilicon/Makefile create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.c create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.h create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.c create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.h create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.c create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.h create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h create mode 100644 drivers/gpu/drm/verisilicon/vs_modeset.c create mode 100644 drivers/gpu/drm/verisilicon/vs_modeset.h create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.c create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.h create mode 100644 drivers/gpu/drm/verisilicon/vs_type.h create mode 100644 include/uapi/drm/vs_drm.h -- 2.34.1

1 year, 11 months

Re: [PATCH v4] drm/test: add a test suite for GEM objects backed by shmem

by Maxime Ripard

On Fri, Nov 24, 2023 at 11:15:12AM +0100, Marco Pagani wrote: > > > On 2023-11-24 09:49, Maxime Ripard wrote: > > Hi, > > > > On Thu, Nov 23, 2023 at 11:01:46AM +0100, Marco Pagani wrote: > >> +static int drm_gem_shmem_test_init(struct kunit *test) > >> +{ > >> + struct device *dev; > >> + struct fake_dev { > >> + struct drm_device drm_dev; > >> + } *fdev; > >> + > > > > [...] > > > >> + > >> + /* > >> + * The DRM core will automatically initialize the GEM core and create > >> + * a DRM Memory Manager object which provides an address space pool > >> + * for GEM objects allocation. > >> + */ > >> + fdev = drm_kunit_helper_alloc_drm_device(test, dev, struct fake_dev, > >> + drm_dev, DRIVER_GEM); > >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdev); > > > > Sorry I missed it earlier, but you don't need the intermediate structure > > if you use > > > > struct drm_device *drm; > > > > drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_GEM); > > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm); > > > > I prefer to use drm_kunit_helper_alloc_drm_device() with the intermediate > structure. It makes the code clearer, in my opinion. Initially, when > developing the suite, I was using __drm_kunit_helper_alloc_drm_device() > as most test suites do, but I feel the list of arguments including > "sizeof(*drm), 0," is less straightforward to understand. Then we can create an init helper, and you can allocate the drm_device through drmm_kzalloc, but I'd like tests to use consistent constructs. This can of course be made as a later patch: you use the same construct the other tests do here, and later we work on the init function and convert all tests to use it. Maxime

1 year, 11 months

[PATCH] dma-buf: fix check in dma_resv_add_fence

by Christian König

It's valid to add the same fence multiple times to a dma-resv object and we shouldn't need one extra slot for each. Signed-off-by: Christian König <christian.koenig(a)amd.com> Fixes: a3f7c10a269d5 ("dma-buf/dma-resv: check if the new fence is really later") Cc: stable(a)vger.kernel.org # v5.19+ --- drivers/dma-buf/dma-resv.c | 2 +- include/linux/dma-fence.h | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 38b4110378de..eb8b733065b2 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -301,7 +301,7 @@ void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence, dma_resv_list_entry(fobj, i, obj, &old, &old_usage); if ((old->context == fence->context && old_usage >= usage && - dma_fence_is_later(fence, old)) || + dma_fence_is_later_or_same(fence, old)) || dma_fence_is_signaled(old)) { dma_resv_list_set(fobj, i, fence, usage); dma_fence_put(old); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index ebe78bd3d121..b3772edca2e6 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -498,6 +498,21 @@ static inline bool dma_fence_is_later(struct dma_fence *f1, return __dma_fence_is_later(f1->seqno, f2->seqno, f1->ops); } +/** + * dma_fence_is_later_or_same - return true if f1 is later or same as f2 + * @f1: the first fence from the same context + * @f2: the second fence from the same context + * + * Returns true if f1 is chronologically later than f2 or the same fence. Both + * fences must be from the same context, since a seqno is not re-used across + * contexts. + */ +static inline bool dma_fence_is_later_or_same(struct dma_fence *f1, + struct dma_fence *f2) +{ + return f1 == f2 || dma_fence_is_later(f1, f2); +} + /** * dma_fence_later - return the chronologically later fence * @f1: the first fence from the same context -- 2.34.1

1 year, 11 months

Re: [PATCH v4] drm/test: add a test suite for GEM objects backed by shmem

by Maxime Ripard

Hi, On Thu, Nov 23, 2023 at 11:01:46AM +0100, Marco Pagani wrote: > +static int drm_gem_shmem_test_init(struct kunit *test) > +{ > + struct device *dev; > + struct fake_dev { > + struct drm_device drm_dev; > + } *fdev; > + [...] > + > + /* > + * The DRM core will automatically initialize the GEM core and create > + * a DRM Memory Manager object which provides an address space pool > + * for GEM objects allocation. > + */ > + fdev = drm_kunit_helper_alloc_drm_device(test, dev, struct fake_dev, > + drm_dev, DRIVER_GEM); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdev); Sorry I missed it earlier, but you don't need the intermediate structure if you use struct drm_device *drm; drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_GEM); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm); Maxime

1 year, 12 months

Re: [PATCH v3] drm/test: add a test suite for GEM objects backed by shmem

by kernel test robot

Hi Marco, kernel test robot noticed the following build warnings: [auto build test WARNING on c79b972eb88b077d2765e7790d0902b3dc94d55c] url: https://github.com/intel-lab-lkp/linux/commits/Marco-Pagani/drm-test-add-a-… base: c79b972eb88b077d2765e7790d0902b3dc94d55c patch link: https://lore.kernel.org/r/20231120150339.104257-1-marpagan%40redhat.com patch subject: [PATCH v3] drm/test: add a test suite for GEM objects backed by shmem config: arm-kismet-CONFIG_DRM_GEM_SHMEM_HELPER-CONFIG_DRM_KUNIT_TEST-0-0 (https://download.01.org/0day-ci/archive/20231121/202311211542.3ZrZO6J2-lkp@…) reproduce: (https://download.01.org/0day-ci/archive/20231121/202311211542.3ZrZO6J2-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202311211542.3ZrZO6J2-lkp@intel.com/ kismet warnings: (new ones prefixed by >>) >> kismet: WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER when selected by DRM_KUNIT_TEST WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && MMU [=n] Selected by [y]: - DRM_KUNIT_TEST [=y] && HAS_IOMEM [=y] && DRM [=y] && KUNIT [=y] -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

2 years

[PATCH v6] drm/mediatek: add dma buffer control for drm plane disable

by Yongqiang Niu

the DMA buffer is released when still accessed by Mediatek display hardware, this flow can lead to M4U reporting a translation fault warning add dma buffer control flow in mediatek driver: get dma buffer when drm plane disable put dma buffer when overlay really disable Fixes: 41016fe1028e ("drm: Rename plane->state variables in atomic update and disable") Signed-off-by: Yongqiang Niu <yongqiang.niu(a)mediatek.com> --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 25 ++++++++++++++++++++++++ drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 + drivers/gpu/drm/mediatek/mtk_drm_plane.c | 12 ++++++++++++ drivers/gpu/drm/mediatek/mtk_drm_plane.h | 1 + 4 files changed, 39 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index c277b9fae950..188aaa97e5e3 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -4,6 +4,7 @@ */ #include <linux/clk.h> +#include <linux/dma-buf.h> #include <linux/dma-mapping.h> #include <linux/mailbox_controller.h> #include <linux/of.h> @@ -283,6 +284,23 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc, return NULL; } +static void mtk_drm_dma_buf_put(struct mtk_drm_crtc *mtk_crtc) +{ + unsigned int i; + + for (i = 0; i < mtk_crtc->layer_nr; i++) { + struct drm_plane *plane = &mtk_crtc->planes[i]; + struct mtk_plane_state *plane_state; + + plane_state = to_mtk_plane_state(plane->state); + + if (plane_state && plane_state->pending.dma_buf) { + dma_buf_put(plane_state->pending.dma_buf); + plane_state->pending.dma_buf = NULL; + } + } +} + #if IS_REACHABLE(CONFIG_MTK_CMDQ) static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg) { @@ -323,6 +341,8 @@ static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg) mtk_crtc->pending_async_planes = false; } + mtk_drm_dma_buf_put(mtk_crtc); + mtk_crtc->cmdq_vblank_cnt = 0; wake_up(&mtk_crtc->cb_blocking_queue); } @@ -627,9 +647,14 @@ static void mtk_crtc_ddp_irq(void *data) else if (mtk_crtc->cmdq_vblank_cnt > 0 && --mtk_crtc->cmdq_vblank_cnt == 0) DRM_ERROR("mtk_crtc %d CMDQ execute command timeout!\n", drm_crtc_index(&mtk_crtc->base)); + + if (!mtk_crtc->cmdq_client.chan) + mtk_drm_dma_buf_put(mtk_crtc); #else if (!priv->data->shadow_register) mtk_crtc_ddp_config(crtc, NULL); + + mtk_drm_dma_buf_put(mtk_crtc); #endif mtk_drm_finish_page_flip(mtk_crtc); } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 2dfaa613276a..4fd232644383 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -1019,4 +1019,5 @@ module_exit(mtk_drm_exit); MODULE_AUTHOR("YT SHEN <yt.shen(a)mediatek.com>"); MODULE_DESCRIPTION("Mediatek SoC DRM driver"); +MODULE_IMPORT_NS(DMA_BUF); MODULE_LICENSE("GPL v2"); diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index ddc9355b06d5..fbfcd0d1a56c 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -12,6 +12,7 @@ #include <drm/drm_framebuffer.h> #include <drm/drm_gem_atomic_helper.h> #include <linux/align.h> +#include <linux/dma-buf.h> #include "mtk_drm_crtc.h" #include "mtk_drm_ddp_comp.h" @@ -283,6 +284,17 @@ static void mtk_plane_atomic_disable(struct drm_plane *plane, struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane); struct mtk_plane_state *mtk_plane_state = to_mtk_plane_state(new_state); + struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state, + plane); + + if (old_state && old_state->fb) { + struct drm_gem_object *gem = old_state->fb->obj[0]; + + if (gem && gem->dma_buf) { + get_dma_buf(gem->dma_buf); + mtk_plane_state->pending.dma_buf = gem->dma_buf; + } + } mtk_plane_state->pending.enable = false; wmb(); /* Make sure the above parameter is set before update */ mtk_plane_state->pending.dirty = true; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h index 99aff7da0831..3aba0b58ef3c 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h @@ -33,6 +33,7 @@ struct mtk_plane_pending_state { bool async_dirty; bool async_config; enum drm_color_encoding color_encoding; + struct dma_buf *dma_buf; }; struct mtk_plane_state { -- 2.25.1

2 years

[PATCH] dma-buf: Replace strlcpy() with strscpy()

by Kees Cook

strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated[1]. Additionally, it returns the size of the source string, not the resulting size of the destination string. In an effort to remove strlcpy() completely[2], replace strlcpy() here with strscpy(). Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [1] Link: https://github.com/KSPP/linux/issues/89 [2] Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: Azeem Shaikh <azeemshaikh38(a)gmail.com> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Signed-off-by: Kees Cook <keescook(a)chromium.org> --- drivers/dma-buf/dma-buf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 21916bba77d5..8fe5aa67b167 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -46,12 +46,12 @@ static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen) { struct dma_buf *dmabuf; char name[DMA_BUF_NAME_LEN]; - size_t ret = 0; + ssize_t ret = 0; dmabuf = dentry->d_fsdata; spin_lock(&dmabuf->name_lock); if (dmabuf->name) - ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN); + ret = strscpy(name, dmabuf->name, sizeof(name)); spin_unlock(&dmabuf->name_lock); return dynamic_dname(buffer, buflen, "/%s:%s", -- 2.34.1

2 years

[PATCH v2 0/8] dma-buf: heaps: Add secure heap

by Yong Wu

This patchset adds three secure heaps: 1) secure_mtk_cm: secure chunk memory for MediaTek SVP (Secure Video Path). The buffer is reserved for the secure world after bootup and it is used for vcodec's ES/working buffer; 2) secure_mtk_cma: secure CMA memory for MediaTek SVP. This buffer is dynamically reserved for the secure world and will be got when we start playing secure videos, Once the security video playing is complete, the CMA will be released. This heap is used for the vcodec's frame buffer. 3) secure_cma: Use the kerne CMA ops as the allocation ops. currently it is a draft version for Vijay and Jaskaran. For the first two MediaTek heaps will be used v4l2[1] and drm[2], thus we cannot put it in v4l2 or drm, and create a common heap for them. Meanwhile We have a limited number of hardware entries to protect memory, we cannot protect memory arbitrarily, thus the secure memory management is actually inside OPTEE. The kernel just tells the TEE what size I want and the TEE will return a "secure handle". [1] https://lore.kernel.org/linux-mediatek/20231106120423.23364-1-yunfei.dong@m… [2] https://lore.kernel.org/linux-mediatek/20231023044549.21412-1-jason-jh.lin@… Change note: v2: 1) Move John's patches into the vcodec patchset since they use the new dma heap interface directly. https://lore.kernel.org/linux-mediatek/20231106120423.23364-1-yunfei.dong@m… 2) Reword the dt-binding description. 3) Rename the heap name from mtk_svp to secure_mtk_cm. This means the current vcodec/DRM upstream code doesn't match this. 4) Add a normal CMA heap. currently it should be a draft version. 5) Regarding the UUID, I still use hard code, but put it in a private data which allow the others could set their own UUID. What's more, UUID is necessary for the session with TEE. If we don't have it, we can't communicate with the TEE, including the get_uuid interface, which tries to make uuid more generic, not working. If there is other way to make UUID more general, please free to tell me. v1: https://lore.kernel.org/linux-mediatek/20230911023038.30649-1-yong.wu@media… Base on v6.6-rc1. Yong Wu (8): dma-buf: heaps: Initialize a secure heap dma-buf: heaps: secure_heap: Add private heap ops dma-buf: heaps: secure_heap: Initialize tee session dma-buf: heaps: secure_heap: Add tee memory service call dma-buf: heaps: secure_heap: Add dma_ops dt-bindings: reserved-memory: Add secure CMA reserved memory range dma_buf: heaps: secure_heap: Add a new MediaTek CMA heap dma-buf: heaps: secure_heap: Add normal CMA heap .../reserved-memory/secure_cma_region.yaml | 44 ++ drivers/dma-buf/heaps/Kconfig | 7 + drivers/dma-buf/heaps/Makefile | 1 + drivers/dma-buf/heaps/secure_heap.c | 602 ++++++++++++++++++ 4 files changed, 654 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml create mode 100644 drivers/dma-buf/heaps/secure_heap.c -- 2.25.1

2 years

Re: [PATCH v2 0/8] dma-buf: heaps: Add secure heap

by Jeffrey Kardatzke

The main goal is for secure video playback, and to also enable other potential uses of this in the future. The 'secure dma-heap' will be used to allocate dma_buf objects that reference memory in the secure world that is inaccessible/unmappable by the non-secure (i.e. kernel/userspace) world. That memory will be used by the secure world to store secure information (i.e. decrypted media content). The dma_bufs allocated from the kernel will be passed to V4L2 for video decoding (as input and output). They will also be used by the drm system for rendering of the content. Hope that helps. Cheers, Jeff On Mon, Nov 13, 2023 at 3:38 AM Pavel Machek <pavel(a)ucw.cz> wrote: > > Hi! > > > This patchset adds three secure heaps: > > 1) secure_mtk_cm: secure chunk memory for MediaTek SVP (Secure Video Path). > > The buffer is reserved for the secure world after bootup and it is used > > for vcodec's ES/working buffer; > > 2) secure_mtk_cma: secure CMA memory for MediaTek SVP. This buffer is > > dynamically reserved for the secure world and will be got when we start > > playing secure videos, Once the security video playing is complete, the > > CMA will be released. This heap is used for the vcodec's frame buffer. > > 3) secure_cma: Use the kerne CMA ops as the allocation ops. > > currently it is a draft version for Vijay and Jaskaran. > > Is there high-level description of what the security goals here are, > somewhere? > > BR, > Pavel > -- > People of Russia, stop Putin before his war on Ukraine escalates.

2 years

Re: [RFC PATCH v2] drm/test: add a test suite for GEM objects backed by shmem

by Maxime Ripard

Hi, On Tue, Nov 14, 2023 at 05:18:08PM +0100, Marco Pagani wrote: > On 2023-11-10 15:41, Maxime Ripard wrote: > > On Wed, Nov 08, 2023 at 02:42:03PM +0100, Marco Pagani wrote: > >> This patch introduces an initial KUnit test suite for GEM objects > >> backed by shmem buffers. > >> > >> Suggested-by: Javier Martinez Canillas <javierm(a)redhat.com> > >> Signed-off-by: Marco Pagani <marpagan(a)redhat.com> > >> > >> v2: > >> - Improved description of test cases > >> - Cleaner error handling using KUnit actions > >> - Alphabetical order in Kconfig and Makefile > >> --- > >> drivers/gpu/drm/Kconfig | 9 +- > >> drivers/gpu/drm/tests/Makefile | 5 +- > >> drivers/gpu/drm/tests/drm_gem_shmem_test.c | 381 +++++++++++++++++++++ > >> 3 files changed, 389 insertions(+), 6 deletions(-) > >> create mode 100644 drivers/gpu/drm/tests/drm_gem_shmem_test.c > >> > >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > >> index 3eee8636f847..a2551c8c393a 100644 > >> --- a/drivers/gpu/drm/Kconfig > >> +++ b/drivers/gpu/drm/Kconfig > >> @@ -76,14 +76,15 @@ config DRM_KUNIT_TEST > >> tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS > >> depends on DRM && KUNIT > >> select PRIME_NUMBERS > >> + select DRM_BUDDY > >> select DRM_DISPLAY_DP_HELPER > >> select DRM_DISPLAY_HELPER > >> - select DRM_LIB_RANDOM > >> - select DRM_KMS_HELPER > >> - select DRM_BUDDY > >> + select DRM_EXEC > >> select DRM_EXPORT_FOR_TESTS if m > >> + select DRM_GEM_SHMEM_HELPER > >> + select DRM_KMS_HELPER > >> select DRM_KUNIT_TEST_HELPERS > >> - select DRM_EXEC > >> + select DRM_LIB_RANDOM > >> default KUNIT_ALL_TESTS > >> help > >> This builds unit tests for DRM. This option is not useful for > >> diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile > >> index ba7baa622675..d6183b3d7688 100644 > >> --- a/drivers/gpu/drm/tests/Makefile > >> +++ b/drivers/gpu/drm/tests/Makefile > >> @@ -9,15 +9,16 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \ > >> drm_connector_test.o \ > >> drm_damage_helper_test.o \ > >> drm_dp_mst_helper_test.o \ > >> + drm_exec_test.o \ > >> drm_format_helper_test.o \ > >> drm_format_test.o \ > >> drm_framebuffer_test.o \ > >> + drm_gem_shmem_test.o \ > >> drm_managed_test.o \ > >> drm_mm_test.o \ > >> drm_modes_test.o \ > >> drm_plane_helper_test.o \ > >> drm_probe_helper_test.o \ > >> - drm_rect_test.o \ > >> - drm_exec_test.o > >> + drm_rect_test.o > > > > Thanks for reordering the tests and symbols, but they should part of a > > preliminary patch. > > > > Okay, I'll send it as a separate patch before v3. Thanks for taking care of this. [...] > >> +/* > >> + * Wrappers to avoid an explicit type casting when passing action > >> + * functions to kunit_add_action(). > >> + */ > >> +static void kfree_wrapper(void *p) > >> +{ > >> + kfree(p); > >> +} > >> + > >> +static void sg_free_table_wrapper(void *sgt) > >> +{ > >> + sg_free_table(sgt); > >> +} > >> + > >> +static void drm_gem_shmem_free_wrapper(void *shmem) > >> +{ > >> + drm_gem_shmem_free(shmem); > >> +} > > > > I think you need to explicitly cast the pointer (or do a temporary > > assignment to the proper type) to avoid a compiler warning. > > > > Do you mean like: > > static void drm_gem_shmem_free_wrapper(void *shmem) > { > drm_gem_shmem_free((struct drm_gem_shmem_object *)shmem); > } yeah, or static void drm_gem_shmem_free_wrapper(void *ptr) { struct drm_gem_shmem_object *shmem = ptr; drm_gem_shmem_free(shmem); } > I built the current version with clang 16.0.6 and gcc 13.2.1 but got > no cast warnings. Clang spotted an uninitialized variable, though. The same thing happened to me, gcc didn't spot those issues but Intel's build bot did. They might run with extra warnings. Maxime

2 years

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig