dri-devel July 2021

dri-devel@lists.freedesktop.org

321 participants
829 discussions

nouveau: failed to initialise sync
by Corentin Labbe 14 Jul '21

14 Jul '21

Hello Since some days on next, nouveau fail to load: [ 2.754087] nouveau 0000:02:00.0: vgaarb: deactivate vga console [ 2.761260] Console: switching to colour dummy device 80x25 [ 2.766888] nouveau 0000:02:00.0: NVIDIA MCP77/MCP78 (0aa480a2) [ 2.783954] nouveau 0000:02:00.0: bios: version 62.77.2a.00.04 [ 2.810122] nouveau 0000:02:00.0: fb: 256 MiB stolen system memory [ 3.484031] nouveau 0000:02:00.0: DRM: VRAM: 256 MiB [ 3.488993] nouveau 0000:02:00.0: DRM: GART: 1048576 MiB [ 3.494308] nouveau 0000:02:00.0: DRM: TMDS table version 2.0 [ 3.500052] nouveau 0000:02:00.0: DRM: DCB version 4.0 [ 3.505192] nouveau 0000:02:00.0: DRM: DCB outp 00: 01000300 0000001e [ 3.511632] nouveau 0000:02:00.0: DRM: DCB outp 01: 01011332 00020010 [ 3.518074] nouveau 0000:02:00.0: DRM: DCB conn 00: 00000100 [ 3.523728] nouveau 0000:02:00.0: DRM: DCB conn 01: 00001261 [ 3.529455] nouveau 0000:02:00.0: DRM: failed to initialise sync subsystem, -28 [ 3.545946] nouveau: probe of 0000:02:00.0 failed with error -28 I bisected it to: git bisect start # good: [62fb9874f5da54fdb243003b386128037319b219] Linux 5.13 git bisect good 62fb9874f5da54fdb243003b386128037319b219 # bad: [fb0ca446157a86b75502c1636b0d81e642fe6bf1] Add linux-next specific files for 20210701 git bisect bad fb0ca446157a86b75502c1636b0d81e642fe6bf1 # good: [f63c4fda987a19b1194cc45cb72fd5bf968d9d90] Merge remote-tracking branch 'rdma/for-next' git bisect good f63c4fda987a19b1194cc45cb72fd5bf968d9d90 # bad: [49c8769be0b910d4134eba07cae5d9c71b861c4a] Merge remote-tracking branch 'drm/drm-next' git bisect bad 49c8769be0b910d4134eba07cae5d9c71b861c4a # good: [4e3db44a242a4e2afe33b59793898ecbb61d478e] Merge tag 'wireless-drivers-next-2021-06-25' of git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next git bisect good 4e3db44a242a4e2afe33b59793898ecbb61d478e # bad: [5745d647d5563d3e9d32013ad4e5c629acff04d7] Merge tag 'amd-drm-next-5.14-2021-06-02' of https://gitlab.freedesktop.org/agd5f/linux into drm-next git bisect bad 5745d647d5563d3e9d32013ad4e5c629acff04d7 # bad: [c99c4d0ca57c978dcc2a2f41ab8449684ea154cc] Merge tag 'amd-drm-next-5.14-2021-05-19' of https://gitlab.freedesktop.org/agd5f/linux into drm-next git bisect bad c99c4d0ca57c978dcc2a2f41ab8449684ea154cc # bad: [ae25ec2fc6c5a9e5767bf1922cd648501d0f914c] Merge tag 'drm-misc-next-2021-05-17' of git://anongit.freedesktop.org/drm/drm-misc into drm-next git bisect bad ae25ec2fc6c5a9e5767bf1922cd648501d0f914c # bad: [cac80e71cfb0b00202d743c6e90333c45ba77cc5] drm/vkms: rename cursor to plane on ops of planes composition git bisect bad cac80e71cfb0b00202d743c6e90333c45ba77cc5 # good: [178bdba84c5f0ad14de384fc7f15fba0e272919d] drm/ttm/ttm_device: Demote kernel-doc abuses git bisect good 178bdba84c5f0ad14de384fc7f15fba0e272919d # bad: [3f3a6524f6065fd3d130515e012f63eac74d96da] drm/dp: Clarify DP AUX registration time git bisect bad 3f3a6524f6065fd3d130515e012f63eac74d96da # bad: [6dd7efc437611db16d432e0030f72d0c7e890127] drm/gud: cleanup coding style a bit git bisect bad 6dd7efc437611db16d432e0030f72d0c7e890127 # bad: [13b29cc3a722c2c0bc9ab9f72f9047d55d08a2f9] drm/mxsfb: Don't select DRM_KMS_FB_HELPER git bisect bad 13b29cc3a722c2c0bc9ab9f72f9047d55d08a2f9 # bad: [d02117f8efaa5fbc37437df1ae955a147a2a424a] drm/ttm: remove special handling for non GEM drivers git bisect bad d02117f8efaa5fbc37437df1ae955a147a2a424a # good: [13ea9aa1e7d891e950230e82f1dd2c84e5debcff] drm/ttm: fix error handling if no BO can be swapped out v4 git bisect good 13ea9aa1e7d891e950230e82f1dd2c84e5debcff # first bad commit: [d02117f8efaa5fbc37437df1ae955a147a2a424a] drm/ttm: remove special handling for non GEM drivers Reverting the patch permit to have nouveau works again. Regards

5 6

[PATCH] drm: mxsfb: Clear FIFO_CLEAR bit
by Marek Vasut 14 Jul '21

14 Jul '21

Make sure the FIFO_CLEAR bit is latched in when configuring the controller, so that the FIFO is really cleared. And then clear the FIFO_CLEAR bit, since it is not self-clearing. Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller") Signed-off-by: Marek Vasut <marex(a)denx.de> Cc: Daniel Abrecht <public(a)danielabrecht.ch> Cc: Emil Velikov <emil.l.velikov(a)gmail.com> Cc: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com> Cc: Lucas Stach <l.stach(a)pengutronix.de> Cc: Stefan Agner <stefan(a)agner.ch> --- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index 98d8ba0bae84..22cb749fc9bc 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -241,6 +241,9 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb, /* Clear the FIFOs */ writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET); + readl(mxsfb->base + LCDC_CTRL1); + writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_CLR); + readl(mxsfb->base + LCDC_CTRL1); if (mxsfb->devdata->has_overlay) writel(0, mxsfb->base + LCDC_AS_CTRL); -- 2.30.2

3 12

[PATCH v2 01/13] drm/mgag200: Select clock in PLL update functions
by Thomas Zimmermann 14 Jul '21

14 Jul '21

Put the clock-selection code into each of the PLL-update functions to make them select the correct pixel clock. Instead of copying the code, introduce a new helper WREG_MISC_MASKED, which does masked writes into <MISC>. Use it from each individual PLL update function. The pixel clock for video output was not actually set before programming the clock's values. It worked because the device had the correct clock pre-set. v2: * don't duplicate <MISC> update code (Sam) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: db05f8d3dc87 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") Cc: Sam Ravnborg <sam(a)ravnborg.org> Cc: Emil Velikov <emil.velikov(a)collabora.com> Cc: Dave Airlie <airlied(a)redhat.com> Cc: dri-devel(a)lists.freedesktop.org Cc: <stable(a)vger.kernel.org> # v5.9+ --- drivers/gpu/drm/mgag200/mgag200_drv.h | 16 ++++++++++++++++ drivers/gpu/drm/mgag200/mgag200_mode.c | 20 +++++++++++++------- drivers/gpu/drm/mgag200/mgag200_reg.h | 9 ++++----- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index f7a0537c0d0a..5302d6566d7c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -43,6 +43,22 @@ #define ATTR_INDEX 0x1fc0 #define ATTR_DATA 0x1fc1 +#define WREG_MISC(v) \ + WREG8(MGA_MISC_OUT, v) + +#define RREG_MISC(v) \ + ((v) = RREG8(MGA_MISC_IN)) + +#define WREG_MISC_MASKED(v, mask) \ + do { \ + u8 misc_; \ + u8 mask_ = (mask); \ + RREG_MISC(misc_); \ + misc_ &= ~mask_; \ + misc_ |= ((v) & mask_); \ + WREG_MISC(misc_); \ + } while (0) + #define WREG_ATTR(reg, v) \ do { \ RREG8(0x1fda); \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 3b3059f471c2..1bdf21474bcb 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -174,6 +174,8 @@ static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) drm_dbg_kms(dev, "clock: %ld vco: %ld m: %d n: %d p: %d s: %d\n", clock, f_vco, m, n, p, s); + WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK); + WREG_DAC(MGA1064_PIX_PLLC_M, m); WREG_DAC(MGA1064_PIX_PLLC_N, n); WREG_DAC(MGA1064_PIX_PLLC_P, (p | (s << 3))); @@ -289,6 +291,8 @@ static int mga_g200se_set_plls(struct mga_device *mdev, long clock) return 1; } + WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK); + WREG_DAC(MGA1064_PIX_PLLC_M, m); WREG_DAC(MGA1064_PIX_PLLC_N, n); WREG_DAC(MGA1064_PIX_PLLC_P, p); @@ -385,6 +389,8 @@ static int mga_g200wb_set_plls(struct mga_device *mdev, long clock) } } + WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK); + for (i = 0; i <= 32 && pll_locked == false; i++) { if (i > 0) { WREG8(MGAREG_CRTC_INDEX, 0x1e); @@ -522,6 +528,8 @@ static int mga_g200ev_set_plls(struct mga_device *mdev, long clock) } } + WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK); + WREG8(DAC_INDEX, MGA1064_PIX_CLK_CTL); tmp = RREG8(DAC_DATA); tmp |= MGA1064_PIX_CLK_CTL_CLK_DIS; @@ -654,6 +662,9 @@ static int mga_g200eh_set_plls(struct mga_device *mdev, long clock) } } } + + WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK); + for (i = 0; i <= 32 && pll_locked == false; i++) { WREG8(DAC_INDEX, MGA1064_PIX_CLK_CTL); tmp = RREG8(DAC_DATA); @@ -754,6 +765,8 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock) } } + WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK); + WREG8(DAC_INDEX, MGA1064_PIX_CLK_CTL); tmp = RREG8(DAC_DATA); tmp |= MGA1064_PIX_CLK_CTL_CLK_DIS; @@ -787,8 +800,6 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock) static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) { - u8 misc; - switch(mdev->type) { case G200_PCI: case G200_AGP: @@ -808,11 +819,6 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) return mga_g200er_set_plls(mdev, clock); } - misc = RREG8(MGA_MISC_IN); - misc &= ~MGAREG_MISC_CLK_SEL_MASK; - misc |= MGAREG_MISC_CLK_SEL_MGA_MSK; - WREG8(MGA_MISC_OUT, misc); - return 0; } diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index 977be0565c06..60e705283fe8 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -222,11 +222,10 @@ #define MGAREG_MISC_IOADSEL (0x1 << 0) #define MGAREG_MISC_RAMMAPEN (0x1 << 1) -#define MGAREG_MISC_CLK_SEL_MASK GENMASK(3, 2) -#define MGAREG_MISC_CLK_SEL_VGA25 (0x0 << 2) -#define MGAREG_MISC_CLK_SEL_VGA28 (0x1 << 2) -#define MGAREG_MISC_CLK_SEL_MGA_PIX (0x2 << 2) -#define MGAREG_MISC_CLK_SEL_MGA_MSK (0x3 << 2) +#define MGAREG_MISC_CLKSEL_MASK GENMASK(3, 2) +#define MGAREG_MISC_CLKSEL_VGA25 (0x0 << 2) +#define MGAREG_MISC_CLKSEL_VGA28 (0x1 << 2) +#define MGAREG_MISC_CLKSEL_MGA (0x3 << 2) #define MGAREG_MISC_VIDEO_DIS (0x1 << 4) #define MGAREG_MISC_HIGH_PG_SEL (0x1 << 5) #define MGAREG_MISC_HSYNCPOL BIT(6) -- 2.32.0

1 0

[PATCH v2 0/4] drm/amd/display: Base changes for isolating FPU operation in a single place
by Rodrigo Siqueira 14 Jul '21

14 Jul '21

Hi, In the display core, we utilize floats and doubles units for calculating modesetting parameters. One side effect of our approach to use double-precision is the fact that we spread multiple FPU access across our driver, which means that we can accidentally clobber user space FPU state. # Challenges 1. Keep in mind that this FPU code is ingrained in our display driver and performs several crucial tasks. Additionally, we already have multiple architectures available in the kernel and a large set of users; in other words, we prefer to avoid a radical approach that might break our user's system. 2. We share our display code with other OSs; thus, we need to maintain the interoperability between these two systems. 3. We need a mechanism for identifying which function uses FPU registers; fortunately, Peter Zijlstra wrote a series a couple of months ago where he introduced an FPU check for objtool. I used the following command for identifying the potential FPU usage: ./tools/objtool/objtool check -Ffa "drivers/gpu/drm/amd/display/dc/ANY_FILE.o" 4. Since our code heavily relies on FPU and the fact that we spread kernel_fpu_begin/end across multiple functions, we can have some complex scenarios that will require code refactoring. However, we want to avoid complicated changes since this is a formula to introduce regressions; we want something that allows us to fix it in small, safe, and reliable steps. 5. Unfortunately, for legacy reasons, we have some problems in how we program our FPU access, which in some weird scenarios can generate situations where we try to enter in the fpu mode multiple times or exit too early. # Our approach For trying to solve this problem, we came up with the following strategy: 1. Keep in mind that we are using kernel_fpu_begin/end spread in various areas and sometimes across multiple functions. If we try to move some of the functions to an isolated place, we can generate a situation where we can call the FPU protection more than once, causing multiple warnings. We can deal with this problem by adding a thin management layer around the kernel_fpu_begin/end used inside the display. 2. We will need a trace mechanism for this FPU management inside our display code. 3. After we get the thin layer that manages FPU, we can start to move each function that uses FPU to a centralized place. Our DQE runs multiple tests in different ASICs every week; we can take advantage of this to ensure that our FPU patches work does not introduce any regression. The idea is to work on a specific part of the code every week (e.g., week 1: DCN2, week 1: DCN2.1, etc.). 4. Finally, after we can isolate the FPU operations in a single place, we can altogether remove the FPU flags from other files and eliminate an unnecessary code introduced to deal with this problem. We can also remove the thin layer added in the step 3. # This series To maintain the interoperability between multiple OSes, we already have a define named DC_FP_START/END, which is a straightforward wrapper to kernel_fpu_begin/end in the Linux side. In this series, I decided to expand the scope of this DC_FP_* wrapper to trace FPU entrance and exit in the display code, but I also add a mechanism for managing the entrance and exit of kernel_fpu_begin/end. You can see the details on how I did that in the last two patches. I also isolate a simple function that requires FPU access to demonstrate my strategy for isolating this FPU access in a single place. If this series gets accepted, the following steps consist of moving all FPU functions weekly until we isolate everything in the fpu_operation folder. Changes since V1: - Use a better name for variables. - Update documentation. - Avoid preemption. * See update details per commit message Best Regards Rodrigo Siqueira Rodrigo Siqueira (4): drm/amd/display: Introduce FPU directory inside DC drm/amd/display: Add FPU event trace drm/amd/display: Add control mechanism for FPU utilization drm/amd/display: Add DC_FP helper to check FPU state .../gpu/drm/amd/display/amdgpu_dm/Makefile | 3 +- .../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 24 ++++ .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 114 ++++++++++++++++++ .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.h | 34 ++++++ drivers/gpu/drm/amd/display/dc/Makefile | 1 + drivers/gpu/drm/amd/display/dc/dc_trace.h | 3 + .../drm/amd/display/dc/dcn20/dcn20_resource.c | 41 +------ .../drm/amd/display/dc/dcn20/dcn20_resource.h | 2 - .../drm/amd/display/dc/dcn21/dcn21_resource.c | 2 + .../amd/display/dc/fpu_operations/Makefile | 58 +++++++++ .../drm/amd/display/dc/fpu_operations/dcn2x.c | 104 ++++++++++++++++ .../drm/amd/display/dc/fpu_operations/dcn2x.h | 33 +++++ drivers/gpu/drm/amd/display/dc/os_types.h | 6 +- 13 files changed, 382 insertions(+), 43 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.h create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/Makefile create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.c create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.h -- 2.25.1

2 8

[PATCH 1/5] drm/i915: document caching related bits
by Matthew Auld 14 Jul '21

14 Jul '21

Try to document the object caching related bits, like cache_coherent and cache_dirty. Suggested-by: Daniel Vetter <daniel.vetter(a)ffwll.ch> Signed-off-by: Matthew Auld <matthew.auld(a)intel.com> --- .../gpu/drm/i915/gem/i915_gem_object_types.h | 135 +++++++++++++++++- drivers/gpu/drm/i915/i915_drv.h | 9 -- 2 files changed, 131 insertions(+), 13 deletions(-) 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 ef3de2ae9723..02c3529b774c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -92,6 +92,57 @@ struct drm_i915_gem_object_ops { const char *name; /* friendly name for debug, e.g. lockdep classes */ }; +/** + * enum i915_cache_level - The supported GTT caching values for system memory + * pages. + * + * These translate to some special GTT PTE bits when binding pages into some + * address space. It also determines whether an object, or rather its pages are + * coherent with the GPU, when also reading or writing through the CPU cache + * with those pages. + * + * Userspace can also control this through struct drm_i915_gem_caching. + */ +enum i915_cache_level { + /** + * @I915_CACHE_NONE: + * + * Not coherent with the CPU cache. If the cache is dirty and we need + * the underlying pages to be coherent with some later GPU access then + * we need to manually flush the pages. + * + * Note that on shared-LLC platforms reads through the CPU cache are + * still coherent even with this setting. See also + * I915_BO_CACHE_COHERENT_FOR_READ for more details. + */ + I915_CACHE_NONE = 0, + /** + * @I915_CACHE_LLC: + * + * Coherent with the CPU cache. If the cache is dirty, then the GPU will + * ensure that access remains coherent, when both reading and writing + * through the CPU cache. + * + * Applies to both platforms with shared-LLC(HAS_LLC), and snooping + * based platforms(HAS_SNOOP). + */ + I915_CACHE_LLC, + /** + * @I915_CACHE_L3_LLC: + * + * gen7+, L3 sits between the domain specifc caches, eg sampler/render + * caches, and the large Last-Level-Cache. LLC is coherent with the CPU, + * but L3 is only visible to the GPU. + */ + I915_CACHE_L3_LLC, + /** + * @I915_CACHE_WT: + * + * hsw:gt3e Write-through for scanout buffers. + */ + I915_CACHE_WT, +}; + enum i915_map_type { I915_MAP_WB = 0, I915_MAP_WC, @@ -228,14 +279,90 @@ struct drm_i915_gem_object { unsigned int mem_flags; #define I915_BO_FLAG_STRUCT_PAGE BIT(0) /* Object backed by struct pages */ #define I915_BO_FLAG_IOMEM BIT(1) /* Object backed by IO memory */ - /* - * Is the object to be mapped as read-only to the GPU - * Only honoured if hardware has relevant pte bit + /** + * @cache_level: The desired GTT caching level. + * + * See enum i915_cache_level for possible values, along with what + * each does. */ unsigned int cache_level:3; - unsigned int cache_coherent:2; + /** + * @cache_coherent: + * + * Track whether the pages are coherent with the GPU if reading or + * writing through the CPU cache. + * + * This largely depends on the @cache_level, for example if the object + * is marked as I915_CACHE_LLC, then GPU access is coherent for both + * reads and writes through the CPU cache. + * + * Note that on platforms with shared-LLC support(HAS_LLC) reads through + * the CPU cache are always coherent, regardless of the @cache_level. On + * snooping based platforms this is not the case, unless the full + * I915_CACHE_LLC or similar setting is used. + * + * As a result of this we need to track coherency separately for reads + * and writes, in order to avoid superfluous flushing on shared-LLC + * platforms, for reads. + * + * I915_BO_CACHE_COHERENT_FOR_READ: + * + * When reading through the CPU cache, the GPU is still coherent. Note + * that no data has actually been modified here, so it might seem + * strange that we care about this. + * + * As an example, if some object is mapped on the CPU with write-back + * caching, and we read some page, then the cache likely now contains + * the data from that read. At this point the cache and main memory + * match up, so all good. But next the GPU needs to write some data to + * that same page. Now if the @cache_level is I915_CACHE_NONE and the + * the platform doesn't have the shared-LLC, then the GPU will + * effectively skip invalidating the cache(or however that works + * internally) when writing the new value. This is really bad since the + * GPU has just written some new data to main memory, but the CPU cache + * is still valid and now contains stale data. As a result the next time + * we do a cached read with the CPU, we are rewarded with stale data. + * Likewise if the cache is later flushed, we might be rewarded with + * overwriting main memory with stale data. + * + * I915_BO_CACHE_COHERENT_FOR_WRITE: + * + * When writing through the CPU cache, the GPU is still coherent. Note + * that this also implies I915_BO_CACHE_COHERENT_FOR_READ. + * + * This is never set when I915_CACHE_NONE is used for @cache_level, + * where instead we have to manually flush the caches after writing + * through the CPU cache. For other cache levels this should be set and + * the object is therefore considered coherent for both reads and writes + * through the CPU cache. + */ #define I915_BO_CACHE_COHERENT_FOR_READ BIT(0) #define I915_BO_CACHE_COHERENT_FOR_WRITE BIT(1) + unsigned int cache_coherent:2; + /** + * @cache_dirty: + * + * Track if the cache might be dirty for the @pages i.e it has yet to be + * written back to main memory. As a result reading directly from main + * memory might yield stale data. + * + * This also ties into whether the kernel is tracking the object as + * coherent with the GPU, as per @cache_coherent, as it determines if + * flushing might be needed at various points. + * + * Another part of @cache_dirty is managing flushing when first + * acquiring the pages for system memory, at this point the pages are + * considered foreign, so the default assumption is that the cache is + * dirty, for example the page zeroing done my the kernel might leave + * writes though the CPU cache, or swapping-in, while the actual data in + * main memory is potentially stale. Note that this is a potential + * security issue when dealing with userspace objects and zeroing. Now, + * whether we actually need apply the big sledgehammer of flushing all + * the pages on acquire depends on if @cache_coherent is marked as + * I915_BO_CACHE_COHERENT_FOR_WRITE, i.e that the GPU will be coherent + * for both reads and writes though the CPU cache. So pretty much this + * should only be needed for I915_CACHE_NONE objects. + */ unsigned int cache_dirty:1; /** diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c4747f4407ef..37bb1a3cadd4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -394,15 +394,6 @@ struct drm_i915_display_funcs { void (*read_luts)(struct intel_crtc_state *crtc_state); }; -enum i915_cache_level { - I915_CACHE_NONE = 0, - I915_CACHE_LLC, /* also used for snoopable memory on non-LLC */ - I915_CACHE_L3_LLC, /* gen7+, L3 sits between the domain specifc - caches, eg sampler/render caches, and the - large Last-Level-Cache. LLC is coherent with - the CPU, but L3 is only visible to the GPU. */ - I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */ -}; #define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */ -- 2.26.3

6 16

[PATCH] dma-buf: support users to change dma_buf.name
by guangming.cao＠mediatek.com 14 Jul '21

14 Jul '21

From: Guangming Cao <Guangming.Cao(a)mediatek.com> User space user can call DMA_BUF_SET_NAME to set dma_buf.name, also add a kernel api for users to do same thing at kernel side. Signed-off-by: Guangming Cao <Guangming.Cao(a)mediatek.com> --- drivers/dma-buf/dma-buf.c | 28 ++++++++++++++++++++++------ include/linux/dma-buf.h | 1 + 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 511fe0d217a0..949af232c644 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -331,20 +331,20 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll) * purpose between different devices. * * @dmabuf: [in] dmabuf buffer that will be renamed. - * @buf: [in] A piece of userspace memory that contains the name of + * @buf: [in] A piece of memory that contains the name of * the dma-buf. * * Returns 0 on success. If the dma-buf buffer is already attached to * devices, return -EBUSY. * */ -static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf) +long dma_buf_set_name(struct dma_buf *dmabuf, const char *buf) { - char *name = strndup_user(buf, DMA_BUF_NAME_LEN); + char *name = kstrndup(buf, DMA_BUF_NAME_LEN, GFP_KERNEL); long ret = 0; - if (IS_ERR(name)) - return PTR_ERR(name); + if (!name) + return -ENOMEM; dma_resv_lock(dmabuf->resv, NULL); if (!list_empty(&dmabuf->attachments)) { @@ -361,6 +361,22 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf) dma_resv_unlock(dmabuf->resv); return ret; } +EXPORT_SYMBOL_GPL(dma_buf_set_name); + +static long +dma_buf_set_name_user(struct dma_buf *dmabuf, const char __user *buf) +{ + char *name = strndup_user(buf, DMA_BUF_NAME_LEN); + long ret = 0; + + if (IS_ERR(name)) + return PTR_ERR(name); + + ret = dma_buf_set_name(dmabuf, name); + kfree(name); + + return ret; +} static long dma_buf_ioctl(struct file *file, unsigned int cmd, unsigned long arg) @@ -403,7 +419,7 @@ static long dma_buf_ioctl(struct file *file, case DMA_BUF_SET_NAME_A: case DMA_BUF_SET_NAME_B: - return dma_buf_set_name(dmabuf, (const char __user *)arg); + return dma_buf_set_name_user(dmabuf, (const char __user *)arg); default: return -ENOTTY; diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index efdc56b9d95f..e6612ab59a59 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -507,4 +507,5 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, unsigned long); int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map); void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map); +long dma_buf_set_name(struct dma_buf *dmabuf, const char *name); #endif /* __DMA_BUF_H__ */ -- 2.17.1

2 1

[PATCH v2 0/2] Add support of HDMI for rk3568
by Benjamin Gaignard 14 Jul '21

14 Jul '21

Add a compatible and platform datas to support HDMI for rk3568 SoC. version 2: - Add the clocks needed for the phy. Benjamin Gaignard (2): dt-bindings: display: rockchip: Add compatible for rk3568 HDMI drm/rockchip: dw_hdmi: add rk3568 support .../display/rockchip/rockchip,dw-hdmi.yaml | 6 +- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 68 +++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) -- 2.25.1

7 10

Re: [PATCH] dma-buf: add kernel count for dma_buf
by Guangming.Cao 14 Jul '21

14 Jul '21

On Wed, 2021-07-14 at 12:43 +0200, Christian König wrote: > Am 14.07.21 um 11:44 schrieb guangming.cao(a)mediatek.com: > > From: Guangming Cao <Guangming.Cao(a)mediatek.com> > > > > On Wed, 2021-07-14 at 10:46 +0200, Christian König wrote: > > > Am 14.07.21 um 09:11 schrieb guangming.cao(a)mediatek.com: > > > > From: Guangming Cao <Guangming.Cao(a)mediatek.com> > > > > > > > > Add a refcount for kernel to prevent UAF(Use After Free) issue. > > > > > > Well NAK on so many levels. > > > > > > > We can assume a case like below: > > > > 1. kernel space alloc dma_buf(file count = 1) > > > > 2. kernel use dma_buf to get fd(file count = 1) > > > > 3. userspace use fd to do mapping (file count = 2) > > > > > > Creating an userspace mapping increases the reference count for > > > the > > > underlying file object. > > > > > > See the implementation of mmap_region(): > > > ... > > > vma->vm_file = get_file(file); > > > error = call_mmap(file, vma); > > > ... > > > > > > What can happen is the the underlying exporter redirects the mmap > > > to > > > a > > > different file, e.g. TTM or GEM drivers do that all the time. > > > > > > But this is fine since then the VA mapping is independent of the > > > DMA- > > > buf. > > > > > > > 4. kernel call dma_buf_put (file count = 1) > > > > 5. userpsace close buffer fd(file count = 0) > > > > 6. at this time, buffer is released, but va is valid!! > > > > So we still can read/write buffer via mmap va, > > > > it maybe cause memory leak, or kernel exception. > > > > And also, if we use "ls -ll" to watch corresponding > > > > process > > > > fd link info, it also will cause kernel exception. > > > > > > > > Another case: > > > > Using dma_buf_fd to generate more than 1 fd, because > > > > dma_buf_fd will not increase file count, thus, when > > > > close > > > > the second fd, it maybe occurs error. > > > > > > Each opened fd will increase the reference count so this is > > > certainly > > > not correct what you describe here. > > > > > > Regards, > > > Christian. > > > > > > > Yes, mmap will increase file count by calling get_file, so step[2] > > -> > > step[3], file count increase 1. > > > > But, dma_buf_fd() will not increase file count. > > function "dma_buf_fd(struct dma_buf *dmabuf, int flags)" just get > > an > > unused fd, via call "get_unused_fd_flags(flags)", and call > > "fd_install(fd, dmabuf->file)", it will let associated "struct > > file*" > > in task's fdt->fd[fd] points to this dma_buf.file, not increase the > > file count of dma_buf.file. > > I think this is confusing, I can get more than 1 fds via > > dma_buf_fd, > > but they don't need to close it because they don't increase file > > count. > > > > However, dma_buf_put() can decrease file count at kernel side > > directly. > > If somebody write a ko to put file count of dma_buf.file many > > times, it > > will cause buffer freed earlier than except. At last on Android, I > > think this is a little bit dangerous. > > dma_buf_fd() takes the dma_buf pointer and converts it into a fd. So > the > reference is consumed. > > That's why users of this interface make sure to get a separate > reference, see drm_gem_prime_handle_to_fd() for example: > > ... > out_have_handle: > ret = dma_buf_fd(dmabuf, flags); > /* > * We must _not_ remove the buffer from the handle cache since > the > newly > * created dma buf is already linked in the global obj->dma_buf > pointer, > * and that is invariant as long as a userspace gem handle > exists. > * Closing the handle will clean out the cache anyway, so we > don't > leak. > */ > if (ret < 0) { > goto fail_put_dmabuf; > } else { > *prime_fd = ret; > ret = 0; > } > > goto out; > > fail_put_dmabuf: > dma_buf_put(dmabuf); > out: > ... > > You could submit a patch to improve the documentation and explicitly > note on dma_buf_fd() that the reference is consumed, but all of this > is > working perfectly fine. > > Regards, > Christian. > Thanks for your reply! Yes, drm works fine because it fully understand what dma-buf api will do. Improve the documentation is really good idea to prevent this case. But, what I can't understand is, for kernel api exported to corresponding users, we don't need to ensure all api is safe? And for general cases, dma-buf framework also need to prevent this case, isn't it, it will make dma-buf framework more strong? BRs! Guangming > > > > > > Solution: > > > > Add a kernel count for dma_buf, and make sure the file > > > > count > > > > of dma_buf.file hold by kernel is 1. > > > > > > > > Notes: For this solution, kref couldn't work because kernel ref > > > > maybe added from 0, but kref don't allow it. > > > > > > > > Signed-off-by: Guangming Cao <Guangming.Cao(a)mediatek.com> > > > > --- > > > > drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++---- > > > > include/linux/dma-buf.h | 6 ++++-- > > > > 2 files changed, 23 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma- > > > > buf.c > > > > index 511fe0d217a0..04ee92aac8b9 100644 > > > > --- a/drivers/dma-buf/dma-buf.c > > > > +++ b/drivers/dma-buf/dma-buf.c > > > > @@ -62,6 +62,7 @@ static void dma_buf_release(struct dentry > > > > *dentry) > > > > if (unlikely(!dmabuf)) > > > > return; > > > > > > > > + WARN_ON(atomic64_read(&dmabuf->kernel_ref)); > > > > BUG_ON(dmabuf->vmapping_counter); > > > > > > > > /* > > > > @@ -555,6 +556,7 @@ struct dma_buf *dma_buf_export(const struct > > > > dma_buf_export_info *exp_info) > > > > goto err_module; > > > > } > > > > > > > > + atomic64_set(&dmabuf->kernel_ref, 1); > > > > dmabuf->priv = exp_info->priv; > > > > dmabuf->ops = exp_info->ops; > > > > dmabuf->size = exp_info->size; > > > > @@ -617,6 +619,9 @@ int dma_buf_fd(struct dma_buf *dmabuf, int > > > > flags) > > > > > > > > fd_install(fd, dmabuf->file); > > > > > > > > + /* Add file cnt for each new fd */ > > > > + get_file(dmabuf->file); > > > > + > > > > return fd; > > > > } > > > > EXPORT_SYMBOL_GPL(dma_buf_fd); > > > > @@ -626,12 +631,13 @@ EXPORT_SYMBOL_GPL(dma_buf_fd); > > > > * @fd: [in] fd associated with the struct dma_buf to > > > > be > > > > returned > > > > * > > > > * On success, returns the struct dma_buf associated with an > > > > fd; > > > > uses > > > > - * file's refcounting done by fget to increase refcount. > > > > returns > > > > ERR_PTR > > > > - * otherwise. > > > > + * dmabuf's ref refcounting done by kref_get to increase > > > > refcount. > > > > + * Returns ERR_PTR otherwise. > > > > */ > > > > struct dma_buf *dma_buf_get(int fd) > > > > { > > > > struct file *file; > > > > + struct dma_buf *dmabuf; > > > > > > > > file = fget(fd); > > > > > > > > @@ -643,7 +649,12 @@ struct dma_buf *dma_buf_get(int fd) > > > > return ERR_PTR(-EINVAL); > > > > } > > > > > > > > - return file->private_data; > > > > + dmabuf = file->private_data; > > > > + /* replace file count increase as ref increase for kernel > > > > user > > > > */ > > > > + get_dma_buf(dmabuf); > > > > + fput(file); > > > > + > > > > + return dmabuf; > > > > } > > > > EXPORT_SYMBOL_GPL(dma_buf_get); > > > > > > > > @@ -662,7 +673,11 @@ void dma_buf_put(struct dma_buf *dmabuf) > > > > if (WARN_ON(!dmabuf || !dmabuf->file)) > > > > return; > > > > > > > > - fput(dmabuf->file); > > > > + if (WARN_ON(!atomic64_read(&dmabuf->kernel_ref))) > > > > + return; > > > > + > > > > + if (!atomic64_dec_return(&dmabuf->kernel_ref)) > > > > + fput(dmabuf->file); > > > > } > > > > EXPORT_SYMBOL_GPL(dma_buf_put); > > > > > > > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > > > > index efdc56b9d95f..bc790cb028eb 100644 > > > > --- a/include/linux/dma-buf.h > > > > +++ b/include/linux/dma-buf.h > > > > @@ -308,6 +308,7 @@ struct dma_buf_ops { > > > > struct dma_buf { > > > > size_t size; > > > > struct file *file; > > > > + atomic64_t kernel_ref; > > > > struct list_head attachments; > > > > const struct dma_buf_ops *ops; > > > > struct mutex lock; > > > > @@ -436,7 +437,7 @@ struct dma_buf_export_info { > > > > .owner = THIS_MODULE } > > > > > > > > /** > > > > - * get_dma_buf - convenience wrapper for get_file. > > > > + * get_dma_buf - increase a kernel ref of dma-buf > > > > * @dmabuf: [in] pointer to dma_buf > > > > * > > > > * Increments the reference count on the dma-buf, needed in > > > > case > > > > of drivers > > > > @@ -446,7 +447,8 @@ struct dma_buf_export_info { > > > > */ > > > > static inline void get_dma_buf(struct dma_buf *dmabuf) > > > > { > > > > - get_file(dmabuf->file); > > > > + if (atomic64_inc_return(&dmabuf->kernel_ref) == 1) > > > > + get_file(dmabuf->file); > > > > } > > > > > > > > /** > >

1 0

[PATCH v6 00/11] Clean up "mediatek,larb"
by Yong Wu 14 Jul '21

14 Jul '21

MediaTek IOMMU block diagram always like below: M4U | smi-common | ------------- | | ... | | larb1 larb2 | | vdec venc All the consumer connect with smi-larb, then connect with smi-common. When the consumer works, it should enable the smi-larb's power which also need enable the smi-common's power firstly. Thus, Firstly, use the device link connect the consumer and the smi-larbs. then add device link between the smi-larb and smi-common. After adding the device_link, then "mediatek,larb" property can be removed. the iommu consumer don't need call the mtk_smi_larb_get/put to enable the power and clock of smi-larb and smi-common. About the MM dt-binding/dtsi patches, I guess they should go together, thus I don't split them for each a MM module and each a SoC. Base on v5.14-rc1, and a jpeg[1] and mdp[2] patchset. [1] https://lore.kernel.org/linux-mediatek/20210702102304.3346429-1-hsinyi@chro… [2] https://lore.kernel.org/linux-mediatek/20210709022324.1607884-1-eizan@chrom… Change notes: v6: 1) rebase on v5.14-rc1. 2) Fix the issue commented in v5 from Dafna and Hsin-Yi. 3) Remove the patches about using pm_runtime_resume_and_get since they have already been merged by other patches. v5: https://lore.kernel.org/linux-mediatek/20210410091128.31823-1-yong.wu@media… 1) Base v5.12-rc2. 2) Remove changing the mtk-iommu to module_platform_driver patch, It have already been a independent patch. v4: https://lore.kernel.org/linux-mediatek/1590826218-23653-1-git-send-email-yo… base on v5.7-rc1. 1) Move drm PM patch before smi patchs. 2) Change builtin_platform_driver to module_platform_driver since we may need build as module. 3) Rebase many patchset as above. v3: https://lore.kernel.org/linux-iommu/1567503456-24725-1-git-send-email-yong.… 1) rebase on v5.3-rc1 and the latest mt8183 patchset. 2) Use device_is_bound to check whether the driver is ready from Matthias. 3) Add DL_FLAG_STATELESS flag when calling device_link_add and explain the reason in the commit message[3/14]. 4) Add a display patch[12/14] into this series. otherwise it may affect display HW fastlogo even though it don't happen in mt8183. v2: https://lore.kernel.org/linux-iommu/1560171313-28299-1-git-send-email-yong.… 1) rebase on v5.2-rc1. 2) Move adding device_link between the consumer and smi-larb into iommu_add_device from Robin. 3) add DL_FLAG_AUTOREMOVE_CONSUMER even though the smi is built-in from Evan. 4) Remove the shutdown callback in iommu. v1: https://lore.kernel.org/linux-iommu/1546318276-18993-1-git-send-email-yong.… Yong Wu (10): dt-binding: mediatek: Get rid of mediatek,larb for multimedia HW iommu/mediatek: Add probe_defer for smi-larb iommu/mediatek: Add device_link between the consumer and the larb devices media: mtk-jpeg: Get rid of mtk_smi_larb_get/put media: mtk-mdp: Get rid of mtk_smi_larb_get/put drm/mediatek: Get rid of mtk_smi_larb_get/put media: mtk-vcodec: Get rid of mtk_smi_larb_get/put memory: mtk-smi: Get rid of mtk_smi_larb_get/put arm: dts: mediatek: Get rid of mediatek,larb for MM nodes arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes Yongqiang Niu (1): drm/mediatek: Add pm runtime support for ovl and rdma .../display/mediatek/mediatek,disp.txt | 9 ---- .../bindings/media/mediatek-jpeg-decoder.yaml | 9 ---- .../bindings/media/mediatek-jpeg-encoder.yaml | 9 ---- .../bindings/media/mediatek-mdp.txt | 8 ---- .../bindings/media/mediatek-vcodec.txt | 4 -- arch/arm/boot/dts/mt2701.dtsi | 2 - arch/arm/boot/dts/mt7623n.dtsi | 5 -- arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 ------- arch/arm64/boot/dts/mediatek/mt8183.dtsi | 6 --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 9 +++- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 +++- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 19 ++++---- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 36 +-------------- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +- drivers/iommu/mtk_iommu.c | 24 +++++++++- drivers/iommu/mtk_iommu_v1.c | 22 ++++++++- .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 45 +----------------- .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 2 - drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 46 +------------------ drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 2 - drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 1 - .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 37 ++------------- .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 44 ++---------------- drivers/memory/mtk-smi.c | 14 ------ include/soc/mediatek/smi.h | 20 -------- 28 files changed, 85 insertions(+), 323 deletions(-) -- 2.18.0

3 27

[PATCH v2] drm/bridge: nwl-dsi: Avoid potential multiplication overflow on 32-bit
by Geert Uytterhoeven 14 Jul '21

14 Jul '21

As nwl_dsi.lanes is u32, and NSEC_PER_SEC is 1000000000L, the second multiplication in dsi->lanes * 8 * NSEC_PER_SEC will overflow on a 32-bit platform. Fix this by making the constant unsigned long long, forcing 64-bit arithmetic. As iMX8 is arm64, this driver is currently used on 64-bit platforms only, where long is 64-bit, so this cannot happen. But the issue will start to happen when the driver is reused for a 32-bit SoC (e.g. i.MX7ULP), or when code is copied for a new driver. Signed-off-by: Geert Uytterhoeven <geert+renesas(a)glider.be> Reviewed-by: Fabio Estevam <festevam(a)gmail.com> Reviewed-by: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com> --- Compile-tested only. v2: - Add Reviewed-by, - Add reference to i.MX7ULP. --- drivers/gpu/drm/bridge/nwl-dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c index 873995f0a7416e58..6002404ffcb9df08 100644 --- a/drivers/gpu/drm/bridge/nwl-dsi.c +++ b/drivers/gpu/drm/bridge/nwl-dsi.c @@ -196,7 +196,7 @@ static u32 ps2bc(struct nwl_dsi *dsi, unsigned long long ps) u32 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); return DIV64_U64_ROUND_UP(ps * dsi->mode.clock * bpp, - dsi->lanes * 8 * NSEC_PER_SEC); + dsi->lanes * 8ULL * NSEC_PER_SEC); } /* -- 2.25.1

1 0

← Newer
1
...
58
59
60
61
62
63
64
...
83
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel July 2021