Re: [PATCH 13/23] drm/rockchip: Make use of the helper component_compare_dev
Am Montag, 14. Februar 2022, 07:08:09 CET schrieb Yong Wu: > Use the common compare helper from component. > > Cc: Sandy Huang > Cc: "Heiko St¨¹bner" > Cc: linux-rockc...@lists.infradead.org > Signed-off-by: Yong Wu Acked-by: Heiko Stuebner > --- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index bec207de4544..3c2f2d6ecc36 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -290,11 +290,6 @@ int rockchip_drm_endpoint_is_subdriver(struct > device_node *ep) > return false; > } > > -static int compare_dev(struct device *dev, void *data) > -{ > - return dev == (struct device *)data; > -} > - > static void rockchip_drm_match_remove(struct device *dev) > { > struct device_link *link; > @@ -321,7 +316,7 @@ static struct component_match > *rockchip_drm_match_add(struct device *dev) > break; > > device_link_add(dev, d, DL_FLAG_STATELESS); > - component_match_add(dev, &match, compare_dev, d); > + component_match_add(dev, &match, component_compare_dev, > d); > } while (true); > } > >
Re: [PATCH 2/2] fbdev: Don't sort deferred-I/O pages by default
Hi Thomas, On Thu, Feb 10, 2022 at 4:24 PM Thomas Zimmermann wrote: > Fbdev's deferred I/O sorts all dirty pages by default, which incurs a > significant overhead. Make the sorting step optional and update the few > drivers that require it. Use a FIFO list by default. > > Sorting pages by memory offset for deferred I/O performs an implicit > bubble-sort step on the list of dirty pages. The algorithm goes through > the list of dirty pages and inserts each new page according to its > index field. Even worse, list traversal always starts at the first > entry. As video memory is most likely updated scanline by scanline, the > algorithm traverses through the complete list for each updated page. > > For example, with 1024x768x32bpp a page covers exactly one scanline. > Writing a single screen update from top to bottom requires updating > 768 pages. With an average list length of 384 entries, a screen update > creates (768 * 384 =) 294912 compare operation. What about using folios? If consecutive pages are merged into a single entry, there's much less (or nothing in the example above) to sort. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 2/2] fbdev: Don't sort deferred-I/O pages by default
Hi Am 14.02.22 um 09:05 schrieb Geert Uytterhoeven: Hi Thomas, On Thu, Feb 10, 2022 at 4:24 PM Thomas Zimmermann wrote: Fbdev's deferred I/O sorts all dirty pages by default, which incurs a significant overhead. Make the sorting step optional and update the few drivers that require it. Use a FIFO list by default. Sorting pages by memory offset for deferred I/O performs an implicit bubble-sort step on the list of dirty pages. The algorithm goes through the list of dirty pages and inserts each new page according to its index field. Even worse, list traversal always starts at the first entry. As video memory is most likely updated scanline by scanline, the algorithm traverses through the complete list for each updated page. For example, with 1024x768x32bpp a page covers exactly one scanline. Writing a single screen update from top to bottom requires updating 768 pages. With an average list length of 384 entries, a screen update creates (768 * 384 =) 294912 compare operation. What about using folios? If consecutive pages are merged into a single entry, there's much less (or nothing in the example above) to sort. How would the code know that? Calls to page_mkwrite happen pagefault-by-pagefault in any order AFAICT. Best regards Thomas [1] https://elixir.bootlin.com/linux/v5.16.9/source/include/linux/mm_types.h#L258 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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: [PATCH v12 1/5] drm: improve drm_buddy_alloc function
On Mon, 14 Feb 2022 at 06:32, Christian König wrote: > > Am 13.02.22 um 09:52 schrieb Arunpravin: > > - Make drm_buddy_alloc a single function to handle > >range allocation and non-range allocation demands > > > > - Implemented a new function alloc_range() which allocates > >the requested power-of-two block comply with range limitations > > > > - Moved order computation and memory alignment logic from > >i915 driver to drm buddy > > > > v2: > >merged below changes to keep the build unbroken > > - drm_buddy_alloc_range() becomes obsolete and may be removed > > - enable ttm range allocation (fpfn / lpfn) support in i915 driver > > - apply enhanced drm_buddy_alloc() function to i915 driver > > > > v3(Matthew Auld): > >- Fix alignment issues and remove unnecessary list_empty check > >- add more validation checks for input arguments > >- make alloc_range() block allocations as bottom-up > >- optimize order computation logic > >- replace uint64_t with u64, which is preferred in the kernel > > > > v4(Matthew Auld): > >- keep drm_buddy_alloc_range() function implementation for generic > > actual range allocations > >- keep alloc_range() implementation for end bias allocations > > > > v5(Matthew Auld): > >- modify drm_buddy_alloc() passing argument place->lpfn to lpfn > > as place->lpfn will currently always be zero for i915 > > > > v6(Matthew Auld): > >- fixup potential uaf - If we are unlucky and can't allocate > > enough memory when splitting blocks, where we temporarily > > end up with the given block and its buddy on the respective > > free list, then we need to ensure we delete both blocks, > > and no just the buddy, before potentially freeing them > > > >- fix warnings reported by kernel test robot > > > > v7(Matthew Auld): > >- revert fixup potential uaf > >- keep __alloc_range() add node to the list logic same as > > drm_buddy_alloc_blocks() by having a temporary list variable > >- at drm_buddy_alloc_blocks() keep i915 range_overflows macro > > and add a new check for end variable > > > > v8: > >- fix warnings reported by kernel test robot > > > > v9(Matthew Auld): > >- remove DRM_BUDDY_RANGE_ALLOCATION flag > >- remove unnecessary function description > > > > Signed-off-by: Arunpravin > > Reviewed-by: Matthew Auld > > As long as nobody objects I'm going to push patches 1-3 to drm-misc-next > in the next hour or so: As part of this could you also push https://patchwork.freedesktop.org/series/99842/ ? > > Then going to take a deeper look into patches 4 and 5 to get them reviewed. > > Thanks, > Christian. > > > --- > > drivers/gpu/drm/drm_buddy.c | 292 +- > > drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 63 ++-- > > drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 2 + > > include/drm/drm_buddy.h | 11 +- > > 4 files changed, 250 insertions(+), 118 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c > > index d60878bc9c20..e0c0d786a572 100644 > > --- a/drivers/gpu/drm/drm_buddy.c > > +++ b/drivers/gpu/drm/drm_buddy.c > > @@ -282,23 +282,97 @@ void drm_buddy_free_list(struct drm_buddy *mm, struct > > list_head *objects) > > } > > EXPORT_SYMBOL(drm_buddy_free_list); > > > > -/** > > - * drm_buddy_alloc_blocks - allocate power-of-two blocks > > - * > > - * @mm: DRM buddy manager to allocate from > > - * @order: size of the allocation > > - * > > - * The order value here translates to: > > - * > > - * 0 = 2^0 * mm->chunk_size > > - * 1 = 2^1 * mm->chunk_size > > - * 2 = 2^2 * mm->chunk_size > > - * > > - * Returns: > > - * allocated ptr to the &drm_buddy_block on success > > - */ > > -struct drm_buddy_block * > > -drm_buddy_alloc_blocks(struct drm_buddy *mm, unsigned int order) > > +static inline bool overlaps(u64 s1, u64 e1, u64 s2, u64 e2) > > +{ > > + return s1 <= e2 && e1 >= s2; > > +} > > + > > +static inline bool contains(u64 s1, u64 e1, u64 s2, u64 e2) > > +{ > > + return s1 <= s2 && e1 >= e2; > > +} > > + > > +static struct drm_buddy_block * > > +alloc_range_bias(struct drm_buddy *mm, > > + u64 start, u64 end, > > + unsigned int order) > > +{ > > + struct drm_buddy_block *block; > > + struct drm_buddy_block *buddy; > > + LIST_HEAD(dfs); > > + int err; > > + int i; > > + > > + end = end - 1; > > + > > + for (i = 0; i < mm->n_roots; ++i) > > + list_add_tail(&mm->roots[i]->tmp_link, &dfs); > > + > > + do { > > + u64 block_start; > > + u64 block_end; > > + > > + block = list_first_entry_or_null(&dfs, > > + struct drm_buddy_block, > > + tmp_link); > > + if (!block) > > + break; > > + > > + list_del(&block->tmp_link);
[PATCH v3] drm/mediatek: allow commands to be sent during video mode
Mipi dsi panel drivers can use mipi_dsi_dcs_{set,get}_display_brightness() to request backlight changes. This can be done during panel initialization (dsi is in command mode) or afterwards (dsi is in Video Mode). When the DSI is in Video Mode, all commands are rejected. Detect current DSI mode in mtk_dsi_host_transfer() and switch modes temporarily to allow commands to be sent. Signed-off-by: Julien STEPHAN Signed-off-by: Mattijs Korpershoek --- Changes in v3: - increase readability of code and use correct return variable (see comment https://lore.kernel.org/linux-mediatek/4907bdc1-b4a6-e9ad-5cfa-266fc20c0...@collabora.com/) Changes in v2: - update commit message to be more descriptive drivers/gpu/drm/mediatek/mtk_dsi.c | 33 ++ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 5d90d2eb0019..7f1b2b7ed3fd 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -891,24 +891,33 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, u8 read_data[16]; void *src_addr; u8 irq_flag = CMD_DONE_INT_FLAG; + u32 dsi_mode; + int ret; - if (readl(dsi->regs + DSI_MODE_CTRL) & MODE) { - DRM_ERROR("dsi engine is not command mode\n"); - return -EINVAL; + dsi_mode = readl(dsi->regs + DSI_MODE_CTRL); + if (dsi_mode & MODE) { + mtk_dsi_stop(dsi); + ret = mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); + if (ret) + goto restore_dsi_mode; } if (MTK_DSI_HOST_IS_READ(msg->type)) irq_flag |= LPRX_RD_RDY_INT_FLAG; - if (mtk_dsi_host_send_cmd(dsi, msg, irq_flag) < 0) - return -ETIME; + ret = mtk_dsi_host_send_cmd(dsi, msg, irq_flag); + if (ret) + goto restore_dsi_mode; - if (!MTK_DSI_HOST_IS_READ(msg->type)) - return 0; + if (!MTK_DSI_HOST_IS_READ(msg->type)) { + recv_cnt = 0; + goto restore_dsi_mode; + } if (!msg->rx_buf) { DRM_ERROR("dsi receive buffer size may be NULL\n"); - return -EINVAL; + ret = -EINVAL; + goto restore_dsi_mode; } for (i = 0; i < 16; i++) @@ -933,7 +942,13 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, DRM_INFO("dsi get %d byte data from the panel address(0x%x)\n", recv_cnt, *((u8 *)(msg->tx_buf))); - return recv_cnt; +restore_dsi_mode: + if (dsi_mode & MODE) { + mtk_dsi_set_mode(dsi); + mtk_dsi_start(dsi); + } + + return ret < 0 ? ret: recv_cnt; } static const struct mipi_dsi_host_ops mtk_dsi_ops = { -- 2.35.1
Re: [PATCH v5 2/6] drm/format-helper: Add drm_fb_xrgb8888_to_mono_reversed()
Hello Geert, Thanks for your feedback. On 2/12/22 16:54, Geert Uytterhoeven wrote: [snip] >> + >> + for (i = start; i < end; i++) { >> + unsigned int x = xb * 8 + i; >> + >> + byte >>= 1; >> + if (src[x] >> 7) >> + byte |= BIT(7); >> + } > > x = xb * 8; > for (i = start; i < end; i++) { > byte >>= 1; > byte |= src[x + i] & BIT(7); > } > I think the original version from Noralf is easier to read and understand. It makes explicit that the bit is set if the grayscale value is >= 128. [snip] >> +void drm_fb_xrgb_to_mono_reversed(void *dst, unsigned int dst_pitch, >> const void *vaddr, >> + const struct drm_framebuffer *fb, >> const struct drm_rect *clip) >> +{ [snip] >> + u8 *mono = dst, *gray8; >> + u32 *src32; [snip] >> +* >> +* Allocate a buffer to be used for both copying from the cma >> +* memory and to store the intermediate grayscale line pixels. >> +*/ >> + src32 = kmalloc(len_src32 + linepixels, GFP_KERNEL); >> + if (!src32) >> + return; >> + >> + gray8 = (u8 *)src32 + len_src32; > > The cast can be removed if src32 is changed to "void *". > For symmetry, gray8 and mono can be changed to "void *", too. > Yes, but these being "u32 *" and "u8 *" also express that the source buffer contains 32-bit XRGB pixels, the intermediate buffer a 8-bit grayscale pixel format and the destination buffer a 8-bit packed reversed monochrome. Using "void *" for these would make that less clear when reading the code IMO. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`
On Mon, Feb 14, 2022 at 09:34:26AM +0200, Mika Westerberg wrote: > On Fri, Feb 11, 2022 at 03:45:46PM -0600, Bjorn Helgaas wrote: > > My expectation is that "USB" (like "PCI" and "PCIe") tells me > > something about how a device is electrically connected and how > > software can operate it. It doesn't really tell me anything about > > whether those electrical connections are permanent, made through an > > internal slot, or made through an external connector and cable. > > It is used to identify "tunneled" ports (whether PCIe, USB 3.x or > DisplayPort). Tunnels are created by software (in Linux it is the > Thunderbolt driver) and are dynamic in nature. The USB4 links go over > USB Type-C cable which also is something user can plug/unplug freely. > > I would say it is reasonable expectation that anything behind these > ports can be assumed as "removable". USB gadgets may be soldered to the mainboard. Those cannot be unplugged freely. It is common practice to solder USB Ethernet or USB FTDI serial ports and nothing's preventing a vendor to solder USB4/Thunderbolt gadgets.
Re: [PATCH] drm/doc: Clarify what ioctls can be used on render nodes
On Wed, 9 Feb 2022 11:57:27 -0700 Jeffrey Hugo wrote: > The documentation for render nodes indicates that only "PRIME-related" > ioctls are valid on render nodes, but the documentation does not clarify > what that means. If the reader is not familiar with PRIME, they may > beleive this to be only the ioctls with "PRIME" in the name and not other > ioctls such as set of syncobj ioctls. Clarify the situation for the > reader by referencing where the reader will find a current list of valid > ioctls. > > Signed-off-by: Jeffrey Hugo > --- > > I was confused by this when reading the documentation. Now that I have > figured out what the documentation means, I would like to add a clarification > for the next reader which would have helped me. > > Documentation/gpu/drm-uapi.rst | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 199afb5..ce47b42 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -148,7 +148,9 @@ clients together with the legacy drmAuth authentication > procedure. > If a driver advertises render node support, DRM core will create a > separate render node called renderD. There will be one render node > per device. No ioctls except PRIME-related ioctls will be allowed on > -this node. Especially GEM_OPEN will be explicitly prohibited. Render > +this node. Especially GEM_OPEN will be explicitly prohibited. For a > +complete list of driver-independent ioctls that can be used on render > +nodes, see the ioctls marked DRM_RENDER_ALLOW in drm_ioctl.c Render > nodes are designed to avoid the buffer-leaks, which occur if clients > guess the flink names or mmap offsets on the legacy interface. > Additionally to this basic interface, drivers must mark their Hi, I think this is correct, but I didn't actually check the code, so Acked-by: Pekka Paalanen Thanks, pq pgp8iE_YPPqrY.pgp Description: OpenPGP digital signature
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
Hi Am 11.02.22 um 16:41 schrieb Andy Shevchenko: [...] IMO *always* prefer a for loop over while or do-while. The for (i = 0; i < N; i++) is such a strong paradigm in C. You instantly know how many times you're going to loop, at a glance. Not so with with the alternatives, which should be used sparingly. while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. Naw, that's not true. An idiomatic for loop, such as for (i = ...; i < N; ++i), is such a strong pattern that it's way better than the corresponding while loop. Best regards Thomas And yes, the do-while suggested above is buggy, and you actually need to stop and think to see why. It depends if pixels can be 0 or not and if it's not, then does it contain last or number. The do {} while (--pixels); might be buggy iff pixels may be 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
Re: [PATCH 2/2] fbdev: Don't sort deferred-I/O pages by default
Hi Thomas, On Mon, Feb 14, 2022 at 9:28 AM Thomas Zimmermann wrote: > Am 14.02.22 um 09:05 schrieb Geert Uytterhoeven: > > On Thu, Feb 10, 2022 at 4:24 PM Thomas Zimmermann > > wrote: > >> Fbdev's deferred I/O sorts all dirty pages by default, which incurs a > >> significant overhead. Make the sorting step optional and update the few > >> drivers that require it. Use a FIFO list by default. > >> > >> Sorting pages by memory offset for deferred I/O performs an implicit > >> bubble-sort step on the list of dirty pages. The algorithm goes through > >> the list of dirty pages and inserts each new page according to its > >> index field. Even worse, list traversal always starts at the first > >> entry. As video memory is most likely updated scanline by scanline, the > >> algorithm traverses through the complete list for each updated page. > >> > >> For example, with 1024x768x32bpp a page covers exactly one scanline. > >> Writing a single screen update from top to bottom requires updating > >> 768 pages. With an average list length of 384 entries, a screen update > >> creates (768 * 384 =) 294912 compare operation. > > > > What about using folios? > > If consecutive pages are merged into a single entry, there's much less > > (or nothing in the example above) to sort. > > How would the code know that? Calls to page_mkwrite happen > pagefault-by-pagefault in any order AFAICT. fb_deferred_io_mkwrite() would still be called for a page, but an adjacent page can be merged with an existing entry while adding it to the list. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v12 1/5] drm: improve drm_buddy_alloc function
Am 14.02.22 um 09:36 schrieb Matthew Auld: On Mon, 14 Feb 2022 at 06:32, Christian König wrote: Am 13.02.22 um 09:52 schrieb Arunpravin: - Make drm_buddy_alloc a single function to handle range allocation and non-range allocation demands - Implemented a new function alloc_range() which allocates the requested power-of-two block comply with range limitations - Moved order computation and memory alignment logic from i915 driver to drm buddy v2: merged below changes to keep the build unbroken - drm_buddy_alloc_range() becomes obsolete and may be removed - enable ttm range allocation (fpfn / lpfn) support in i915 driver - apply enhanced drm_buddy_alloc() function to i915 driver v3(Matthew Auld): - Fix alignment issues and remove unnecessary list_empty check - add more validation checks for input arguments - make alloc_range() block allocations as bottom-up - optimize order computation logic - replace uint64_t with u64, which is preferred in the kernel v4(Matthew Auld): - keep drm_buddy_alloc_range() function implementation for generic actual range allocations - keep alloc_range() implementation for end bias allocations v5(Matthew Auld): - modify drm_buddy_alloc() passing argument place->lpfn to lpfn as place->lpfn will currently always be zero for i915 v6(Matthew Auld): - fixup potential uaf - If we are unlucky and can't allocate enough memory when splitting blocks, where we temporarily end up with the given block and its buddy on the respective free list, then we need to ensure we delete both blocks, and no just the buddy, before potentially freeing them - fix warnings reported by kernel test robot v7(Matthew Auld): - revert fixup potential uaf - keep __alloc_range() add node to the list logic same as drm_buddy_alloc_blocks() by having a temporary list variable - at drm_buddy_alloc_blocks() keep i915 range_overflows macro and add a new check for end variable v8: - fix warnings reported by kernel test robot v9(Matthew Auld): - remove DRM_BUDDY_RANGE_ALLOCATION flag - remove unnecessary function description Signed-off-by: Arunpravin Reviewed-by: Matthew Auld As long as nobody objects I'm going to push patches 1-3 to drm-misc-next in the next hour or so: As part of this could you also push https://patchwork.freedesktop.org/series/99842/ ? Sure, but Arun said in our internal chat that I should wait with that anyway since he wanted to sort out one more issue. Christian. Then going to take a deeper look into patches 4 and 5 to get them reviewed. Thanks, Christian. --- drivers/gpu/drm/drm_buddy.c | 292 +- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 63 ++-- drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 2 + include/drm/drm_buddy.h | 11 +- 4 files changed, 250 insertions(+), 118 deletions(-) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index d60878bc9c20..e0c0d786a572 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -282,23 +282,97 @@ void drm_buddy_free_list(struct drm_buddy *mm, struct list_head *objects) } EXPORT_SYMBOL(drm_buddy_free_list); -/** - * drm_buddy_alloc_blocks - allocate power-of-two blocks - * - * @mm: DRM buddy manager to allocate from - * @order: size of the allocation - * - * The order value here translates to: - * - * 0 = 2^0 * mm->chunk_size - * 1 = 2^1 * mm->chunk_size - * 2 = 2^2 * mm->chunk_size - * - * Returns: - * allocated ptr to the &drm_buddy_block on success - */ -struct drm_buddy_block * -drm_buddy_alloc_blocks(struct drm_buddy *mm, unsigned int order) +static inline bool overlaps(u64 s1, u64 e1, u64 s2, u64 e2) +{ + return s1 <= e2 && e1 >= s2; +} + +static inline bool contains(u64 s1, u64 e1, u64 s2, u64 e2) +{ + return s1 <= s2 && e1 >= e2; +} + +static struct drm_buddy_block * +alloc_range_bias(struct drm_buddy *mm, + u64 start, u64 end, + unsigned int order) +{ + struct drm_buddy_block *block; + struct drm_buddy_block *buddy; + LIST_HEAD(dfs); + int err; + int i; + + end = end - 1; + + for (i = 0; i < mm->n_roots; ++i) + list_add_tail(&mm->roots[i]->tmp_link, &dfs); + + do { + u64 block_start; + u64 block_end; + + block = list_first_entry_or_null(&dfs, + struct drm_buddy_block, + tmp_link); + if (!block) + break; + + list_del(&block->tmp_link); + + if (drm_buddy_block_order(block) < order) + continue; + + block_start = drm_buddy_block_offset(block); + block_end = block_start + drm_buddy_block_size(mm, block) - 1; + + if (!overlaps(start, end, block_star
Re: [PATCH v3] drm/mediatek: allow commands to be sent during video mode
Il 14/02/22 09:47, Julien STEPHAN ha scritto: Mipi dsi panel drivers can use mipi_dsi_dcs_{set,get}_display_brightness() to request backlight changes. This can be done during panel initialization (dsi is in command mode) or afterwards (dsi is in Video Mode). When the DSI is in Video Mode, all commands are rejected. Detect current DSI mode in mtk_dsi_host_transfer() and switch modes temporarily to allow commands to be sent. Signed-off-by: Julien STEPHAN Signed-off-by: Mattijs Korpershoek --- Changes in v3: - increase readability of code and use correct return variable (see comment https://lore.kernel.org/linux-mediatek/4907bdc1-b4a6-e9ad-5cfa-266fc20c0...@collabora.com/) Changes in v2: - update commit message to be more descriptive drivers/gpu/drm/mediatek/mtk_dsi.c | 33 ++ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 5d90d2eb0019..7f1b2b7ed3fd 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -891,24 +891,33 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, u8 read_data[16]; void *src_addr; u8 irq_flag = CMD_DONE_INT_FLAG; + u32 dsi_mode; + int ret; - if (readl(dsi->regs + DSI_MODE_CTRL) & MODE) { - DRM_ERROR("dsi engine is not command mode\n"); - return -EINVAL; + dsi_mode = readl(dsi->regs + DSI_MODE_CTRL); + if (dsi_mode & MODE) { + mtk_dsi_stop(dsi); + ret = mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); + if (ret) + goto restore_dsi_mode; } if (MTK_DSI_HOST_IS_READ(msg->type)) irq_flag |= LPRX_RD_RDY_INT_FLAG; - if (mtk_dsi_host_send_cmd(dsi, msg, irq_flag) < 0) - return -ETIME; + ret = mtk_dsi_host_send_cmd(dsi, msg, irq_flag); + if (ret) + goto restore_dsi_mode; - if (!MTK_DSI_HOST_IS_READ(msg->type)) - return 0; + if (!MTK_DSI_HOST_IS_READ(msg->type)) { + recv_cnt = 0; + goto restore_dsi_mode; + } if (!msg->rx_buf) { DRM_ERROR("dsi receive buffer size may be NULL\n"); - return -EINVAL; + ret = -EINVAL; + goto restore_dsi_mode; } for (i = 0; i < 16; i++) @@ -933,7 +942,13 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, DRM_INFO("dsi get %d byte data from the panel address(0x%x)\n", recv_cnt, *((u8 *)(msg->tx_buf))); - return recv_cnt; +restore_dsi_mode: + if (dsi_mode & MODE) { + mtk_dsi_set_mode(dsi); + mtk_dsi_start(dsi); + } + + return ret < 0 ? ret: recv_cnt; Nitpick: you've missed a space ... ret : recv_cnt; With that fixed, Reviewed-by: AngeloGioacchino Del Regno } static const struct mipi_dsi_host_ops mtk_dsi_ops = { -- 2.35.1
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
On Fri, 11 Feb 2022 19:27:12 +0200 Andy Shevchenko wrote: > On Fri, Feb 11, 2022 at 06:25:17PM +0200, Jani Nikula wrote: > > On Fri, 11 Feb 2022, Andy Shevchenko > > wrote: > > > On Fri, Feb 11, 2022 at 02:05:56PM +0200, Jani Nikula wrote: > > >> On Fri, 11 Feb 2022, Thomas Zimmermann wrote: > > >> > Am 11.02.22 um 12:12 schrieb Andy Shevchenko: > > >> >> On Fri, Feb 11, 2022 at 11:40:13AM +0100, Javier Martinez Canillas > > >> >> wrote: > > >> >>> On 2/11/22 11:28, Andy Shevchenko wrote: > > >> On Fri, Feb 11, 2022 at 10:19:22AM +0100, Javier Martinez Canillas > > >> wrote: > > > > > > ... > > > > > >> > +static void drm_fb_xrgb_to_gray8_line(u8 *dst, const u32 > > >> > *src, unsigned int pixels) > > >> > +{ > > >> > + unsigned int x; > > >> > + > > >> > + for (x = 0; x < pixels; x++) { > > >> > + u8 r = (*src & 0x00ff) >> 16; > > >> > + u8 g = (*src & 0xff00) >> 8; > > >> > + u8 b = *src & 0x00ff; > > >> > + > > >> > + /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */ > > >> > + *dst++ = (3 * r + 6 * g + b) / 10; > > >> > + src++; > > >> > + } > > >> > > >> Can be done as > > >> > > >> while (pixels--) { > > >> ... > > >> } > > >> > > >> or > > >> > > >> do { > > >> ... > > >> } while (--pixels); > > >> > > >> >>> > > >> >>> I don't see why a while loop would be an improvement here TBH. > > >> >> > > >> >> Less letters to parse when reading the code. > > >> > > > >> > It's a simple refactoring of code that has worked well so far. Let's > > >> > leave it as-is for now. > > >> > > >> IMO *always* prefer a for loop over while or do-while. > > >> > > >> The for (i = 0; i < N; i++) is such a strong paradigm in C. You > > >> instantly know how many times you're going to loop, at a glance. Not so > > >> with with the alternatives, which should be used sparingly. > > > > > > while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. > > > > And while() is just syntax sugar for goto. :p > > > > The for loop written as for (i = 0; i < N; i++) is hands down the most > > obvious counting loop pattern there is in C. > > > > >> And yes, the do-while suggested above is buggy, and you actually need to > > >> stop and think to see why. > > > > > > It depends if pixels can be 0 or not and if it's not, then does it > > > contain last > > > or number. > > > > > > The do {} while (--pixels); might be buggy iff pixels may be 0. > > > > Yeah. And how long does it take to figure that out? > > Okay, I made a mistake to drop the explanation. So, I (mistakenly) assumed > that people know this difference between post-decrement and pre-decrement > (note, while-loop here is not what is problematic). That was not the question. The question was, how long does it take to figure out if pixels can or cannot be zero? Code is styled for humans other than the author, not for compilers. Having to stop to think about the difference between post- and pre-decrement to figure out when the while-loop runs does take me a few more brain cycles to understand, even though I know the rules very well. I would call that brain cycle optimization, and leave the CPU cycle optimization for the compiler in these cases. Thanks, pq pgp7OjJ7dCrBk.pgp Description: OpenPGP digital signature
[PATCH v4] drm/mediatek: allow commands to be sent during video mode
Mipi dsi panel drivers can use mipi_dsi_dcs_{set,get}_display_brightness() to request backlight changes. This can be done during panel initialization (dsi is in command mode) or afterwards (dsi is in Video Mode). When the DSI is in Video Mode, all commands are rejected. Detect current DSI mode in mtk_dsi_host_transfer() and switch modes temporarily to allow commands to be sent. Signed-off-by: Julien STEPHAN Signed-off-by: Mattijs Korpershoek --- Changes in v4: - fix missing space: "ret : recv_cnt;" Changes in v3: - increase readability of code and use correct return variable (see comment https://lore.kernel.org/linux-mediatek/4907bdc1-b4a6-e9ad-5cfa-266fc20c0...@collabora.com/) Changes in v2: - update commit message to be more descriptive drivers/gpu/drm/mediatek/mtk_dsi.c | 33 ++ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 5d90d2eb0019..abdd9cdebd86 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -891,24 +891,33 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, u8 read_data[16]; void *src_addr; u8 irq_flag = CMD_DONE_INT_FLAG; + u32 dsi_mode; + int ret; - if (readl(dsi->regs + DSI_MODE_CTRL) & MODE) { - DRM_ERROR("dsi engine is not command mode\n"); - return -EINVAL; + dsi_mode = readl(dsi->regs + DSI_MODE_CTRL); + if (dsi_mode & MODE) { + mtk_dsi_stop(dsi); + ret = mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); + if (ret) + goto restore_dsi_mode; } if (MTK_DSI_HOST_IS_READ(msg->type)) irq_flag |= LPRX_RD_RDY_INT_FLAG; - if (mtk_dsi_host_send_cmd(dsi, msg, irq_flag) < 0) - return -ETIME; + ret = mtk_dsi_host_send_cmd(dsi, msg, irq_flag); + if (ret) + goto restore_dsi_mode; - if (!MTK_DSI_HOST_IS_READ(msg->type)) - return 0; + if (!MTK_DSI_HOST_IS_READ(msg->type)) { + recv_cnt = 0; + goto restore_dsi_mode; + } if (!msg->rx_buf) { DRM_ERROR("dsi receive buffer size may be NULL\n"); - return -EINVAL; + ret = -EINVAL; + goto restore_dsi_mode; } for (i = 0; i < 16; i++) @@ -933,7 +942,13 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, DRM_INFO("dsi get %d byte data from the panel address(0x%x)\n", recv_cnt, *((u8 *)(msg->tx_buf))); - return recv_cnt; +restore_dsi_mode: + if (dsi_mode & MODE) { + mtk_dsi_set_mode(dsi); + mtk_dsi_start(dsi); + } + + return ret < 0 ? ret : recv_cnt; } static const struct mipi_dsi_host_ops mtk_dsi_ops = { -- 2.35.1
[PATCH 01/11] drm/ttm: fix resource manager size type and description
That are not pages any more. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_resource.c | 6 +++--- include/drm/ttm/ttm_resource.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 68344c90549b..ae40e144e728 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -153,19 +153,19 @@ void ttm_resource_set_bo(struct ttm_resource *res, * * @man: memory manager object to init * @bdev: ttm device this manager belongs to - * @p_size: size managed area in pages. + * @size: size of managed resources in arbitary units * * Initialise core parts of a manager object. */ void ttm_resource_manager_init(struct ttm_resource_manager *man, struct ttm_device *bdev, - unsigned long p_size) + uint64_t size) { unsigned i; spin_lock_init(&man->move_lock); man->bdev = bdev; - man->size = p_size; + man->size = size; for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) INIT_LIST_HEAD(&man->lru[i]); diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index 69eea9d6399b..555a11fb8a7f 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -278,7 +278,7 @@ void ttm_resource_set_bo(struct ttm_resource *res, void ttm_resource_manager_init(struct ttm_resource_manager *man, struct ttm_device *bdev, - unsigned long p_size); + uint64_t size); int ttm_resource_manager_evict_all(struct ttm_device *bdev, struct ttm_resource_manager *man); -- 2.25.1
[PATCH 04/11] drm/ttm: add resource iterator v2
Instead of duplicating that at different places add an iterator over all the resources in a resource manager. v2: add lockdep annotation and kerneldoc Signed-off-by: Christian König Tested-by: Bas Nieuwenhuizen Reviewed-by: Daniel Vetter --- drivers/gpu/drm/ttm/ttm_bo.c | 41 ++--- drivers/gpu/drm/ttm/ttm_device.c | 26 +++- drivers/gpu/drm/ttm/ttm_resource.c | 49 ++ include/drm/ttm/ttm_resource.h | 23 ++ 4 files changed, 99 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index cb0fa932d495..599be3dda8a9 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -579,38 +579,29 @@ int ttm_mem_evict_first(struct ttm_device *bdev, struct ww_acquire_ctx *ticket) { struct ttm_buffer_object *bo = NULL, *busy_bo = NULL; + struct ttm_resource_cursor cursor; struct ttm_resource *res; bool locked = false; - unsigned i; int ret; spin_lock(&bdev->lru_lock); - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { - list_for_each_entry(res, &man->lru[i], lru) { - bool busy; - - bo = res->bo; - if (!ttm_bo_evict_swapout_allowable(bo, ctx, place, - &locked, &busy)) { - if (busy && !busy_bo && ticket != - dma_resv_locking_ctx(bo->base.resv)) - busy_bo = bo; - continue; - } - - if (!ttm_bo_get_unless_zero(bo)) { - if (locked) - dma_resv_unlock(bo->base.resv); - continue; - } - break; + ttm_resource_manager_for_each_res(man, &cursor, res) { + bool busy; + + if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place, + &locked, &busy)) { + if (busy && !busy_bo && ticket != + dma_resv_locking_ctx(bo->base.resv)) + busy_bo = res->bo; + continue; } - /* If the inner loop terminated early, we have our candidate */ - if (&res->lru != &man->lru[i]) - break; - - bo = NULL; + if (!ttm_bo_get_unless_zero(res->bo)) { + if (locked) + dma_resv_unlock(res->bo->base.resv); + continue; + } + bo = res->bo; } if (!bo) { diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index ba35887147ba..a0562ab386f5 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -142,10 +142,10 @@ EXPORT_SYMBOL(ttm_global_swapout); int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { + struct ttm_resource_cursor cursor; struct ttm_resource_manager *man; - struct ttm_buffer_object *bo; struct ttm_resource *res; - unsigned i, j; + unsigned i; int ret; spin_lock(&bdev->lru_lock); @@ -154,20 +154,16 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx, if (!man || !man->use_tt) continue; - for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) { - list_for_each_entry(res, &man->lru[j], lru) { - uint32_t num_pages; - - bo = res->bo; - num_pages = PFN_UP(bo->base.size); + ttm_resource_manager_for_each_res(man, &cursor, res) { + struct ttm_buffer_object *bo = res->bo; + uint32_t num_pages = PFN_UP(bo->base.size); - ret = ttm_bo_swapout(bo, ctx, gfp_flags); - /* ttm_bo_swapout has dropped the lru_lock */ - if (!ret) - return num_pages; - if (ret != -EBUSY) - return ret; - } + ret = ttm_bo_swapout(bo, ctx, gfp_flags); + /* ttm_bo_swapout has dropped the lru_lock */ + if (!ret) + return num_pages; + if (ret != -EBUSY) + return ret; } } spin_unlock(&bdev->lru_lock); diff --git a/dr
[PATCH 05/11] drm/radeon: remove resource accounting
Use the one provided by TTM instead. Signed-off-by: Christian König Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/radeon/radeon.h| 2 -- drivers/gpu/drm/radeon/radeon_kms.c| 7 -- drivers/gpu/drm/radeon/radeon_object.c | 30 +++--- drivers/gpu/drm/radeon/radeon_object.h | 1 - drivers/gpu/drm/radeon/radeon_ttm.c| 18 ++-- 5 files changed, 10 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 895776c421d4..08f83bf2c330 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -2462,8 +2462,6 @@ struct radeon_device { struct radeon_vm_managervm_manager; struct mutexgpu_clock_mutex; /* memory stats */ - atomic64_t vram_usage; - atomic64_t gtt_usage; atomic64_t num_bytes_moved; atomic_tgpu_reset_counter; /* ACPI interface */ diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index 11ad210919c8..965161b8565b 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -241,6 +241,7 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct drm_radeon_info *info = data; struct radeon_mode_info *minfo = &rdev->mode_info; uint32_t *value, value_tmp, *value_ptr, value_size; + struct ttm_resource_manager *man; uint64_t value64; struct drm_crtc *crtc; int i, found; @@ -550,12 +551,14 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) case RADEON_INFO_VRAM_USAGE: value = (uint32_t*)&value64; value_size = sizeof(uint64_t); - value64 = atomic64_read(&rdev->vram_usage); + man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM); + value64 = ttm_resource_manager_usage(man); break; case RADEON_INFO_GTT_USAGE: value = (uint32_t*)&value64; value_size = sizeof(uint64_t); - value64 = atomic64_read(&rdev->gtt_usage); + man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_TT); + value64 = ttm_resource_manager_usage(man); break; case RADEON_INFO_ACTIVE_CU_COUNT: if (rdev->family >= CHIP_BONAIRE) diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 56ede9d63b12..c9bbed2a25ad 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -49,27 +49,6 @@ static void radeon_bo_clear_surface_reg(struct radeon_bo *bo); * function are calling it. */ -static void radeon_update_memory_usage(struct ttm_buffer_object *bo, - unsigned int mem_type, int sign) -{ - struct radeon_device *rdev = radeon_get_rdev(bo->bdev); - - switch (mem_type) { - case TTM_PL_TT: - if (sign > 0) - atomic64_add(bo->base.size, &rdev->gtt_usage); - else - atomic64_sub(bo->base.size, &rdev->gtt_usage); - break; - case TTM_PL_VRAM: - if (sign > 0) - atomic64_add(bo->base.size, &rdev->vram_usage); - else - atomic64_sub(bo->base.size, &rdev->vram_usage); - break; - } -} - static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo) { struct radeon_bo *bo; @@ -434,7 +413,9 @@ void radeon_bo_fini(struct radeon_device *rdev) static u64 radeon_bo_get_threshold_for_moves(struct radeon_device *rdev) { u64 real_vram_size = rdev->mc.real_vram_size; - u64 vram_usage = atomic64_read(&rdev->vram_usage); + struct ttm_resource_manager *man = + ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM); + u64 vram_usage = ttm_resource_manager_usage(man); /* This function is based on the current VRAM usage. * @@ -725,15 +706,10 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved, } void radeon_bo_move_notify(struct ttm_buffer_object *bo, - unsigned int old_type, struct ttm_resource *new_mem) { struct radeon_bo *rbo; - radeon_update_memory_usage(bo, old_type, -1); - if (new_mem) - radeon_update_memory_usage(bo, new_mem->mem_type, 1); - if (!radeon_ttm_bo_is_radeon_bo(bo)) return; diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index 1afc7992ef91..0b64e202577b 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -161,7 +161,6 @@ extern void radeon_bo_get_ti
[PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
It makes sense to have this in the common manager for debugging and accounting of how much resources are used. v2: cleanup kerneldoc a bit v3: drop the atomic, update counter under lock instead Signed-off-by: Christian König Reviewed-by: Huang Rui (v1) Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/ttm/ttm_resource.c | 30 ++ include/drm/ttm/ttm_resource.h | 11 +-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index ae40e144e728..bbb8a0f7aa14 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo, const struct ttm_place *place, struct ttm_resource *res) { + struct ttm_resource_manager *man; + res->start = 0; res->num_pages = PFN_UP(bo->base.size); res->mem_type = place->mem_type; @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo, res->bus.is_iomem = false; res->bus.caching = ttm_cached; res->bo = bo; + + man = ttm_manager_type(bo->bdev, place->mem_type); + spin_lock(&bo->bdev->lru_lock); + man->usage += bo->base.size; + spin_unlock(&bo->bdev->lru_lock); } EXPORT_SYMBOL(ttm_resource_init); @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init); void ttm_resource_fini(struct ttm_resource_manager *man, struct ttm_resource *res) { + spin_lock(&man->bdev->lru_lock); + man->usage -= res->bo->base.size; + spin_unlock(&man->bdev->lru_lock); } EXPORT_SYMBOL(ttm_resource_fini); @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man, spin_lock_init(&man->move_lock); man->bdev = bdev; man->size = size; + man->usage = 0; for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) INIT_LIST_HEAD(&man->lru[i]); @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev, } EXPORT_SYMBOL(ttm_resource_manager_evict_all); +/** + * ttm_resource_manager_usage + * + * @man: A memory manager object. + * + * Return how many resources are currently used. + */ +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man) +{ + uint64_t usage; + + spin_lock(&man->bdev->lru_lock); + usage = man->usage; + spin_unlock(&man->bdev->lru_lock); + return usage; +} +EXPORT_SYMBOL(ttm_resource_manager_usage); + /** * ttm_resource_manager_debug * @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man, drm_printf(p, " use_type: %d\n", man->use_type); drm_printf(p, " use_tt: %d\n", man->use_tt); drm_printf(p, " size: %llu\n", man->size); + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man)); if (man->func->debug) man->func->debug(man, p); } diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index 555a11fb8a7f..323c14a30c6b 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -130,10 +131,15 @@ struct ttm_resource_manager { struct dma_fence *move; /* -* Protected by the global->lru_lock. +* Protected by the bdev->lru_lock. */ - struct list_head lru[TTM_MAX_BO_PRIORITY]; + + /** +* @usage: How much of the resources are used, protected by the +* bdev->lru_lock. +*/ + uint64_t usage; }; /** @@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man, int ttm_resource_manager_evict_all(struct ttm_device *bdev, struct ttm_resource_manager *man); +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man); void ttm_resource_manager_debug(struct ttm_resource_manager *man, struct drm_printer *p); -- 2.25.1
[PATCH 06/11] drm/amdgpu: remove GTT accounting v2
This is provided by TTM now. Also switch man->size to bytes instead of pages and fix the double printing of size and usage in debugfs. v2: fix size checking as well Signed-off-by: Christian König Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 + drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 - 4 files changed, 16 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index e0c7fbe01d93..3bcd27ae379d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev, struct ttm_resource_manager *man; man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT); - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE); + return sysfs_emit(buf, "%llu\n", man->size); } /** @@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev, { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = drm_to_adev(ddev); + struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager; - return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr)); + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man)); } static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO, @@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, struct amdgpu_gtt_node *node; int r; - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && - atomic64_add_return(num_pages, &mgr->used) > man->size) { - atomic64_sub(num_pages, &mgr->used); - return -ENOSPC; - } - node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL); - if (!node) { - r = -ENOMEM; - goto err_out; - } + if (!node) + return -ENOMEM; node->tbo = tbo; ttm_resource_init(tbo, place, &node->base.base); + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && + ttm_resource_manager_usage(man) > man->size) { + r = -ENOSPC; + goto err_free; + } if (place->lpfn) { spin_lock(&mgr->lock); @@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, err_free: ttm_resource_fini(man, &node->base.base); kfree(node); - -err_out: - if (!(place->flags & TTM_PL_FLAG_TEMPORARY)) - atomic64_sub(num_pages, &mgr->used); - return r; } @@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, drm_mm_remove_node(&node->base.mm_nodes[0]); spin_unlock(&mgr->lock); - if (!(res->placement & TTM_PL_FLAG_TEMPORARY)) - atomic64_sub(res->num_pages, &mgr->used); - ttm_resource_fini(man, res); kfree(node); } -/** - * amdgpu_gtt_mgr_usage - return usage of GTT domain - * - * @mgr: amdgpu_gtt_mgr pointer - * - * Return how many bytes are used in the GTT domain - */ -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr) -{ - return atomic64_read(&mgr->used) * PAGE_SIZE; -} - /** * amdgpu_gtt_mgr_recover - re-init gart * @@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man, spin_lock(&mgr->lock); drm_mm_print(&mgr->mm, printer); spin_unlock(&mgr->lock); - - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n", - man->size, atomic64_read(&mgr->used)); } static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = { @@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size) man->use_tt = true; man->func = &amdgpu_gtt_mgr_func; - ttm_resource_manager_init(man, &adev->mman.bdev, - gtt_size >> PAGE_SHIFT); + ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size); start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS; size = (adev->gmc.gart_size >> PAGE_SHIFT) - start; drm_mm_init(&mgr->mm, start, size); spin_lock_init(&mgr->lock); - atomic64_set(&mgr->used, 0); ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager); ttm_resource_manager_set_used(man, true); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 1ebb91db2274..9ff4aced5da7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -684,7 +684,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) ui64 = amdgpu_vram_mgr_vi
[PATCH 03/11] drm/ttm: move the LRU into resource handling v3
This way we finally fix the problem that new resource are not immediately evict-able after allocation. That has caused numerous problems including OOM on GDS handling and not being able to use TTM as general resource manager. v2: stop assuming in ttm_resource_fini that res->bo is still valid. v3: cleanup kerneldoc, add more lockdep annotation Signed-off-by: Christian König Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c| 115 ++ drivers/gpu/drm/ttm/ttm_bo_util.c | 1 - drivers/gpu/drm/ttm/ttm_device.c| 64 ++--- drivers/gpu/drm/ttm/ttm_resource.c | 122 +++- include/drm/ttm/ttm_bo_api.h| 16 include/drm/ttm/ttm_bo_driver.h | 29 +- include/drm/ttm/ttm_resource.h | 35 +++ 9 files changed, 197 insertions(+), 195 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index b37fc7d7d2c7..f2ce5a0defd9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (vm->bulk_moveable) { spin_lock(&adev->mman.bdev.lru_lock); - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move); + ttm_lru_bulk_move_tail(&vm->lru_bulk_move); spin_unlock(&adev->mman.bdev.lru_lock); return; } - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move)); + ttm_lru_bulk_move_init(&vm->lru_bulk_move); spin_lock(&adev->mman.bdev.lru_lock); list_for_each_entry(bo_base, &vm->idle, vm_status) { @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (!bo->parent) continue; - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource, - &vm->lru_bulk_move); + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move); if (shadow) ttm_bo_move_to_lru_tail(&shadow->tbo, - shadow->tbo.resource, &vm->lru_bulk_move); } spin_unlock(&adev->mman.bdev.lru_lock); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index de3fe79b665a..582e8dc9bc8c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -849,7 +849,7 @@ void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj) bo->priority = I915_TTM_PRIO_NO_PAGES; } - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL); + ttm_bo_move_to_lru_tail(bo, NULL); spin_unlock(&bo->bdev->lru_lock); } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index db3dc7ef5382..cb0fa932d495 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,108 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo) -{ - struct ttm_device *bdev = bo->bdev; - - list_move_tail(&bo->lru, &bdev->pinned); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); -} - -static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) -{ - struct ttm_device *bdev = bo->bdev; - - list_del_init(&bo->lru); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); -} - -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos, -struct ttm_buffer_object *bo) -{ - if (!pos->first) - pos->first = bo; - pos->last = bo; -} - void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, -struct ttm_resource *mem, struct ttm_lru_bulk_move *bulk) { - struct ttm_device *bdev = bo->bdev; - struct ttm_resource_manager *man; - - if (!bo->deleted) - dma_resv_assert_held(bo->base.resv); - - if (bo->pin_count) { - ttm_bo_move_to_pinned(bo); - return; - } - - if (!mem) - return; - - man = ttm_manager_type(bdev, mem->mem_type); - list_move_tail(&bo->lru, &man->lru[bo->priority]); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); - - if (bulk && !bo->pin_count) { - switch (bo->resource->mem_type) { - case TTM_PL_TT: - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo); - break; + dma_resv_
[PATCH 09/11] drm/amdgpu: drop amdgpu_gtt_node
We have the BO pointer in the base structure now as well. Signed-off-by: Christian König Reviewed-by: Daniel Vetter Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 - include/drm/ttm/ttm_resource.h | 8 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 3bcd27ae379d..68494b959116 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -26,23 +26,12 @@ #include "amdgpu.h" -struct amdgpu_gtt_node { - struct ttm_buffer_object *tbo; - struct ttm_range_mgr_node base; -}; - static inline struct amdgpu_gtt_mgr * to_gtt_mgr(struct ttm_resource_manager *man) { return container_of(man, struct amdgpu_gtt_mgr, manager); } -static inline struct amdgpu_gtt_node * -to_amdgpu_gtt_node(struct ttm_resource *res) -{ - return container_of(res, struct amdgpu_gtt_node, base.base); -} - /** * DOC: mem_info_gtt_total * @@ -106,9 +95,9 @@ const struct attribute_group amdgpu_gtt_mgr_attr_group = { */ bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *res) { - struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res); + struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res); - return drm_mm_node_allocated(&node->base.mm_nodes[0]); + return drm_mm_node_allocated(&node->mm_nodes[0]); } /** @@ -128,15 +117,14 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, { struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man); uint32_t num_pages = PFN_UP(tbo->base.size); - struct amdgpu_gtt_node *node; + struct ttm_range_mgr_node *node; int r; - node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL); + node = kzalloc(struct_size(node, mm_nodes, 1), GFP_KERNEL); if (!node) return -ENOMEM; - node->tbo = tbo; - ttm_resource_init(tbo, place, &node->base.base); + ttm_resource_init(tbo, place, &node->base); if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && ttm_resource_manager_usage(man) > man->size) { r = -ENOSPC; @@ -145,8 +133,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, if (place->lpfn) { spin_lock(&mgr->lock); - r = drm_mm_insert_node_in_range(&mgr->mm, - &node->base.mm_nodes[0], + r = drm_mm_insert_node_in_range(&mgr->mm, &node->mm_nodes[0], num_pages, tbo->page_alignment, 0, place->fpfn, place->lpfn, DRM_MM_INSERT_BEST); @@ -154,18 +141,18 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, if (unlikely(r)) goto err_free; - node->base.base.start = node->base.mm_nodes[0].start; + node->base.start = node->mm_nodes[0].start; } else { - node->base.mm_nodes[0].start = 0; - node->base.mm_nodes[0].size = node->base.base.num_pages; - node->base.base.start = AMDGPU_BO_INVALID_OFFSET; + node->mm_nodes[0].start = 0; + node->mm_nodes[0].size = node->base.num_pages; + node->base.start = AMDGPU_BO_INVALID_OFFSET; } - *res = &node->base.base; + *res = &node->base; return 0; err_free: - ttm_resource_fini(man, &node->base.base); + ttm_resource_fini(man, &node->base); kfree(node); return r; } @@ -181,12 +168,12 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, struct ttm_resource *res) { - struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res); + struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res); struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man); spin_lock(&mgr->lock); - if (drm_mm_node_allocated(&node->base.mm_nodes[0])) - drm_mm_remove_node(&node->base.mm_nodes[0]); + if (drm_mm_node_allocated(&node->mm_nodes[0])) + drm_mm_remove_node(&node->mm_nodes[0]); spin_unlock(&mgr->lock); ttm_resource_fini(man, res); @@ -202,7 +189,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, */ int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr) { - struct amdgpu_gtt_node *node; + struct ttm_range_mgr_node *node; struct drm_mm_node *mm_node; struct amdgpu_device *adev; int r = 0; @@ -210,8 +197,8 @@ int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr) adev = container_of(mgr, typeof(*adev), mman.gtt_mgr); spin_lock(&mgr->lock);
[PATCH 08/11] drm/amdgpu: remove VRAM accounting v2
This is provided by TTM now. Also switch man->size to bytes instead of pages and fix the double printing of size and usage in debugfs. v2: fix size checking as well Signed-off-by: Christian König Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 - drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 58 +++- 7 files changed, 32 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index e8440d306496..025748e9c772 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -314,7 +314,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev, } total_vram = adev->gmc.real_vram_size - atomic64_read(&adev->vram_pin_size); - used_vram = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr); + used_vram = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager); free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram; spin_lock(&adev->mm_stats.lock); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 9ff4aced5da7..0beab961b18b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -678,7 +678,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) ui64 = atomic64_read(&adev->num_vram_cpu_page_faults); return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0; case AMDGPU_INFO_VRAM_USAGE: - ui64 = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr); + ui64 = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager); return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0; case AMDGPU_INFO_VIS_VRAM_USAGE: ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr); @@ -717,6 +717,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct drm_amdgpu_memory_info mem; struct ttm_resource_manager *gtt_man = &adev->mman.gtt_mgr.manager; + struct ttm_resource_manager *vram_man = + &adev->mman.vram_mgr.manager; memset(&mem, 0, sizeof(mem)); mem.vram.total_heap_size = adev->gmc.real_vram_size; @@ -724,7 +726,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) atomic64_read(&adev->vram_pin_size) - AMDGPU_VM_RESERVED_VRAM; mem.vram.heap_usage = - amdgpu_vram_mgr_usage(&adev->mman.vram_mgr); + ttm_resource_manager_usage(vram_man); mem.vram.max_allocation = mem.vram.usable_heap_size * 3 / 4; mem.cpu_accessible_vram.total_heap_size = diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 514754142f69..ea0cde4904f0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -460,7 +460,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev, if (domain & AMDGPU_GEM_DOMAIN_VRAM) { man = ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM); - if (size < (man->size << PAGE_SHIFT)) + if (size < man->size) return true; else goto fail; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index d178fbec7048..5859ed0552a4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1884,7 +1884,7 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable) size = adev->gmc.real_vram_size; else size = adev->gmc.visible_vram_size; - man->size = size >> PAGE_SHIFT; + man->size = size; adev->mman.buffer_funcs_enabled = enable; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index 4e8577dad16a..58c64871c94a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -44,7 +44,6 @@ struct amdgpu_vram_mgr { spinlock_t lock; struct list_head reservations_pending; struct list_head reserved_pages; - atomic64_t usage; atomic64_t vis_usage; }; @@ -122,7 +121,6 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev, void amdgpu_vram_mgr_free_sgt(struct device *dev,
[PATCH 10/11] drm/ttm: allow bulk moves for all domains
Not just TT and VRAM. Signed-off-by: Christian König Reviewed-by: Daniel Vetter Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/ttm/ttm_resource.c | 52 +- include/drm/ttm/ttm_device.h | 2 -- include/drm/ttm/ttm_resource.h | 4 +-- 3 files changed, 17 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 1f9b8205b3a5..9df8fcc7e479 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -51,38 +51,24 @@ EXPORT_SYMBOL(ttm_lru_bulk_move_init); */ void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk) { - unsigned i; - - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { - struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i]; - struct ttm_resource_manager *man; + unsigned i, j; - if (!pos->first) - continue; + for (i = 0; i < TTM_NUM_MEM_TYPES; ++i) { + for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) { + struct ttm_lru_bulk_move_pos *pos = &bulk->pos[i][j]; + struct ttm_resource_manager *man; - lockdep_assert_held(&pos->first->bo->bdev->lru_lock); - dma_resv_assert_held(pos->first->bo->base.resv); - dma_resv_assert_held(pos->last->bo->base.resv); + if (!pos->first) + continue; - man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT); - list_bulk_move_tail(&man->lru[i], &pos->first->lru, - &pos->last->lru); - } - - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { - struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i]; - struct ttm_resource_manager *man; + lockdep_assert_held(&pos->first->bo->bdev->lru_lock); + dma_resv_assert_held(pos->first->bo->base.resv); + dma_resv_assert_held(pos->last->bo->base.resv); - if (!pos->first) - continue; - - lockdep_assert_held(&pos->first->bo->bdev->lru_lock); - dma_resv_assert_held(pos->first->bo->base.resv); - dma_resv_assert_held(pos->last->bo->base.resv); - - man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM); - list_bulk_move_tail(&man->lru[i], &pos->first->lru, - &pos->last->lru); + man = ttm_manager_type(pos->first->bo->bdev, i); + list_bulk_move_tail(&man->lru[j], &pos->first->lru, + &pos->last->lru); + } } } EXPORT_SYMBOL(ttm_lru_bulk_move_tail); @@ -122,15 +108,7 @@ void ttm_resource_move_to_lru_tail(struct ttm_resource *res, if (!bulk) return; - switch (res->mem_type) { - case TTM_PL_TT: - ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res); - break; - - case TTM_PL_VRAM: - ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res); - break; - } + ttm_lru_bulk_move_set_pos(&bulk->pos[res->mem_type][bo->priority], res); } /** diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index 0a4ddec78d8f..425150f35fbe 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -30,8 +30,6 @@ #include #include -#define TTM_NUM_MEM_TYPES 8 - struct ttm_device; struct ttm_placement; struct ttm_buffer_object; diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index 8a780a348025..990ed0b289a2 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -37,6 +37,7 @@ #include #define TTM_MAX_BO_PRIORITY4U +#define TTM_NUM_MEM_TYPES 8 struct ttm_device; struct ttm_resource_manager; @@ -217,8 +218,7 @@ struct ttm_lru_bulk_move_pos { * Helper structure for bulk moves on the LRU list. */ struct ttm_lru_bulk_move { - struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY]; - struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY]; + struct ttm_lru_bulk_move_pos pos[TTM_NUM_MEM_TYPES][TTM_MAX_BO_PRIORITY]; }; /** -- 2.25.1
[PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting
This is provided by TTM now. Signed-off-by: Christian König --- .../gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 62 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +-- 2 files changed, 6 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c index 0d85c2096ab5..e8adfd0a570a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c @@ -25,12 +25,6 @@ #include "amdgpu.h" -static inline struct amdgpu_preempt_mgr * -to_preempt_mgr(struct ttm_resource_manager *man) -{ - return container_of(man, struct amdgpu_preempt_mgr, manager); -} - /** * DOC: mem_info_preempt_used * @@ -45,10 +39,9 @@ static ssize_t mem_info_preempt_used_show(struct device *dev, { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = drm_to_adev(ddev); - struct ttm_resource_manager *man; + struct ttm_resource_manager *man = &adev->mman.preempt_mgr; - man = ttm_manager_type(&adev->mman.bdev, AMDGPU_PL_PREEMPT); - return sysfs_emit(buf, "%llu\n", amdgpu_preempt_mgr_usage(man)); + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man)); } static DEVICE_ATTR_RO(mem_info_preempt_used); @@ -68,16 +61,12 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager *man, const struct ttm_place *place, struct ttm_resource **res) { - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); - *res = kzalloc(sizeof(**res), GFP_KERNEL); if (!*res) return -ENOMEM; ttm_resource_init(tbo, place, *res); (*res)->start = AMDGPU_BO_INVALID_OFFSET; - - atomic64_add((*res)->num_pages, &mgr->used); return 0; } @@ -92,49 +81,13 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager *man, static void amdgpu_preempt_mgr_del(struct ttm_resource_manager *man, struct ttm_resource *res) { - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); - - atomic64_sub(res->num_pages, &mgr->used); ttm_resource_fini(man, res); kfree(res); } -/** - * amdgpu_preempt_mgr_usage - return usage of PREEMPT domain - * - * @man: TTM memory type manager - * - * Return how many bytes are used in the GTT domain - */ -uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man) -{ - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); - s64 result = atomic64_read(&mgr->used); - - return (result > 0 ? result : 0) * PAGE_SIZE; -} - -/** - * amdgpu_preempt_mgr_debug - dump VRAM table - * - * @man: TTM memory type manager - * @printer: DRM printer to use - * - * Dump the table content using printk. - */ -static void amdgpu_preempt_mgr_debug(struct ttm_resource_manager *man, -struct drm_printer *printer) -{ - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); - - drm_printf(printer, "man size:%llu pages, preempt used:%lld pages\n", - man->size, (u64)atomic64_read(&mgr->used)); -} - static const struct ttm_resource_manager_func amdgpu_preempt_mgr_func = { .alloc = amdgpu_preempt_mgr_new, .free = amdgpu_preempt_mgr_del, - .debug = amdgpu_preempt_mgr_debug }; /** @@ -146,8 +99,7 @@ static const struct ttm_resource_manager_func amdgpu_preempt_mgr_func = { */ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev) { - struct amdgpu_preempt_mgr *mgr = &adev->mman.preempt_mgr; - struct ttm_resource_manager *man = &mgr->manager; + struct ttm_resource_manager *man = &adev->mman.preempt_mgr; int ret; man->use_tt = true; @@ -155,16 +107,13 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev) ttm_resource_manager_init(man, &adev->mman.bdev, (1 << 30)); - atomic64_set(&mgr->used, 0); - ret = device_create_file(adev->dev, &dev_attr_mem_info_preempt_used); if (ret) { DRM_ERROR("Failed to create device file mem_info_preempt_used\n"); return ret; } - ttm_set_driver_manager(&adev->mman.bdev, AMDGPU_PL_PREEMPT, - &mgr->manager); + ttm_set_driver_manager(&adev->mman.bdev, AMDGPU_PL_PREEMPT, man); ttm_resource_manager_set_used(man, true); return 0; } @@ -179,8 +128,7 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev) */ void amdgpu_preempt_mgr_fini(struct amdgpu_device *adev) { - struct amdgpu_preempt_mgr *mgr = &adev->mman.preempt_mgr; - struct ttm_resource_manager *man = &mgr->manager; + struct ttm_resource_manager *man = &adev->mman.preempt_mgr; int ret; ttm_resource_manager_set_used(man, false); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdg
[PATCH 11/11] drm/ttm: rework bulk move handling v2
Instead of providing the bulk move structure for each LRU update set this as property of the BO. This should avoid costly bulk move rebuilds with some games under RADV. v2: some name polishing, add a few more kerneldoc words. v3: add some lockdep Signed-off-by: Christian König Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 72 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 - drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c| 47 +++-- drivers/gpu/drm/ttm/ttm_resource.c | 87 ++--- include/drm/ttm/ttm_bo_api.h| 16 ++--- include/drm/ttm/ttm_bo_driver.h | 2 +- include/drm/ttm/ttm_device.h| 9 --- include/drm/ttm/ttm_resource.h | 9 ++- 10 files changed, 128 insertions(+), 120 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 5859ed0552a4..57ac118fc266 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1498,7 +1498,6 @@ static struct ttm_device_funcs amdgpu_bo_driver = { .io_mem_reserve = &amdgpu_ttm_io_mem_reserve, .io_mem_pfn = amdgpu_ttm_io_mem_pfn, .access_memory = &amdgpu_ttm_access_memory, - .del_from_lru_notify = &amdgpu_vm_del_from_lru_notify }; /* diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index f2ce5a0defd9..28f5e8b21a99 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -375,7 +375,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base, if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv) return; - vm->bulk_moveable = false; + ttm_bo_set_bulk_move(&bo->tbo, &vm->lru_bulk_move); if (bo->tbo.type == ttm_bo_type_kernel && bo->parent) amdgpu_vm_bo_relocated(base); else @@ -637,36 +637,6 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm, list_add(&entry->tv.head, validated); } -/** - * amdgpu_vm_del_from_lru_notify - update bulk_moveable flag - * - * @bo: BO which was removed from the LRU - * - * Make sure the bulk_moveable flag is updated when a BO is removed from the - * LRU. - */ -void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo) -{ - struct amdgpu_bo *abo; - struct amdgpu_vm_bo_base *bo_base; - - if (!amdgpu_bo_is_amdgpu_bo(bo)) - return; - - if (bo->pin_count) - return; - - abo = ttm_to_amdgpu_bo(bo); - if (!abo->parent) - return; - for (bo_base = abo->vm_bo; bo_base; bo_base = bo_base->next) { - struct amdgpu_vm *vm = bo_base->vm; - - if (abo->tbo.base.resv == vm->root.bo->tbo.base.resv) - vm->bulk_moveable = false; - } - -} /** * amdgpu_vm_move_to_lru_tail - move all BOs to the end of LRU * @@ -679,33 +649,9 @@ void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo) void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, struct amdgpu_vm *vm) { - struct amdgpu_vm_bo_base *bo_base; - - if (vm->bulk_moveable) { - spin_lock(&adev->mman.bdev.lru_lock); - ttm_lru_bulk_move_tail(&vm->lru_bulk_move); - spin_unlock(&adev->mman.bdev.lru_lock); - return; - } - - ttm_lru_bulk_move_init(&vm->lru_bulk_move); - spin_lock(&adev->mman.bdev.lru_lock); - list_for_each_entry(bo_base, &vm->idle, vm_status) { - struct amdgpu_bo *bo = bo_base->bo; - struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo); - - if (!bo->parent) - continue; - - ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move); - if (shadow) - ttm_bo_move_to_lru_tail(&shadow->tbo, - &vm->lru_bulk_move); - } + ttm_lru_bulk_move_tail(&vm->lru_bulk_move); spin_unlock(&adev->mman.bdev.lru_lock); - - vm->bulk_moveable = true; } /** @@ -728,8 +674,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_vm_bo_base *bo_base, *tmp; int r; - vm->bulk_moveable &= list_empty(&vm->evicted); - list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) { struct amdgpu_bo *bo = bo_base->bo; struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo); @@ -1047,10 +991,16 @@ static void amdgpu_vm_free_table(struct amdgpu_vm_bo_base *entry) if (!entry->bo) return; + shadow = amdgpu_bo_shadowed(entry->bo); + if (shadow) { + ttm_bo_set_bulk_move(&shadow->tbo, NULL); +
Re: [PATCH 01/11] drm/ttm: fix resource manager size type and description
Hi guys, crap I once more forgot the cover letter, but this set should be pretty straight forward by now. Please review and comment, Christian. Am 14.02.22 um 10:34 schrieb Christian König: That are not pages any more. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_resource.c | 6 +++--- include/drm/ttm/ttm_resource.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 68344c90549b..ae40e144e728 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -153,19 +153,19 @@ void ttm_resource_set_bo(struct ttm_resource *res, * * @man: memory manager object to init * @bdev: ttm device this manager belongs to - * @p_size: size managed area in pages. + * @size: size of managed resources in arbitary units * * Initialise core parts of a manager object. */ void ttm_resource_manager_init(struct ttm_resource_manager *man, struct ttm_device *bdev, - unsigned long p_size) + uint64_t size) { unsigned i; spin_lock_init(&man->move_lock); man->bdev = bdev; - man->size = p_size; + man->size = size; for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) INIT_LIST_HEAD(&man->lru[i]); diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index 69eea9d6399b..555a11fb8a7f 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -278,7 +278,7 @@ void ttm_resource_set_bo(struct ttm_resource *res, void ttm_resource_manager_init(struct ttm_resource_manager *man, struct ttm_device *bdev, - unsigned long p_size); + uint64_t size); int ttm_resource_manager_evict_all(struct ttm_device *bdev, struct ttm_resource_manager *man);
[PATCH 5.10 065/116] drm/panel: simple: Assign data from panel_dpi_probe() correctly
From: Christoph Niedermaier [ Upstream commit 6df4432a5eca101b5fd80fbee41d309f3d67928d ] In the function panel_simple_probe() the pointer panel->desc is assigned to the passed pointer desc. If function panel_dpi_probe() is called panel->desc will be updated, but further on only desc will be evaluated. So update the desc pointer to be able to use the data from the function panel_dpi_probe(). Fixes: 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") Signed-off-by: Christoph Niedermaier Cc: Marek Vasut Cc: Thierry Reding Cc: Sam Ravnborg Cc: David Airlie Cc: Daniel Vetter To: dri-devel@lists.freedesktop.org Reviewed-by: Sam Ravnborg Signed-off-by: Marek Vasut Link: https://patchwork.freedesktop.org/patch/msgid/20220201110153.3479-1-cniederma...@dh-electronics.com Signed-off-by: Sasha Levin --- drivers/gpu/drm/panel/panel-simple.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 204674fccd646..7ffd2a04ab23a 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -557,6 +557,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) err = panel_dpi_probe(dev, panel); if (err) goto free_ddc; + desc = panel->desc; } else { if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) panel_simple_parse_panel_timing_node(dev, panel, &dt); -- 2.34.1
Re: [PATCH v4] drm/mediatek: allow commands to be sent during video mode
Il 14/02/22 10:27, Julien STEPHAN ha scritto: Mipi dsi panel drivers can use mipi_dsi_dcs_{set,get}_display_brightness() to request backlight changes. This can be done during panel initialization (dsi is in command mode) or afterwards (dsi is in Video Mode). When the DSI is in Video Mode, all commands are rejected. Detect current DSI mode in mtk_dsi_host_transfer() and switch modes temporarily to allow commands to be sent. Signed-off-by: Julien STEPHAN Signed-off-by: Mattijs Korpershoek Please, next time, don't drop the tags that reviewers are giving to you, unless the patch changes radically. Reviewed-by: AngeloGioacchino Del Regno --- Changes in v4: - fix missing space: "ret : recv_cnt;" Changes in v3: - increase readability of code and use correct return variable (see comment https://lore.kernel.org/linux-mediatek/4907bdc1-b4a6-e9ad-5cfa-266fc20c0...@collabora.com/) Changes in v2: - update commit message to be more descriptive drivers/gpu/drm/mediatek/mtk_dsi.c | 33 ++ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 5d90d2eb0019..abdd9cdebd86 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -891,24 +891,33 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, u8 read_data[16]; void *src_addr; u8 irq_flag = CMD_DONE_INT_FLAG; + u32 dsi_mode; + int ret; - if (readl(dsi->regs + DSI_MODE_CTRL) & MODE) { - DRM_ERROR("dsi engine is not command mode\n"); - return -EINVAL; + dsi_mode = readl(dsi->regs + DSI_MODE_CTRL); + if (dsi_mode & MODE) { + mtk_dsi_stop(dsi); + ret = mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); + if (ret) + goto restore_dsi_mode; } if (MTK_DSI_HOST_IS_READ(msg->type)) irq_flag |= LPRX_RD_RDY_INT_FLAG; - if (mtk_dsi_host_send_cmd(dsi, msg, irq_flag) < 0) - return -ETIME; + ret = mtk_dsi_host_send_cmd(dsi, msg, irq_flag); + if (ret) + goto restore_dsi_mode; - if (!MTK_DSI_HOST_IS_READ(msg->type)) - return 0; + if (!MTK_DSI_HOST_IS_READ(msg->type)) { + recv_cnt = 0; + goto restore_dsi_mode; + } if (!msg->rx_buf) { DRM_ERROR("dsi receive buffer size may be NULL\n"); - return -EINVAL; + ret = -EINVAL; + goto restore_dsi_mode; } for (i = 0; i < 16; i++) @@ -933,7 +942,13 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, DRM_INFO("dsi get %d byte data from the panel address(0x%x)\n", recv_cnt, *((u8 *)(msg->tx_buf))); - return recv_cnt; +restore_dsi_mode: + if (dsi_mode & MODE) { + mtk_dsi_set_mode(dsi); + mtk_dsi_start(dsi); + } + + return ret < 0 ? ret : recv_cnt; } static const struct mipi_dsi_host_ops mtk_dsi_ops = { -- 2.35.1
Re: [PATCH v4 0/10] clk: Improve clock range handling
Hi Maxime and Stephen, We have recently posted a driver for the BCM2711 Unicam CSI-2 receiver (see [1]) which is a perfect candidate for this API, as it needs a minimum rate for the VPU clock. Any chance we can get this series merged ? :-) [1] https://lore.kernel.org/linux-media/20220208155027.891055-1-jeanmichel.hautb...@ideasonboard.com On Tue, Jan 25, 2022 at 03:15:39PM +0100, Maxime Ripard wrote: > Hi, > > This is a follow-up of the discussion here: > https://lore.kernel.org/linux-clk/20210319150355.xzw7ikwdaga2dwhv@gilmour/ > > and here: > https://lore.kernel.org/all/20210914093515.260031-1-max...@cerno.tech/ > > While the initial proposal implemented a new API to temporarily raise and > lower > clock rates based on consumer workloads, Stephen suggested an > alternative approach implemented here. > > The main issue that needed to be addressed in our case was that in a > situation where we would have multiple calls to clk_set_rate_range, we > would end up with a clock at the maximum of the minimums being set. This > would be expected, but the issue was that if one of the users was to > relax or drop its requirements, the rate would be left unchanged, even > though the ideal rate would have changed. > > So something like > > clk_set_rate(user1_clk, 1000); > clk_set_min_rate(user1_clk, 2000); > clk_set_min_rate(user2_clk, 3000); > clk_set_min_rate(user2_clk, 1000); > > Would leave the clock running at 3000Hz, while the minimum would now be > 2000Hz. > > This was mostly due to the fact that the core only triggers a rate > change in clk_set_rate_range() if the current rate is outside of the > boundaries, but not if it's within the new boundaries. > > That series changes that and will trigger a rate change on every call, > with the former rate being tried again. This way, providers have a > chance to follow whatever policy they see fit for a given clock each > time the boundaries change. > > This series also implements some kunit tests, first to test a few rate > related functions in the CCF, and then extends it to make sure that > behaviour has some test coverage. > > Let me know what you think > Maxime > > Changes from v3: > - Renamed the test file and Kconfig option > - Add option to .kunitconfig > - Switch to kunit_kzalloc > - Use KUNIT_EXPECT_* instead of KUNIT_ASSERT_* where relevant > - Test directly relevant calls instead of going through a temporary variable > - Switch to more precise KUNIT_ASSERT_* macros where relevant > > Changes from v2: > - Rebased on current next > - Rewrote the whole thing according to Stephen reviews > - Implemented some kunit tests > > Changes from v1: > - Return NULL in clk_request_start if clk pointer is NULL > - Test for clk_req pointer in clk_request_done > - Add another user in vc4 > - Rebased on top of v5.15-rc1 > > Maxime Ripard (10): > clk: Introduce Kunit Tests for the framework > clk: Always clamp the rounded rate > clk: Use clamp instead of open-coding our own > clk: Always set the rate on clk_set_range_rate > clk: Add clk_drop_range > clk: bcm: rpi: Add variant structure > clk: bcm: rpi: Set a default minimum rate > clk: bcm: rpi: Run some clocks at the minimum rate allowed > drm/vc4: Add logging and comments > drm/vc4: hdmi: Remove clock rate initialization > > drivers/clk/.kunitconfig | 1 + > drivers/clk/Kconfig | 7 + > drivers/clk/Makefile | 1 + > drivers/clk/bcm/clk-raspberrypi.c | 125 +- > drivers/clk/clk-test.c| 621 ++ > drivers/clk/clk.c | 51 ++- > drivers/gpu/drm/vc4/vc4_hdmi.c| 13 - > drivers/gpu/drm/vc4/vc4_kms.c | 11 + > include/linux/clk.h | 11 + > 9 files changed, 786 insertions(+), 55 deletions(-) > create mode 100644 drivers/clk/clk-test.c -- Regards, Laurent Pinchart
[PATCH 5.15 105/172] drm/panel: simple: Assign data from panel_dpi_probe() correctly
From: Christoph Niedermaier [ Upstream commit 6df4432a5eca101b5fd80fbee41d309f3d67928d ] In the function panel_simple_probe() the pointer panel->desc is assigned to the passed pointer desc. If function panel_dpi_probe() is called panel->desc will be updated, but further on only desc will be evaluated. So update the desc pointer to be able to use the data from the function panel_dpi_probe(). Fixes: 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") Signed-off-by: Christoph Niedermaier Cc: Marek Vasut Cc: Thierry Reding Cc: Sam Ravnborg Cc: David Airlie Cc: Daniel Vetter To: dri-devel@lists.freedesktop.org Reviewed-by: Sam Ravnborg Signed-off-by: Marek Vasut Link: https://patchwork.freedesktop.org/patch/msgid/20220201110153.3479-1-cniederma...@dh-electronics.com Signed-off-by: Sasha Levin --- drivers/gpu/drm/panel/panel-simple.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 9b6c4e6c38a1b..b7b654f2dfd90 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -721,6 +721,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc, err = panel_dpi_probe(dev, panel); if (err) goto free_ddc; + desc = panel->desc; } else { if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) panel_simple_parse_panel_timing_node(dev, panel, &dt); -- 2.34.1
[PATCH 5.16 125/203] drm/panel: simple: Assign data from panel_dpi_probe() correctly
From: Christoph Niedermaier [ Upstream commit 6df4432a5eca101b5fd80fbee41d309f3d67928d ] In the function panel_simple_probe() the pointer panel->desc is assigned to the passed pointer desc. If function panel_dpi_probe() is called panel->desc will be updated, but further on only desc will be evaluated. So update the desc pointer to be able to use the data from the function panel_dpi_probe(). Fixes: 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") Signed-off-by: Christoph Niedermaier Cc: Marek Vasut Cc: Thierry Reding Cc: Sam Ravnborg Cc: David Airlie Cc: Daniel Vetter To: dri-devel@lists.freedesktop.org Reviewed-by: Sam Ravnborg Signed-off-by: Marek Vasut Link: https://patchwork.freedesktop.org/patch/msgid/20220201110153.3479-1-cniederma...@dh-electronics.com Signed-off-by: Sasha Levin --- drivers/gpu/drm/panel/panel-simple.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index eb475a3a774b7..87f30bced7b7e 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -588,6 +588,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) err = panel_dpi_probe(dev, panel); if (err) goto free_ddc; + desc = panel->desc; } else { if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) panel_simple_parse_panel_timing_node(dev, panel, &dt); -- 2.34.1
[RFC v2 0/5] Add data flow metering support for HDMI2.1
The below patches add support for data flow metering as mentioned in the section 6.5.6 FRL data flow metering of HDMI 2.1 specification. Add functions to calclulate the DFM parameters for the given frl config, which is further used to evaluate the data flow metering requirement as specified in the spec. As per the spec the below patches implement the frl capacity computation functions for both compressed and uncompressed video. Finally exposing 1 function each for compressed and uncompressed video to figure out if the data flow metering requirement is met or not. v2: Changed u32 to unsigned int, corrected patch 4 to address build issue, addressed checkpatch issues, moved the drm_frl_dfm_helper under kms_helpers section for compilation in the Makefile. Ankit Nautiyal (1): drm/hdmi21: Add support for DFM calculation with DSC Vandita Kulkarni (4): drm/hdmi21: Define frl_dfm structure drm/hdmi21: Add non dsc frl capacity computation helpers drm/hdmi21: Add helpers to verify non-dsc DFM requirements drm/hdmi21: Add frl_dfm_helper to Makefile drivers/gpu/drm/Makefile | 4 +- drivers/gpu/drm/drm_frl_dfm_helper.c | 854 +++ include/drm/drm_frl_dfm_helper.h | 129 3 files changed, 986 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_frl_dfm_helper.c create mode 100644 include/drm/drm_frl_dfm_helper.h -- 2.32.0
[RFC v2 2/5] drm/hdmi21: Add non dsc frl capacity computation helpers
Add helper functions for computing non dsc frl link characteristics Signed-off-by: Vandita Kulkarni --- drivers/gpu/drm/drm_frl_dfm_helper.c | 396 +++ 1 file changed, 396 insertions(+) create mode 100644 drivers/gpu/drm/drm_frl_dfm_helper.c diff --git a/drivers/gpu/drm/drm_frl_dfm_helper.c b/drivers/gpu/drm/drm_frl_dfm_helper.c new file mode 100644 index ..d3ae35653370 --- /dev/null +++ b/drivers/gpu/drm/drm_frl_dfm_helper.c @@ -0,0 +1,396 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2022 Intel Corp + */ + +#include +#include +#include +#include + +/* Total frl charecters per super block */ +static unsigned int drm_get_frl_char_per_super_blk(unsigned int lanes) +{ + unsigned int frl_char_per_sb; + + frl_char_per_sb = (4 * FRL_CHAR_PER_CHAR_BLK) + lanes; + return frl_char_per_sb; +} + +/* + * Determine the overhead due to the inclusion of + * the SR and SSB FRL charecters used for + * super block framing + */ +static unsigned int drm_get_overhead_super_blk(unsigned int lanes) +{ + return (lanes * EFFICIENCY_MULTIPLIER) / drm_get_frl_char_per_super_blk(lanes); +} + +/* + * Determine the overhead due to the inclusion of RS FEC pairity + * symbols. Each charecter block uses 8 FRL charecters for RS Pairity + * and there are 4 charecter blocks per super block + */ +static unsigned int drm_get_overhead_rs(unsigned int lanes) +{ + return (8 * 4 * EFFICIENCY_MULTIPLIER) / drm_get_frl_char_per_super_blk(lanes); +} + +/* Determine the overhead due to FRL Map charecters. + * In a bandwidth constrained application, the FRL packets will be long, + * there will typically be two FRL Map Charecters per Super Block most of the time. + * When a tracnsition occurs between Hactive and Hblank (uncomperssed video) or + * HCactive and HCblank (compressed video transport), there may be a + * third FRL Map Charected. Therefore this spec assumes 2.5 FRL Map Charecters + * per Super Block. + */ +static unsigned int drm_get_overhead_frl_map_char(unsigned int lanes) +{ + return (25 * EFFICIENCY_MULTIPLIER) / (10 * drm_get_frl_char_per_super_blk(lanes)); +} + +/* Total minimum overhead multiplied by EFFICIENCY_MULIPLIER */ +static unsigned int drm_get_total_minimum_overhead(unsigned int lanes) +{ + unsigned int total_overhead_min; + unsigned int overhead_sb = drm_get_overhead_super_blk(lanes); + unsigned int overhead_rs = drm_get_overhead_rs(lanes); + unsigned int overhead_map = drm_get_overhead_frl_map_char(lanes); + + total_overhead_min = overhead_sb + overhead_rs + overhead_map; + + return total_overhead_min; +} + +/* + * Additional margin to the overhead is provided to account for the possibility + * of more Map Charecters, zero padding at the end of HCactive, and other minor + * items + */ +static unsigned int drm_get_max_overhead(unsigned int total_overhead_min) +{ + unsigned int total_overhead_max; + + total_overhead_max = total_overhead_min + OVERHEAD_M; + return total_overhead_max; +} + +/* Collect the link charecteristics */ + +/* Determine the maximum legal pixel rate */ +static unsigned int drm_get_max_legal_pixel_rate(unsigned int fpixel_clock_nominal_k) +{ + unsigned int fpixel_clock_max_k = (fpixel_clock_nominal_k * + (1000 + TOLERANCE_PIXEL_CLOCK)) / 1000; + return fpixel_clock_max_k; +} + +/* Determine the minimum Video Line period */ +static unsigned int drm_get_min_video_line_period(unsigned int hactive, unsigned int hblank, + unsigned int fpixel_clock_max_k) +{ + unsigned int line_time_ns; + + line_time_ns = ((hactive + hblank) * FRL_TIMING_NS_MULTIPLIER) / + fpixel_clock_max_k; + return line_time_ns; +} + +/* Determine the worst-case slow FRL Bit Rate in kbps*/ +static unsigned int drm_get_min_frl_bit_rate(unsigned int frl_bit_rate_nominal_k) +{ + unsigned int frl_bit_rate_min_k; + + frl_bit_rate_min_k = (frl_bit_rate_nominal_k / 100) * +(100 - TOLERANCE_FRL_BIT_RATE); + return frl_bit_rate_min_k; +} + +/* Determine the worst-case slow FRL Charecter Rate */ +static unsigned int drm_get_min_frl_char_rate(unsigned int frl_bit_rate_min_k) +{ + unsigned int frl_char_rate_min_k; + + frl_char_rate_min_k = frl_bit_rate_min_k / 18; + return frl_char_rate_min_k; +} + +/* Determine the Minimum Total FRL charecters per line period */ +static unsigned int +drm_get_total_frl_char_per_line_period(unsigned int line_time_ns, unsigned int frl_char_rate_min_k, + unsigned int lanes) +{ + unsigned int frl_char_per_line_period; + + frl_char_per_line_period = (line_time_ns * frl_char_rate_min_k * lanes * + 1000) / FRL_TIMING_NS_MULTIPLIER; + return frl_char_per_line_period; +} + +/* Audio Suppor
[RFC v2 3/5] drm/hdmi21: Add helpers to verify non-dsc DFM requirements
Add helpers to compute DFM variables and to verify if the DFM requirements are met or not in non dsc cases. Signed-off-by: Vandita Kulkarni --- drivers/gpu/drm/drm_frl_dfm_helper.c | 161 +++ include/drm/drm_frl_dfm_helper.h | 2 + 2 files changed, 163 insertions(+) diff --git a/drivers/gpu/drm/drm_frl_dfm_helper.c b/drivers/gpu/drm/drm_frl_dfm_helper.c index d3ae35653370..b8f4f8ee50d3 100644 --- a/drivers/gpu/drm/drm_frl_dfm_helper.c +++ b/drivers/gpu/drm/drm_frl_dfm_helper.c @@ -394,3 +394,164 @@ drm_compute_payload_utilization(unsigned int frl_char_payload_actual, unsigned i utilization = (frl_char_payload_actual * EFFICIENCY_MULTIPLIER) / frl_char_per_line_period; return utilization; } + +/* Collect link characteristics */ +static void +drm_frl_dfm_compute_link_characteristics(struct drm_hdmi_frl_dfm *frl_dfm) +{ + unsigned int frl_bit_rate_min_kbps; + + frl_dfm->params.pixel_clock_max_khz = + drm_get_max_legal_pixel_rate(frl_dfm->config.pixel_clock_nominal_khz); + frl_dfm->params.line_time_ns = + drm_get_min_video_line_period(frl_dfm->config.hblank, + frl_dfm->config.hactive, + frl_dfm->params.pixel_clock_max_khz); + frl_bit_rate_min_kbps = drm_get_min_frl_bit_rate(frl_dfm->config.bit_rate_kbps); + frl_dfm->params.char_rate_min_kbps = drm_get_min_frl_char_rate(frl_bit_rate_min_kbps); + frl_dfm->params.cfrl_line = + drm_get_total_frl_char_per_line_period(frl_dfm->params.line_time_ns, + frl_dfm->params.char_rate_min_kbps, + frl_dfm->config.lanes); +} + +/* Determine FRL link overhead */ +static void drm_frl_dfm_compute_max_frl_link_overhead(struct drm_hdmi_frl_dfm *frl_dfm) +{ + unsigned int overhead_min; + + overhead_min = drm_get_total_minimum_overhead(frl_dfm->config.lanes); + frl_dfm->params.overhead_max = drm_get_max_overhead(overhead_min); +} + +/* Audio support Verification computations */ +static void +drm_frl_dfm_compute_audio_hblank_min(struct drm_hdmi_frl_dfm *frl_dfm) +{ + unsigned int num_audio_pkt, audio_pkt_rate; + + /* +* TBD: get the actual audio pkt type as described in +* table 6.44 of HDMI2.1 spec to find the num_audio_pkt, +* for now assume audio sample packet and audio packet +* layout as 1, resulting in number of audio packets +* required to carry each audio sample or audio frame +* as 1 +*/ + num_audio_pkt = 1; + audio_pkt_rate = drm_get_audio_pkt_rate(frl_dfm->config.audio_hz, num_audio_pkt); + frl_dfm->params.num_audio_pkts_line = +drm_get_audio_pkts_hblank(audio_pkt_rate, frl_dfm->params.line_time_ns); + frl_dfm->params.hblank_audio_min = + drm_get_audio_hblank_min(frl_dfm->params.num_audio_pkts_line); +} + +/* + * Determine the number of tribytes required for active video , blanking period + * with the pixel configuration + */ +static void +drm_frl_dfm_compute_tbactive_tbblank(struct drm_hdmi_frl_dfm *frl_dfm) +{ + unsigned int bpp, bytes_per_line; + + bpp = drm_get_frl_bits_per_pixel(frl_dfm->config.color_format, frl_dfm->config.bpc); + bytes_per_line = drm_get_video_bytes_per_line(bpp, frl_dfm->config.hactive); + + frl_dfm->params.tb_active = drm_get_active_video_tribytes_reqd(bytes_per_line); + frl_dfm->params.tb_blank = + drm_get_blanking_tribytes_avail(frl_dfm->config.color_format, + frl_dfm->config.hblank, + frl_dfm->config.bpc); +} + +/* Verify the configuration meets the capacity requirements for the FRL configuration*/ +static bool +drm_frl_dfm_verify_frl_capacity_requirement(struct drm_hdmi_frl_dfm *frl_dfm) +{ + unsigned int tactive_ref_ns, tblank_ref_ns, tactive_min_ns, tblank_min_ns; + unsigned int tborrowed_ns; + + frl_dfm->params.ftb_avg_k = + drm_get_avg_tribyte_rate(frl_dfm->params.pixel_clock_max_khz, +frl_dfm->params.tb_active, frl_dfm->params.tb_blank, +frl_dfm->config.hactive, frl_dfm->config.hblank); + tactive_ref_ns = drm_get_tactive_ref(frl_dfm->params.line_time_ns, +frl_dfm->config.hblank, +frl_dfm->config.hactive); + tblank_ref_ns = drm_get_tblank_ref(frl_dfm->params.line_time_ns, + frl_dfm->config.hblank, + frl_dfm->config.hactive); + tactive_min_ns = drm_get_tactive_min(frl_dfm->config.lanes, +
[RFC v2 1/5] drm/hdmi21: Define frl_dfm structure
Define frl_dfm structure to hold frl characteristics needed for frl capacity computation in order to meet the data flow metering requirement. Signed-off-by: Vandita Kulkarni --- include/drm/drm_frl_dfm_helper.h | 124 +++ 1 file changed, 124 insertions(+) create mode 100644 include/drm/drm_frl_dfm_helper.h diff --git a/include/drm/drm_frl_dfm_helper.h b/include/drm/drm_frl_dfm_helper.h new file mode 100644 index ..5cab102fe25f --- /dev/null +++ b/include/drm/drm_frl_dfm_helper.h @@ -0,0 +1,124 @@ +/* SPDX-License-Identifier: MIT + * Copyright © 2022 Intel Corp + */ + +#ifndef DRM_FRL_DFM_H_ +#define DRM_FRL_DFM_H_ + +/* DFM constraints and tolerance values from HDMI2.1 spec */ +#define TB_BORROWED_MAX400 +#define FRL_CHAR_PER_CHAR_BLK 510 +/* Tolerance pixel clock unit is in mHz */ +#define TOLERANCE_PIXEL_CLOCK 5 +#define TOLERANCE_FRL_BIT_RATE 300 +#define TOLERANCE_AUDIO_CLOCK 1000 +#define ACR_RATE_MAX 1500 +#define EFFICIENCY_MULTIPLIER 1000 +#define OVERHEAD_M (3 * EFFICIENCY_MULTIPLIER / 1000) +#define BPP_MULTIPLIER 16 +#define FRL_TIMING_NS_MULTIPLIER 10 + +/* ALl the input config needed to compute DFM requirements */ +struct drm_frl_dfm_input_config { + /* +* Pixel clock rate kHz, when FVA is +* enabled this rate is the rate after adjustment +*/ + unsigned int pixel_clock_nominal_khz; + + /* active pixels per line */ + unsigned int hactive; + + /* Blanking pixels per line */ + unsigned int hblank; + + /* Bits per component */ + unsigned int bpc; + + /* Pixel encoding */ + unsigned int color_format; + + /* FRL bit rate in kbps */ + unsigned int bit_rate_kbps; + + /* FRL lanes */ + unsigned int lanes; + + /* Number of audio channels */ + unsigned int audio_channels; + + /* Audio rate in Hz */ + unsigned int audio_hz; + + /* Selected bpp target value */ + unsigned int target_bpp_16; + + /* +* Number of horizontal pixels in a slice. +* Equivalent to PPS parameter slice_width +*/ + unsigned int slice_width; +}; + +/* Computed dfm parameters as per the HDMI2.1 spec */ +struct drm_frl_dfm_params { + /* +* Link overhead in percentage +* multiplied by 1000 (efficiency multiplier) +*/ + unsigned int overhead_max; + + /* Maximum pixel rate in kHz */ + unsigned int pixel_clock_max_khz; + + /* Minimum video line period in nano sec */ + unsigned int line_time_ns; + + /* worst case slow frl character rate in kbps */ + unsigned int char_rate_min_kbps; + + /* minimum total frl charecters per line perios */ + unsigned int cfrl_line; + + /* Average tribyte rate in khz */ + unsigned int ftb_avg_k; + + /* Audio characteristics */ + + /* number of audio packets needed during hblank */ + unsigned int num_audio_pkts_line; + + /* +* Minimum required hblank assuming no control preiod +* RC compression +*/ + unsigned int hblank_audio_min; + + /* Number of tribytes required to carry active video */ + unsigned int tb_active; + + /* Total available tribytes during the blanking period */ + unsigned int tb_blank; + + /* +* Number of tribytes required to be transmitted during +* the hblank period +*/ + unsigned int tb_borrowed; + + /* DSC frl characteristics */ + + /* Tribytes required to carry the target bpp */ + unsigned int hcactive_target; + + /* tribytes available during blanking with target bpp */ + unsigned int hcblank_target; +}; + +/* FRL DFM structure to hold involved in DFM computation */ +struct drm_hdmi_frl_dfm { + struct drm_frl_dfm_input_config config; + struct drm_frl_dfm_params params; +}; + +#endif -- 2.32.0
[RFC v2 5/5] drm/hdmi21: Add frl_dfm_helper to Makefile
Add the new frl_dfm_helper file to drm Makefile Signed-off-by: Vandita Kulkarni --- drivers/gpu/drm/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 8675c2af7ae1..81fe3df8bfda 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -57,7 +57,9 @@ drm_kms_helper-y := drm_bridge_connector.o drm_crtc_helper.o \ drm_scdc_helper.o drm_gem_atomic_helper.o \ drm_gem_framebuffer_helper.o \ drm_atomic_state_helper.o drm_damage_helper.o \ - drm_format_helper.o drm_self_refresh_helper.o drm_rect.o + drm_format_helper.o drm_self_refresh_helper.o drm_rect.o \ + drm_frl_dfm_helper.o + drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o -- 2.32.0
[RFC v2 4/5] drm/hdmi21: Add support for DFM calculation with DSC
From: Ankit Nautiyal Add helper functions for calculating FRL capacity and DFM requirements with given compressed bpp. Signed-off-by: Ankit Nautiyal Signed-off-by: Vandita Kulkarni --- drivers/gpu/drm/drm_frl_dfm_helper.c | 297 +++ include/drm/drm_frl_dfm_helper.h | 3 + 2 files changed, 300 insertions(+) diff --git a/drivers/gpu/drm/drm_frl_dfm_helper.c b/drivers/gpu/drm/drm_frl_dfm_helper.c index b8f4f8ee50d3..9eb91dd4e21e 100644 --- a/drivers/gpu/drm/drm_frl_dfm_helper.c +++ b/drivers/gpu/drm/drm_frl_dfm_helper.c @@ -555,3 +555,300 @@ drm_frl_dfm_nondsc_requirement_met(struct drm_hdmi_frl_dfm *frl_dfm) return false; } EXPORT_SYMBOL(drm_frl_dfm_nondsc_requirement_met); + +/* DSC DFM functions */ +/* Get FRL Available characters */ +static unsigned int +drm_get_frl_available_chars(unsigned int overhead_max, unsigned int cfrl_line) +{ + unsigned int frl_char_avlb = ((EFFICIENCY_MULTIPLIER - overhead_max) * cfrl_line); + + return frl_char_avlb / EFFICIENCY_MULTIPLIER; +} + +/* Get required no. of tribytes during HCActive */ +static unsigned int +drm_get_frl_hcactive_tb_target(unsigned int dsc_bpp_x16, unsigned int slice_width, unsigned int num_slices) +{ + unsigned int bytes_target; + + bytes_target = num_slices * DIV_ROUND_UP(dsc_bpp_x16 * slice_width, +8 * BPP_MULTIPLIER); + + return DIV_ROUND_UP(bytes_target, 3); +} + +/* Get required no. of tribytes (estimate1) during HCBlank */ +static unsigned int +drm_get_frl_hcblank_tb_est1_target(unsigned int hcactive_target_tb, + unsigned int hactive, unsigned int hblank) +{ + return DIV_ROUND_UP(hcactive_target_tb * hblank, hactive); +} + +/* Get required no. of tribytes during HCBlank */ +static unsigned int +drm_get_frl_hcblank_tb_target(unsigned int hcactive_target_tb, unsigned int hactive, unsigned int hblank, + unsigned int hcblank_audio_min, unsigned int cfrl_available) +{ + unsigned int hcblank_target_tb1 = drm_get_frl_hcblank_tb_est1_target(hcactive_target_tb, + hactive, hblank); + unsigned int hcblank_target_tb2 = max(hcblank_target_tb1, hcblank_audio_min); + + return 4 * (min(hcblank_target_tb2, + (2 * cfrl_available - 3 * hcactive_target_tb) / 2) / 4); +} + +/* Get the avg no of tribytes sent per sec (Kbps) */ +static unsigned int +drm_frl_dsc_get_ftb_avg(unsigned int hcactive_target_tb, unsigned int hcblank_target_tb, + unsigned int hactive, unsigned int hblank, + unsigned int fpixelclock_max_khz) +{ + return (hcactive_target_tb + hcblank_target_tb) * (fpixelclock_max_khz / (hactive + hblank)); +} + +/* Time to send Active tribytes in nanoseconds */ +static unsigned int +drm_frl_dsc_get_tactive_ref_ns(unsigned int line_time_ns, unsigned int hactive, unsigned int hblank) +{ + return (line_time_ns * hactive) / (hactive + hblank); +} + +/* Time to send Blanking tribytes in nanoseconds */ +static unsigned int +drm_frl_dsc_get_tblank_ref_ns(unsigned int line_time_ns, unsigned int hactive, unsigned int hblank) +{ + return (line_time_ns * hblank) / (hactive + hblank); +} + +/* Get time to send all tribytes in hcactive region in nsec*/ +static unsigned int +drm_frl_dsc_tactive_target_ns(unsigned int frl_lanes, unsigned int hcactive_target_tb, unsigned int ftb_avg_k, + unsigned int min_frl_char_rate_k, unsigned int overhead_max) +{ + unsigned int avg_tribyte_time_ns, tribyte_time_ns; + unsigned int num_chars_hcactive; + unsigned int frl_char_rate_k; + + /* Avg time to transmit all active region tribytes */ + avg_tribyte_time_ns = (hcactive_target_tb * FRL_TIMING_NS_MULTIPLIER) / + (ftb_avg_k * 1000); + + /* +* 2 bytes in active region = 1 FRL characters +* 1 Tribyte in active region = 3/2 FRL characters +*/ + + num_chars_hcactive = (hcactive_target_tb * 3) / 2; + + /* +* FRL rate = lanes * frl character rate +* But actual bandwidth wil be less, due to FRL limitations so account +* for the overhead involved. +* FRL rate with overhead = FRL rate * (100 - overhead %) / 100 +*/ + frl_char_rate_k = frl_lanes * min_frl_char_rate_k; + frl_char_rate_k = (frl_char_rate_k * (EFFICIENCY_MULTIPLIER - overhead_max)) / + EFFICIENCY_MULTIPLIER; + + /* Time to transmit all characters with FRL limitations */ + tribyte_time_ns = (num_chars_hcactive * FRL_TIMING_NS_MULTIPLIER) / + frl_char_rate_k * 1000; + + return max(avg_tribyte_time_ns, tribyte_time_ns); +} + +/* Get no. of tri bytes borrowed with DSC enabled */ +static unsigned int +drm_frl_get_dsc_tri_bytes_borr
Re: [PATCH] drivers: fbtft: Refactor backlight logic
On Sat, Feb 12, 2022 at 12:27:39PM +0800, qianfangui...@163.com wrote: > From: qianfan Zhao > > Control led gpios by using GPIO_ACTIVE flags, don't detect the polarity > by reading the gpio value when probe. ... > + enum gpiod_flags flags = GPIOD_OUT_HIGH; > + > + /* request and turn on backlight */ > + return fbtft_request_one_gpio_with_flags(par, name, index, flags, > gpiop); But taking into consideration all possible polarities this is wrong assumption. Depending on the flags in DT or ACPI this may give an opposite effect. P.S. Please, Cc all your fbtft patches to fbdev maintainer as well. -- With Best Regards, Andy Shevchenko
Re: [PATCH 06/23] drm/ingenic: Make use of the helper component_compare_of
Hi, Le lun., févr. 14 2022 at 14:08:02 +0800, Yong Wu a écrit : Use the common compare helper from component. Cc: Paul Cercueil Cc: linux-m...@vger.kernel.org Signed-off-by: Yong Wu Acked-by: Paul Cercueil Cheers, -Paul --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index b4943a56be09..23b8f012b418 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -1322,11 +1322,6 @@ static int ingenic_drm_bind_with_components(struct device *dev) return ingenic_drm_bind(dev, true); } -static int compare_of(struct device *dev, void *data) -{ - return dev->of_node == data; -} - static void ingenic_drm_unbind(struct device *dev) { struct ingenic_drm *priv = dev_get_drvdata(dev); @@ -1360,7 +1355,7 @@ static int ingenic_drm_probe(struct platform_device *pdev) if (!np) return ingenic_drm_bind(dev, false); - drm_of_component_match_add(dev, &match, compare_of, np); + drm_of_component_match_add(dev, &match, component_compare_of, np); of_node_put(np); return component_master_add_with_match(dev, &ingenic_master_ops, match); -- 2.18.0
Re: [PATCH v7 0/7] drm/lsdc: add drm driver for loongson display controller
On 2022/2/14 13:54, Jiaxun Yang wrote: 在 2022/2/13 14:16, Sui Jingfeng 写道: There is a display controller in loongson's LS2K1000 SoC and LS7A1000 bridge chip, the DC is a PCI device in those chips. It has two display pipes but with only one hardware cursor. Each way has a DVO interface which provide RGB888 signals, vertical & horizontal synchronisations, data enable and the pixel clock. Each CRTC is able to scanout from 1920x1080 resolution at 60Hz. The maxmium resolution is 2048x2048 according to the hardware spec. Hi Jiangfeng, I see you added dts for those boards, but I didn't see you wire up them in Makefile and code? How can you use them in present systems? I guess to make those dts work for general all-in-one kernel, what you need to do is, for example Lemota A1901: 1. Add __dtb_lemote_a1901 to builtin_dtbs.h 2. Wire up with something like: if (!strcmp("LEMOTE-/LS3A4000/-7A1000-1w-V01-pc", eboard->name) loongson_fdt_blob = __dtb_lemote_a1901 In arch/mips/loongson64/env.c. Thanks. - Jiaxun For most board, this driver is ready to be use out of box. Device tree is for supplement purpose.
Re: [PATCH v3 1/2] drm/panel: Add inx Himax8279d MIPI-DSI LCD panel driver
Il 13/02/22 07:31, Hsin-Yi Wang ha scritto: From: xiazhengqiao Add STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel Signed-off-by: xiazhengqiao Signed-off-by: Hsin-Yi Wang I have no way of testing this driver but the code itself looks good to me, so, strictly for the code: Reviewed-by: AngeloGioacchino Del Regno --- v2->v3: rebase to next-20220211 --- drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile| 1 + .../gpu/drm/panel/panel-innolux-himax8279d.c | 515 ++ 3 files changed, 525 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-innolux-himax8279d.c
Re: [PATCH] drivers: fbtft: Add property 'keep-bootlogo'
On Sat, Feb 12, 2022 at 01:37:11PM +0800, qianfangui...@163.com wrote: > From: qianfan Zhao > > Keep the logo draw by bootloader Please, Cc to fbdev maintainer. Personally I'm not sure we want this from maintenance perspective, but I understand what you want to achieve with it. -- With Best Regards, Andy Shevchenko
Re: [PATCH v7 1/7] drm/lsdc: add drm driver for loongson display controller
On Sun, Feb 13, 2022 at 10:16:43PM +0800, Sui Jingfeng wrote: > From: suijingfeng > > There is a display controller in loongson's LS2K1000 SoC and LS7A1000 > bridge chip, the DC is a PCI device in those chips. It has two display > pipes but with only one hardware cursor. Each way has a DVO interface > which provide RGB888 signals, vertical & horizontal synchronisations, > data enable and the pixel clock. Each CRTC is able to scanout from > 1920x1080 resolution at 60Hz. The maxmium resolution is 2048x2048 > according to the hardware spec. > > Loongson display controllers are simple which require scanout buffers > to be physically contiguous. LS2K1000 is a SOC, Only system memory is > available. Therefore CMA helper based driver is intended to be use, > although it is possible to use VRAM helper based solution by carving > out part of system memory as VRAM. > > On LS7A1000/LS7A2000 bridge chip, the DC is equipped with a dedicated > video memory which is typically 64MB or more. In this case, VRAM helper > based solution which scanout from local VRAM is reconmended to use. > It is reliable to use for massive production, but CMA based helper > solution is still usable on ls7a1000 and ls7a2000, at the price of > the CRTC must access the FB in RAM through the PCIe bus and HT3.0 bus. > This causes continuous traffic on the bus regardless of whether the FB > image is updating or not. Sadly, it suffer from screen flickering under > RAM pressure on LS7A1000. Luckily, It show extremely good condition on > LS7A2000 even under stressapptest, Maybe the hardware engineer resolve > this issue. Integrating two distict helpers based driver into one piece > allow code sharing. > > We have also implemented demage update on top of CMA helper which copy > the demaged region from the shadow framebuffer in system RAM to the real > framebuffer in VRAM manually. This is intend to overcome the screen > flicking issue on LS7A1000, but the performance is not good. > Using "lsdc.dirty_update=1" in the kernel commmand line if you would like > to have a try. > > For LS7A1000, there are 4 dedicated GPIOs whose control register is > located at the DC register space, They are used to emulate two way i2c. > One for DVO0, another for DVO1. This is the reason why this driver is > not switch to drm bridge framework yet. LS2K1000 and LS2K0500 SoC don't > have such GPIO hardwared, they grab i2c adapter from other module, > either general purpose GPIO emulated i2c or hardware i2c adapter. > Drm bridge and drm panel driver for the external encoder is suitable for > those SoC. We have already implemented this on our downstream 4.19.190 > kernel. But due to the GPIO, PWM and I2C device driver support for > LS2K1000 is not upstreamed yet, this driver still can be use to bring > the graphic environment up by providing display timings or similar things > in the device tree. > > The DC in LS7A1000 has only one hardware cursor, we simply let the two > CRTC share it. The DC in LS7A2000 have two cursor, two built-in hdmi > encoder and one transparent vga encoder and more, surport for LS7A2000 > is on the way. In short, we have built-in gpio emulated i2c support, > we also have hardware cursor support. LS7A2000 The kind of tiny drivers > in drm/tiny is not suitable for us. > > +--++---+ > | DDR4 || +---+| > +--+| | PCIe Root complex | LS7A1000 | >|| MC0 | +--++-+++| > +--+ HT 3.0 | || || | > | LS3A4000 |<>| +---++---+ +--++--++-+ +--+ > | CPU|<>| | GC1000 | | LSDC |<-->| DDR3 MC |<->| VRAM | > +--+ | ++ +-+--+-++-+ +--+ >|| MC1 +---|--|+ > +--+| | > | DDR4 | +---+ DVO0 | | DVO1 +--+ > +--+ VGA <--|ADV7125|<+ +>|TFP410|--> DVI/HDMI > +---+ +--+ > > The above picture give a simple usage of LS7A1000, note that the encoder > is not necessary adv7125 or tfp410, it is a choice of the downstream board > manufacturer. Other candicate encoders can be ch7034b, sil9022 and ite66121 > lt8618 etc. Besides, the DC in both ls2k1000 and ls7k1000 has the same of > PCI vendor id and pci device id. Both is 0x0014:0x7a06, the reverison id > is also same. This is the firmware engineer's mistake, but such firmware > and various boards ship with such firmware already released. We choose to > deduce the chip's identification from information provided by device tree. > For lsdc, there is only a 1:1 mapping of encoders and connectors. > > v2: fixup warnings reported by kernel test robot > > v3: fix more grammar mistakes in Kconfig reported by Randy Dunlap and give > more details about lsdc. > >
Re: [PATCH v15 1/7] drm/ingenic: Fix support for JZ4780 HDMI output
Hi, Le sam., févr. 12 2022 at 16:50:49 +0100, H. Nikolaus Schaller a écrit : From: Paul Boddie We have to make sure that - JZ_LCD_OSDC_ALPHAEN is set - plane f0 is disabled and not seen from user-space Actually it will still be seen from user-space, but it won't be possible to use it. So before applying I'll change this to: "plane f0 is disabled as it's not working yet" If that's OK with you. Cheers, -Paul Tested on MIPS Creator CI20 board. Signed-off-by: Paul Boddie Signed-off-by: Ezequiel Garcia Signed-off-by: H. Nikolaus Schaller --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 7f10d6eed549d..dcf44cb00821f 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -65,8 +65,10 @@ struct ingenic_dma_hwdescs { struct jz_soc_info { bool needs_dev_clk; bool has_osd; + bool has_alpha; bool map_noncoherent; bool use_extended_hwdesc; + bool plane_f0_not_working; unsigned int max_width, max_height; const u32 *formats_f0, *formats_f1; unsigned int num_formats_f0, num_formats_f1; @@ -453,7 +455,7 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, if (!crtc) return 0; - if (plane == &priv->f0) + if (priv->soc_info->plane_f0_not_working && plane == &priv->f0) return -EINVAL; crtc_state = drm_atomic_get_existing_crtc_state(state, @@ -1055,6 +1057,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) long parent_rate; unsigned int i, clone_mask = 0; int ret, irq; + u32 osdc = 0; soc_info = of_device_get_match_data(dev); if (!soc_info) { @@ -1312,7 +1315,10 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) /* Enable OSD if available */ if (soc_info->has_osd) - regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN); + osdc |= JZ_LCD_OSDC_OSDEN; + if (soc_info->has_alpha) + osdc |= JZ_LCD_OSDC_ALPHAEN; + regmap_write(priv->map, JZ_REG_LCD_OSDC, osdc); mutex_init(&priv->clk_mutex); priv->clock_nb.notifier_call = ingenic_drm_update_pixclk; @@ -1511,7 +1517,9 @@ static const struct jz_soc_info jz4770_soc_info = { static const struct jz_soc_info jz4780_soc_info = { .needs_dev_clk = true, .has_osd = true, + .has_alpha = true, .use_extended_hwdesc = true, + .plane_f0_not_working = true, /* REVISIT */ .max_width = 4096, .max_height = 2048, .formats_f1 = jz4770_formats_f1, -- 2.33.0
Re: [PATCH v3 1/2] drm/panel: Add inx Himax8279d MIPI-DSI LCD panel driver
On Mon, Feb 14, 2022 at 6:10 PM AngeloGioacchino Del Regno wrote: > > Il 13/02/22 07:31, Hsin-Yi Wang ha scritto: > > From: xiazhengqiao > > > > Add STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel > > > > Signed-off-by: xiazhengqiao > > Signed-off-by: Hsin-Yi Wang > > I have no way of testing this driver but the code itself looks good to me, > so, strictly for the code: > > Reviewed-by: AngeloGioacchino Del Regno > > This driver is used by ASUS Chromebook Detachable CZ1 [1]. The dts will be accepted after this panel is accepted. [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20211213162856.235130-1-hsi...@chromium.org/ > > --- > > v2->v3: > > rebase to next-20220211 > > --- > > drivers/gpu/drm/panel/Kconfig | 9 + > > drivers/gpu/drm/panel/Makefile| 1 + > > .../gpu/drm/panel/panel-innolux-himax8279d.c | 515 ++ > > 3 files changed, 525 insertions(+) > > create mode 100644 drivers/gpu/drm/panel/panel-innolux-himax8279d.c > >
Re: [PATCH v15 6/7] drm/ingenic: dw-hdmi: make hot plug detection work for CI20
Hi Nikolaus, Le sam., févr. 12 2022 at 16:50:54 +0100, H. Nikolaus Schaller a écrit : There is no hpd-gpio installed on the CI20 board HDMI connector. Hence there is no hpd detection by the connector driver and we have to enable polling by the dw-hdmi driver. We need to set .poll_enabled but that struct component can only be accessed in the core code. Hence we use the public setter function drm_kms_helper_hotplug_event(). As I said in your v13 - if you move your patch [2/7] after the patch [5/7] then you can drop this patch (merge it with the patch that introduces ingenic-dw-hdmi.c). Otherwise between the introduction of the driver and the hotplug detection fix, the driver is not usable. Cheers, -Paul Signed-off-by: H. Nikolaus Schaller --- drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c index 34e986dd606cf..90547a28dc5c7 100644 --- a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c @@ -55,6 +55,8 @@ ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data, if (mode->clock > 216000) return MODE_CLOCK_HIGH; + dw_hdmi_enable_poll(hdmi, true); + return MODE_OK; } -- 2.33.0
Re: [PATCH v15 1/7] drm/ingenic: Fix support for JZ4780 HDMI output
Hi Paul, > Am 14.02.2022 um 11:13 schrieb Paul Cercueil : > > Hi, > > Le sam., févr. 12 2022 at 16:50:49 +0100, H. Nikolaus Schaller > a écrit : >> From: Paul Boddie >> We have to make sure that >> - JZ_LCD_OSDC_ALPHAEN is set >> - plane f0 is disabled and not seen from user-space > > Actually it will still be seen from user-space, but it won't be possible to > use it. So before applying I'll change this to: > "plane f0 is disabled as it's not working yet" > > If that's OK with you. Yes. You understand much better than me the implications... BR and thanks, Nikolaus
Re: [PATCH v15 2/7] drm/ingenic: Add dw-hdmi driver specialization for jz4780
Hi, Le sam., févr. 12 2022 at 16:50:50 +0100, H. Nikolaus Schaller a écrit : From: Paul Boddie A specialisation of the generic Synopsys HDMI driver is employed for JZ4780 HDMI support. This requires a new driver, plus device tree and configuration modifications. Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code. Signed-off-by: Paul Boddie Signed-off-by: Ezequiel Garcia Signed-off-by: H. Nikolaus Schaller --- drivers/gpu/drm/ingenic/Kconfig | 9 ++ drivers/gpu/drm/ingenic/Makefile | 1 + drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 104 ++ 3 files changed, 114 insertions(+) create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig index 001f59fb06d56..090830bcbde7f 100644 --- a/drivers/gpu/drm/ingenic/Kconfig +++ b/drivers/gpu/drm/ingenic/Kconfig @@ -24,4 +24,13 @@ config DRM_INGENIC_IPU The Image Processing Unit (IPU) will appear as a second primary plane. +config DRM_INGENIC_DW_HDMI + tristate "Ingenic specific support for Synopsys DW HDMI" + depends on MACH_JZ4780 + select DRM_DW_HDMI + help + Choose this option to enable Synopsys DesignWare HDMI based driver. + If you want to enable HDMI on Ingenic JZ4780 based SoC, you should + select this option. + endif diff --git a/drivers/gpu/drm/ingenic/Makefile b/drivers/gpu/drm/ingenic/Makefile index d313326bdddbb..f10cc1c5a5f22 100644 --- a/drivers/gpu/drm/ingenic/Makefile +++ b/drivers/gpu/drm/ingenic/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o ingenic-drm-y = ingenic-drm-drv.o ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o +obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c new file mode 100644 index 0..34e986dd606cf --- /dev/null +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc. + * Copyright (C) 2019, 2020 Paul Boddie + * + * Derived from dw_hdmi-imx.c with i.MX portions removed. + * Probe and remove operations derived from rcar_dw_hdmi.c. + */ + +#include +#include +#include + +#include +#include +#include + +static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = { + { 4525, { { 0x01e0, 0x }, { 0x21e1, 0x }, { 0x41e2, 0x } } }, + { 9250, { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142, 0x0005 } } }, + { 14850, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2, 0x000a } } }, + { 21600, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002, 0x000f } } }, + { ~0UL, { { 0x, 0x }, { 0x, 0x }, { 0x, 0x } } } +}; + +static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = { + /*pixelclk bpp8bpp10 bpp12 */ + { 5400, { 0x091c, 0x091c, 0x06dc } }, + { 5840, { 0x091c, 0x06dc, 0x06dc } }, + { 7200, { 0x06dc, 0x06dc, 0x091c } }, + { 7425, { 0x06dc, 0x0b5c, 0x091c } }, + { 11880, { 0x091c, 0x091c, 0x06dc } }, + { 21600, { 0x06dc, 0x0b5c, 0x091c } }, + { ~0UL, { 0x, 0x, 0x } }, +}; + +/* + * Resistance term 133Ohm Cfg + * PREEMP config 0.00 + * TX/CK level 10 + */ +static const struct dw_hdmi_phy_config ingenic_phy_config[] = { + /*pixelclk symbol term vlev */ + { 21600, 0x800d, 0x0005, 0x01ad}, + { ~0UL, 0x, 0x, 0x} +}; + +static enum drm_mode_status +ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data, + const struct drm_display_info *info, + const struct drm_display_mode *mode) +{ + if (mode->clock < 13500) + return MODE_CLOCK_LOW; + /* FIXME: Hardware is capable of 270MHz, but setup data is missing. */ + if (mode->clock > 216000) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + +static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = { + .mpll_cfg = ingenic_mpll_cfg, + .cur_ctr= ingenic_cur_ctr, + .phy_config = ingenic_phy_config, + .mode_valid = ingenic_dw_hdmi_mode_valid, + .output_port= 1, +}; + +static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = { + { .compatible = "ingenic,jz4780-dw-hdmi" }, + { /* Sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids); + +static void ingenic_dw_hdmi_cleanup(void *data) +{ + struct dw_hdmi *hdmi = (struct dw_hdmi *)data; + + dw_hdmi_remove(hdmi); +} + +static int ingenic_dw_hdmi_probe(struct platform_device *pdev) +{ + struct dw_hdmi *hdmi; + + hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data); + if (IS_ERR(hdmi)) + return PTR_ERR(hdmi); + + return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_clean
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
On Mon, Feb 14, 2022 at 11:17:11AM +0200, Pekka Paalanen wrote: > On Fri, 11 Feb 2022 19:27:12 +0200 > Andy Shevchenko wrote: > > On Fri, Feb 11, 2022 at 06:25:17PM +0200, Jani Nikula wrote: > > > On Fri, 11 Feb 2022, Andy Shevchenko > > > wrote: > > > > On Fri, Feb 11, 2022 at 02:05:56PM +0200, Jani Nikula wrote: > > > >> On Fri, 11 Feb 2022, Thomas Zimmermann wrote: > > > >> > Am 11.02.22 um 12:12 schrieb Andy Shevchenko: > > > >> >> On Fri, Feb 11, 2022 at 11:40:13AM +0100, Javier Martinez Canillas > > > >> >> wrote: > > > >> >>> On 2/11/22 11:28, Andy Shevchenko wrote: > > > >> On Fri, Feb 11, 2022 at 10:19:22AM +0100, Javier Martinez > > > >> Canillas wrote: ... > > > >> > +static void drm_fb_xrgb_to_gray8_line(u8 *dst, const u32 > > > >> > *src, unsigned int pixels) > > > >> > +{ > > > >> > +unsigned int x; > > > >> > + > > > >> > +for (x = 0; x < pixels; x++) { > > > >> > +u8 r = (*src & 0x00ff) >> 16; > > > >> > +u8 g = (*src & 0xff00) >> 8; > > > >> > +u8 b = *src & 0x00ff; > > > >> > + > > > >> > +/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B > > > >> > */ > > > >> > +*dst++ = (3 * r + 6 * g + b) / 10; > > > >> > +src++; > > > >> > +} > > > >> > > > >> Can be done as > > > >> > > > >> while (pixels--) { > > > >> ... > > > >> } > > > >> > > > >> or > > > >> > > > >> do { > > > >> ... > > > >> } while (--pixels); > > > >> > > > >> >>> > > > >> >>> I don't see why a while loop would be an improvement here TBH. > > > >> >> > > > >> >> Less letters to parse when reading the code. > > > >> > > > > >> > It's a simple refactoring of code that has worked well so far. Let's > > > >> > leave it as-is for now. > > > >> > > > >> IMO *always* prefer a for loop over while or do-while. > > > >> > > > >> The for (i = 0; i < N; i++) is such a strong paradigm in C. You > > > >> instantly know how many times you're going to loop, at a glance. Not so > > > >> with with the alternatives, which should be used sparingly. > > > > > > > > while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. > > > > > > And while() is just syntax sugar for goto. :p > > > > > > The for loop written as for (i = 0; i < N; i++) is hands down the most > > > obvious counting loop pattern there is in C. > > > > > > >> And yes, the do-while suggested above is buggy, and you actually need > > > >> to > > > >> stop and think to see why. > > > > > > > > It depends if pixels can be 0 or not and if it's not, then does it > > > > contain last > > > > or number. > > > > > > > > The do {} while (--pixels); might be buggy iff pixels may be 0. > > > > > > Yeah. And how long does it take to figure that out? > > > > Okay, I made a mistake to drop the explanation. So, I (mistakenly) assumed > > that people know this difference between post-decrement and pre-decrement > > (note, while-loop here is not what is problematic). > > That was not the question. > > The question was, how long does it take to figure out if pixels can or > cannot be zero? To me these patterns, while() {} and do {} while(), while being shorter, also give a hint. So if one is familiar with C, the do {} while (--foo) _gives a hint_ while being shorter. It requires _less_ brain power to get this. But I assume my brain is unique and not working as million of others. > Code is styled for humans other than the author, not for compilers. > > Having to stop to think about the difference between post- and > pre-decrement to figure out when the while-loop runs does take me a few > more brain cycles to understand, even though I know the rules very well. > > I would call that brain cycle optimization, and leave the CPU cycle > optimization for the compiler in these cases. -- With Best Regards, Andy Shevchenko
Re: [PATCH 01/11] drm/ttm: fix resource manager size type and description
On Mon, 14 Feb 2022 at 09:34, Christian König wrote: > > That are not pages any more. "Leave the man->size units as driver defined." > > Signed-off-by: Christian König > --- > drivers/gpu/drm/ttm/ttm_resource.c | 6 +++--- > include/drm/ttm/ttm_resource.h | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c > b/drivers/gpu/drm/ttm/ttm_resource.c > index 68344c90549b..ae40e144e728 100644 > --- a/drivers/gpu/drm/ttm/ttm_resource.c > +++ b/drivers/gpu/drm/ttm/ttm_resource.c > @@ -153,19 +153,19 @@ void ttm_resource_set_bo(struct ttm_resource *res, > * > * @man: memory manager object to init > * @bdev: ttm device this manager belongs to > - * @p_size: size managed area in pages. > + * @size: size of managed resources in arbitary units s/arbitary/arbitrary/ Reviewed-by: Matthew Auld > * > * Initialise core parts of a manager object. > */ > void ttm_resource_manager_init(struct ttm_resource_manager *man, >struct ttm_device *bdev, > - unsigned long p_size) > + uint64_t size) > { > unsigned i; > > spin_lock_init(&man->move_lock); > man->bdev = bdev; > - man->size = p_size; > + man->size = size; > > for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) > INIT_LIST_HEAD(&man->lru[i]); > diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h > index 69eea9d6399b..555a11fb8a7f 100644 > --- a/include/drm/ttm/ttm_resource.h > +++ b/include/drm/ttm/ttm_resource.h > @@ -278,7 +278,7 @@ void ttm_resource_set_bo(struct ttm_resource *res, > > void ttm_resource_manager_init(struct ttm_resource_manager *man, >struct ttm_device *bdev, > - unsigned long p_size); > + uint64_t size); > > int ttm_resource_manager_evict_all(struct ttm_device *bdev, >struct ttm_resource_manager *man); > -- > 2.25.1 >
Re: [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
On Mon, 14 Feb 2022 at 09:34, Christian König wrote: > > It makes sense to have this in the common manager for debugging and > accounting of how much resources are used. > > v2: cleanup kerneldoc a bit > v3: drop the atomic, update counter under lock instead > > Signed-off-by: Christian König > Reviewed-by: Huang Rui (v1) > Tested-by: Bas Nieuwenhuizen > --- > drivers/gpu/drm/ttm/ttm_resource.c | 30 ++ > include/drm/ttm/ttm_resource.h | 11 +-- > 2 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c > b/drivers/gpu/drm/ttm/ttm_resource.c > index ae40e144e728..bbb8a0f7aa14 100644 > --- a/drivers/gpu/drm/ttm/ttm_resource.c > +++ b/drivers/gpu/drm/ttm/ttm_resource.c > @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo, > const struct ttm_place *place, > struct ttm_resource *res) > { > + struct ttm_resource_manager *man; > + > res->start = 0; > res->num_pages = PFN_UP(bo->base.size); > res->mem_type = place->mem_type; > @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo, > res->bus.is_iomem = false; > res->bus.caching = ttm_cached; > res->bo = bo; > + > + man = ttm_manager_type(bo->bdev, place->mem_type); > + spin_lock(&bo->bdev->lru_lock); > + man->usage += bo->base.size; > + spin_unlock(&bo->bdev->lru_lock); > } > EXPORT_SYMBOL(ttm_resource_init); > > @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init); > void ttm_resource_fini(struct ttm_resource_manager *man, >struct ttm_resource *res) > { > + spin_lock(&man->bdev->lru_lock); > + man->usage -= res->bo->base.size; > + spin_unlock(&man->bdev->lru_lock); > } > EXPORT_SYMBOL(ttm_resource_fini); > > @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct > ttm_resource_manager *man, > spin_lock_init(&man->move_lock); > man->bdev = bdev; > man->size = size; > + man->usage = 0; > > for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) > INIT_LIST_HEAD(&man->lru[i]); > @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device > *bdev, > } > EXPORT_SYMBOL(ttm_resource_manager_evict_all); > > +/** > + * ttm_resource_manager_usage > + * > + * @man: A memory manager object. > + * > + * Return how many resources are currently used. Maybe mention the units here? "Return how many resources are currently used, in bytes." Anyway, Reviewed-by: Matthew Auld > + */ > +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man) > +{ > + uint64_t usage; > + > + spin_lock(&man->bdev->lru_lock); > + usage = man->usage; > + spin_unlock(&man->bdev->lru_lock); > + return usage; > +} > +EXPORT_SYMBOL(ttm_resource_manager_usage); > + > /** > * ttm_resource_manager_debug > * > @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct > ttm_resource_manager *man, > drm_printf(p, " use_type: %d\n", man->use_type); > drm_printf(p, " use_tt: %d\n", man->use_tt); > drm_printf(p, " size: %llu\n", man->size); > + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man)); > if (man->func->debug) > man->func->debug(man, p); > } > diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h > index 555a11fb8a7f..323c14a30c6b 100644 > --- a/include/drm/ttm/ttm_resource.h > +++ b/include/drm/ttm/ttm_resource.h > @@ -27,6 +27,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -130,10 +131,15 @@ struct ttm_resource_manager { > struct dma_fence *move; > > /* > -* Protected by the global->lru_lock. > +* Protected by the bdev->lru_lock. > */ > - > struct list_head lru[TTM_MAX_BO_PRIORITY]; > + > + /** > +* @usage: How much of the resources are used, protected by the > +* bdev->lru_lock. > +*/ > + uint64_t usage; > }; > > /** > @@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct > ttm_resource_manager *man, > int ttm_resource_manager_evict_all(struct ttm_device *bdev, >struct ttm_resource_manager *man); > > +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man); > void ttm_resource_manager_debug(struct ttm_resource_manager *man, > struct drm_printer *p); > > -- > 2.25.1 >
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
On Mon, Feb 14, 2022 at 10:03:53AM +0100, Thomas Zimmermann wrote: > Am 11.02.22 um 16:41 schrieb Andy Shevchenko: ... > > > IMO *always* prefer a for loop over while or do-while. > > > > > > The for (i = 0; i < N; i++) is such a strong paradigm in C. You > > > instantly know how many times you're going to loop, at a glance. Not so > > > with with the alternatives, which should be used sparingly. > > > > while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. > > Naw, that's not true. In the section 3.5 "Loops - While and For" in "The C Programming Language" 2nd by K&R, the authors said: The for statement ... is equivalent to ... while..." They said that for is equivalent to while, and not otherwise. Also, syntax sugar by definition declares something that can be written as a single line of code, which usually is done using more (not always). > An idiomatic for loop, such as for (i = ...; i < N; > ++i), is such a strong pattern that it's way better than the corresponding > while loop. > > > And yes, the do-while suggested above is buggy, and you actually need to > > > stop and think to see why. > > > > It depends if pixels can be 0 or not and if it's not, then does it contain > > last > > or number. > > > > The do {} while (--pixels); might be buggy iff pixels may be 0. -- With Best Regards, Andy Shevchenko
Re: [PATCH v7 1/7] drm/lsdc: add drm driver for loongson display controller
On 2022/2/14 18:10, Maxime Ripard wrote: On Sun, Feb 13, 2022 at 10:16:43PM +0800, Sui Jingfeng wrote: From: suijingfeng There is a display controller in loongson's LS2K1000 SoC and LS7A1000 bridge chip, the DC is a PCI device in those chips. It has two display pipes but with only one hardware cursor. Each way has a DVO interface which provide RGB888 signals, vertical & horizontal synchronisations, data enable and the pixel clock. Each CRTC is able to scanout from 1920x1080 resolution at 60Hz. The maxmium resolution is 2048x2048 according to the hardware spec. Loongson display controllers are simple which require scanout buffers to be physically contiguous. LS2K1000 is a SOC, Only system memory is available. Therefore CMA helper based driver is intended to be use, although it is possible to use VRAM helper based solution by carving out part of system memory as VRAM. On LS7A1000/LS7A2000 bridge chip, the DC is equipped with a dedicated video memory which is typically 64MB or more. In this case, VRAM helper based solution which scanout from local VRAM is reconmended to use. It is reliable to use for massive production, but CMA based helper solution is still usable on ls7a1000 and ls7a2000, at the price of the CRTC must access the FB in RAM through the PCIe bus and HT3.0 bus. This causes continuous traffic on the bus regardless of whether the FB image is updating or not. Sadly, it suffer from screen flickering under RAM pressure on LS7A1000. Luckily, It show extremely good condition on LS7A2000 even under stressapptest, Maybe the hardware engineer resolve this issue. Integrating two distict helpers based driver into one piece allow code sharing. We have also implemented demage update on top of CMA helper which copy the demaged region from the shadow framebuffer in system RAM to the real framebuffer in VRAM manually. This is intend to overcome the screen flicking issue on LS7A1000, but the performance is not good. Using "lsdc.dirty_update=1" in the kernel commmand line if you would like to have a try. For LS7A1000, there are 4 dedicated GPIOs whose control register is located at the DC register space, They are used to emulate two way i2c. One for DVO0, another for DVO1. This is the reason why this driver is not switch to drm bridge framework yet. LS2K1000 and LS2K0500 SoC don't have such GPIO hardwared, they grab i2c adapter from other module, either general purpose GPIO emulated i2c or hardware i2c adapter. Drm bridge and drm panel driver for the external encoder is suitable for those SoC. We have already implemented this on our downstream 4.19.190 kernel. But due to the GPIO, PWM and I2C device driver support for LS2K1000 is not upstreamed yet, this driver still can be use to bring the graphic environment up by providing display timings or similar things in the device tree. The DC in LS7A1000 has only one hardware cursor, we simply let the two CRTC share it. The DC in LS7A2000 have two cursor, two built-in hdmi encoder and one transparent vga encoder and more, surport for LS7A2000 is on the way. In short, we have built-in gpio emulated i2c support, we also have hardware cursor support. LS7A2000 The kind of tiny drivers in drm/tiny is not suitable for us. +--++---+ | DDR4 || +---+| +--+| | PCIe Root complex | LS7A1000 | || MC0 | +--++-+++| +--+ HT 3.0 | || || | | LS3A4000 |<>| +---++---+ +--++--++-+ +--+ | CPU|<>| | GC1000 | | LSDC |<-->| DDR3 MC |<->| VRAM | +--+ | ++ +-+--+-++-+ +--+ || MC1 +---|--|+ +--+| | | DDR4 | +---+ DVO0 | | DVO1 +--+ +--+ VGA <--|ADV7125|<+ +>|TFP410|--> DVI/HDMI +---+ +--+ The above picture give a simple usage of LS7A1000, note that the encoder is not necessary adv7125 or tfp410, it is a choice of the downstream board manufacturer. Other candicate encoders can be ch7034b, sil9022 and ite66121 lt8618 etc. Besides, the DC in both ls2k1000 and ls7k1000 has the same of PCI vendor id and pci device id. Both is 0x0014:0x7a06, the reverison id is also same. This is the firmware engineer's mistake, but such firmware and various boards ship with such firmware already released. We choose to deduce the chip's identification from information provided by device tree. For lsdc, there is only a 1:1 mapping of encoders and connectors. v2: fixup warnings reported by kernel test robot v3: fix more grammar mistakes in Kconfig reported by Randy Dunlap and give more details about lsdc. v4: 1) Add dts required and explain why device tree is required. 2) Give more description about lsdc an
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
On Monday, February 14th, 2022 at 11:38, Andy Shevchenko wrote: > > > > IMO *always* prefer a for loop over while or do-while. > > > > > > > > The for (i = 0; i < N; i++) is such a strong paradigm in C. You > > > > instantly know how many times you're going to loop, at a glance. Not so > > > > with with the alternatives, which should be used sparingly. > > > > > > while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. > > > > Naw, that's not true. > > In the section 3.5 "Loops - While and For" in "The C Programming > Language" 2nd by K&R, the authors said: > > The for statement ... is equivalent to ... while..." > > They said that for is equivalent to while, and not otherwise. > > Also, syntax sugar by definition declares something that can be written as > a single line of code, which usually is done using more (not always). arr[i] is syntaxic sugar for *(arr + i), yet we keep writing the former, because it's way more readable. The same goes for the for vs. while loops. It may be obvious for you because you're a C guru, but to me it just obfuscates the code. Too many C projects end up becoming completely unreadable because of patterns like these. Idiomatic C code isn't written by doing pointless micro-optimizations.
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
Hi Andy, On Mon, Feb 14, 2022 at 11:39 AM Andy Shevchenko wrote: > On Mon, Feb 14, 2022 at 10:03:53AM +0100, Thomas Zimmermann wrote: > > Am 11.02.22 um 16:41 schrieb Andy Shevchenko: > > > > IMO *always* prefer a for loop over while or do-while. > > > > > > > > The for (i = 0; i < N; i++) is such a strong paradigm in C. You > > > > instantly know how many times you're going to loop, at a glance. Not so > > > > with with the alternatives, which should be used sparingly. > > > > > > while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. > > > > Naw, that's not true. > > In the section 3.5 "Loops - While and For" in "The C Programming > Language" 2nd by K&R, the authors said: > > The for statement ... is equivalent to ... while..." > > They said that for is equivalent to while, and not otherwise. When I learned C, people told me to prefer while() over for() when possible, as several compilers are better at optimizing while()-loops than for()-loops. During the last 3 decades, optimizers got better, and all the bad old compilers went the way of the dodo (see also [1])... But even for a human, it's still less symbols to decode (and verify all the details about =//<=/>=/++/--/...) for while (n--) { ... } than for for (i = 0; i < n; i++) { ... } [1] https://lwn.net/Articles/871283/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`
On Mon, Feb 14, 2022 at 09:52:02AM +0100, Lukas Wunner wrote: > On Mon, Feb 14, 2022 at 09:34:26AM +0200, Mika Westerberg wrote: > > On Fri, Feb 11, 2022 at 03:45:46PM -0600, Bjorn Helgaas wrote: > > > My expectation is that "USB" (like "PCI" and "PCIe") tells me > > > something about how a device is electrically connected and how > > > software can operate it. It doesn't really tell me anything about > > > whether those electrical connections are permanent, made through an > > > internal slot, or made through an external connector and cable. > > > > It is used to identify "tunneled" ports (whether PCIe, USB 3.x or > > DisplayPort). Tunnels are created by software (in Linux it is the > > Thunderbolt driver) and are dynamic in nature. The USB4 links go over > > USB Type-C cable which also is something user can plug/unplug freely. > > > > I would say it is reasonable expectation that anything behind these > > ports can be assumed as "removable". > > USB gadgets may be soldered to the mainboard. Those cannot be > unplugged freely. It is common practice to solder USB Ethernet > or USB FTDI serial ports and nothing's preventing a vendor to solder > USB4/Thunderbolt gadgets. Right, that's why I say it is "reasonable expectation" that anything behind these ports can be assumed "removable" :) Of course they don't have to be but if we assume that in the driver where this actually matters we should be on the safe side, no?
Re: [PATCH 05/11] drm/radeon: remove resource accounting
On Mon, 14 Feb 2022 at 09:34, Christian König wrote: > > Use the one provided by TTM instead. > > Signed-off-by: Christian König > Tested-by: Bas Nieuwenhuizen > --- > drivers/gpu/drm/radeon/radeon.h| 2 -- > drivers/gpu/drm/radeon/radeon_kms.c| 7 -- > drivers/gpu/drm/radeon/radeon_object.c | 30 +++--- > drivers/gpu/drm/radeon/radeon_object.h | 1 - > drivers/gpu/drm/radeon/radeon_ttm.c| 18 ++-- > 5 files changed, 10 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index 895776c421d4..08f83bf2c330 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -2462,8 +2462,6 @@ struct radeon_device { > struct radeon_vm_managervm_manager; > struct mutexgpu_clock_mutex; > /* memory stats */ > - atomic64_t vram_usage; > - atomic64_t gtt_usage; > atomic64_t num_bytes_moved; > atomic_tgpu_reset_counter; > /* ACPI interface */ > diff --git a/drivers/gpu/drm/radeon/radeon_kms.c > b/drivers/gpu/drm/radeon/radeon_kms.c > index 11ad210919c8..965161b8565b 100644 > --- a/drivers/gpu/drm/radeon/radeon_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_kms.c > @@ -241,6 +241,7 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > struct drm_radeon_info *info = data; > struct radeon_mode_info *minfo = &rdev->mode_info; > uint32_t *value, value_tmp, *value_ptr, value_size; > + struct ttm_resource_manager *man; > uint64_t value64; > struct drm_crtc *crtc; > int i, found; > @@ -550,12 +551,14 @@ int radeon_info_ioctl(struct drm_device *dev, void > *data, struct drm_file *filp) > case RADEON_INFO_VRAM_USAGE: > value = (uint32_t*)&value64; > value_size = sizeof(uint64_t); > - value64 = atomic64_read(&rdev->vram_usage); > + man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM); > + value64 = ttm_resource_manager_usage(man); > break; > case RADEON_INFO_GTT_USAGE: > value = (uint32_t*)&value64; > value_size = sizeof(uint64_t); > - value64 = atomic64_read(&rdev->gtt_usage); > + man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_TT); > + value64 = ttm_resource_manager_usage(man); > break; > case RADEON_INFO_ACTIVE_CU_COUNT: > if (rdev->family >= CHIP_BONAIRE) > diff --git a/drivers/gpu/drm/radeon/radeon_object.c > b/drivers/gpu/drm/radeon/radeon_object.c > index 56ede9d63b12..c9bbed2a25ad 100644 > --- a/drivers/gpu/drm/radeon/radeon_object.c > +++ b/drivers/gpu/drm/radeon/radeon_object.c > @@ -49,27 +49,6 @@ static void radeon_bo_clear_surface_reg(struct radeon_bo > *bo); > * function are calling it. > */ > > -static void radeon_update_memory_usage(struct ttm_buffer_object *bo, > - unsigned int mem_type, int sign) > -{ > - struct radeon_device *rdev = radeon_get_rdev(bo->bdev); > - > - switch (mem_type) { > - case TTM_PL_TT: > - if (sign > 0) > - atomic64_add(bo->base.size, &rdev->gtt_usage); > - else > - atomic64_sub(bo->base.size, &rdev->gtt_usage); > - break; > - case TTM_PL_VRAM: > - if (sign > 0) > - atomic64_add(bo->base.size, &rdev->vram_usage); > - else > - atomic64_sub(bo->base.size, &rdev->vram_usage); > - break; > - } > -} > - > static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo) > { > struct radeon_bo *bo; > @@ -434,7 +413,9 @@ void radeon_bo_fini(struct radeon_device *rdev) > static u64 radeon_bo_get_threshold_for_moves(struct radeon_device *rdev) > { > u64 real_vram_size = rdev->mc.real_vram_size; > - u64 vram_usage = atomic64_read(&rdev->vram_usage); > + struct ttm_resource_manager *man = > + ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM); > + u64 vram_usage = ttm_resource_manager_usage(man); > > /* This function is based on the current VRAM usage. > * > @@ -725,15 +706,10 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool > has_moved, > } > > void radeon_bo_move_notify(struct ttm_buffer_object *bo, > - unsigned int old_type, >struct ttm_resource *new_mem) new_mem looks to also be unused now? > { > struct radeon_bo *rbo; > > - radeon_update_memory_usage(bo, old_type, -1); > - if (new_mem) > - radeon_update_memory_usage(bo, new_mem->mem_type, 1); > - > if (!radeon_ttm_
[syzbot] inconsistent lock state in sync_timeline_debug_remove
Hello, syzbot found the following issue on: HEAD commit:f4bc5bbb5fef Merge tag 'nfsd-5.17-2' of git://git.kernel.o.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=10fc74c270 kernel config: https://syzkaller.appspot.com/x/.config?x=266de9da75c71a45 dashboard link: https://syzkaller.appspot.com/bug?extid=7dcd254b8987a29f6450 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10c73c7470 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1440451c70 Bisection is inconclusive: the issue happens on the oldest tested release. bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=10a40d8470 final oops: https://syzkaller.appspot.com/x/report.txt?x=12a40d8470 console output: https://syzkaller.appspot.com/x/log.txt?x=14a40d8470 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+7dcd254b8987a29f6...@syzkaller.appspotmail.com WARNING: inconsistent lock state 5.17.0-rc3-syzkaller-00043-gf4bc5bbb5fef #0 Not tainted inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. syz-executor198/3596 [HC1[1]:SC0[0]:HE0:SE1] takes: 8c7096d8 (sync_timeline_list_lock){?.+.}-{2:2}, at: sync_timeline_debug_remove+0x25/0x190 drivers/dma-buf/sync_debug.c:31 {HARDIRQ-ON-W} state was registered at: __trace_hardirqs_on_caller kernel/locking/lockdep.c:4224 [inline] lockdep_hardirqs_on_prepare kernel/locking/lockdep.c:4292 [inline] lockdep_hardirqs_on_prepare+0x135/0x400 kernel/locking/lockdep.c:4244 trace_hardirqs_on+0x5b/0x1c0 kernel/trace/trace_preemptirq.c:49 __raw_spin_unlock_irq include/linux/spinlock_api_smp.h:159 [inline] _raw_spin_unlock_irq+0x1f/0x40 kernel/locking/spinlock.c:202 spin_unlock_irq include/linux/spinlock.h:399 [inline] sync_print_obj drivers/dma-buf/sync_debug.c:118 [inline] sync_info_debugfs_show+0xeb/0x200 drivers/dma-buf/sync_debug.c:153 seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230 seq_read+0x3e8/0x5c0 fs/seq_file.c:162 vfs_read+0x1b5/0x600 fs/read_write.c:479 ksys_read+0x12d/0x250 fs/read_write.c:619 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae irq event stamp: 5708 hardirqs last enabled at (5707): [] __raw_spin_unlock_irq include/linux/spinlock_api_smp.h:159 [inline] hardirqs last enabled at (5707): [] _raw_spin_unlock_irq+0x1f/0x40 kernel/locking/spinlock.c:202 hardirqs last disabled at (5708): [] sysvec_irq_work+0xb/0xc0 arch/x86/kernel/irq_work.c:17 softirqs last enabled at (5570): [] spin_unlock_bh include/linux/spinlock.h:394 [inline] softirqs last enabled at (5570): [] __tun_set_ebpf+0xf6/0x1c0 drivers/net/tun.c:2245 softirqs last disabled at (5568): [] spin_lock_bh include/linux/spinlock.h:354 [inline] softirqs last disabled at (5568): [] __tun_set_ebpf+0xa3/0x1c0 drivers/net/tun.c:2241 other info that might help us debug this: Possible unsafe locking scenario: CPU0 lock(sync_timeline_list_lock); lock(sync_timeline_list_lock); *** DEADLOCK *** no locks held by syz-executor198/3596. stack backtrace: CPU: 0 PID: 3596 Comm: syz-executor198 Not tainted 5.17.0-rc3-syzkaller-00043-gf4bc5bbb5fef #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106 print_usage_bug kernel/locking/lockdep.c:203 [inline] valid_state kernel/locking/lockdep.c:3945 [inline] mark_lock_irq kernel/locking/lockdep.c:4148 [inline] mark_lock.cold+0x61/0x8e kernel/locking/lockdep.c:4605 mark_usage kernel/locking/lockdep.c:4497 [inline] __lock_acquire+0x1499/0x5470 kernel/locking/lockdep.c:4981 lock_acquire kernel/locking/lockdep.c:5639 [inline] lock_acquire+0x1ab/0x510 kernel/locking/lockdep.c:5604 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x39/0x50 kernel/locking/spinlock.c:162 sync_timeline_debug_remove+0x25/0x190 drivers/dma-buf/sync_debug.c:31 sync_timeline_free drivers/dma-buf/sw_sync.c:104 [inline] kref_put include/linux/kref.h:65 [inline] sync_timeline_put drivers/dma-buf/sw_sync.c:116 [inline] timeline_fence_release+0x263/0x340 drivers/dma-buf/sw_sync.c:144 dma_fence_release+0x2ee/0x590 drivers/dma-buf/dma-fence.c:549 kref_put include/linux/kref.h:65 [inline] dma_fence_put include/linux/dma-fence.h:276 [inline] dma_fence_array_release+0x1e4/0x2b0 drivers/dma-buf/dma-fence-array.c:120 dma_fence_release+0x2ee/0x590 drivers/dma-buf/dma-fence.c:549 kref_put include/linux/kref.h:65 [inline] dma_fence_put include/linux/dma-fence.h:276 [inline] irq_dma_fence_array_work+0xa5/0xd0 drivers/dma-buf/dma-fence-array.c:52 irq_work_single+0x120/0x270 kern
Re: [PATCH v15 2/7] drm/ingenic: Add dw-hdmi driver specialization for jz4780
Hi Paul, > Am 14.02.2022 um 11:24 schrieb Paul Cercueil : > > Hi, > > Le sam., févr. 12 2022 at 16:50:50 +0100, H. Nikolaus Schaller > a écrit : >> +static void ingenic_dw_hdmi_cleanup(void *data) >> +{ >> +struct dw_hdmi *hdmi = (struct dw_hdmi *)data; >> + >> +dw_hdmi_remove(hdmi); >> +} >> + >> +static int ingenic_dw_hdmi_probe(struct platform_device *pdev) >> +{ >> +struct dw_hdmi *hdmi; >> + >> +hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data); >> +if (IS_ERR(hdmi)) >> +return PTR_ERR(hdmi); >> + >> +return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup, >> hdmi); > > I think I said it already, but in this driver you could use a .remove > callback, there's not much point in using devm cleanups in such a simple > setup. Well it was your suggestion after v8: https://lore.kernel.org/all/dia33r.qe29k7rkli...@crapouillou.net/ So we now almost go back to RFC v1 almost 2 years ago: https://patchwork.kernel.org/project/linux-mips/patch/2c131e1fb19e19f958a612f7186bc83f4afb0b0a.1582744379.git@goldelico.com/ Of course there was a good reason to better handle the regulator AND the dw_hdmi_remove() by a single mechanism. Now the regulator has gone and been replaced by the hdmi connector and we can go back. > > In your probe you could just: > return PTR_ERR_OR_ZERO(hdmi); No, this does not work since we need to platform_set_drvdata(). to be able to access the private struct in the remove callback. And checking errors after platform_set_drvdata() can be done but looks strange to me. It is up to you what you prefer. > >> +} >> + >> +static struct platform_driver ingenic_dw_hdmi_driver = { >> +.probe = ingenic_dw_hdmi_probe, >> +.driver = { >> +.name = "dw-hdmi-ingenic", >> +.of_match_table = ingenic_dw_hdmi_dt_ids, >> +}, >> +}; >> +module_platform_driver(ingenic_dw_hdmi_driver); >> + >> +MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension"); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_ALIAS("platform:dwhdmi-ingenic"); > > Should probably be "platform:dw-hdmi-ingenic"? Yes, indeed. Thanks for spotting! Was also good in v1. Probably someone deleted the hyphen unnoticed during editing of "jz4780" to "ingenic"... BR and thanks, Nikolaus
Re: [PATCH 06/11] drm/amdgpu: remove GTT accounting v2
On Mon, 14 Feb 2022 at 09:34, Christian König wrote: > > This is provided by TTM now. > > Also switch man->size to bytes instead of pages and fix the double > printing of size and usage in debugfs. > > v2: fix size checking as well > > Signed-off-by: Christian König > Tested-by: Bas Nieuwenhuizen > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 + > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 - > 4 files changed, 16 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > index e0c7fbe01d93..3bcd27ae379d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device > *dev, > struct ttm_resource_manager *man; > > man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT); > - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE); > + return sysfs_emit(buf, "%llu\n", man->size); > } > > /** > @@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device > *dev, > { > struct drm_device *ddev = dev_get_drvdata(dev); > struct amdgpu_device *adev = drm_to_adev(ddev); > + struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager; > > - return sysfs_emit(buf, "%llu\n", > amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr)); > + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man)); > } > > static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO, > @@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct > ttm_resource_manager *man, > struct amdgpu_gtt_node *node; > int r; > > - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && > - atomic64_add_return(num_pages, &mgr->used) > man->size) { > - atomic64_sub(num_pages, &mgr->used); I guess this behaviour is now slightly different, since TEMPORARY will now get accounted like everything else. Hopefully that is not a concern. Otherwise seems mechanical, Reviewed-by: Matthew Auld > - return -ENOSPC; > - } > - > node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL); > - if (!node) { > - r = -ENOMEM; > - goto err_out; > - } > + if (!node) > + return -ENOMEM; > > node->tbo = tbo; > ttm_resource_init(tbo, place, &node->base.base); > + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && > + ttm_resource_manager_usage(man) > man->size) { > + r = -ENOSPC; > + goto err_free; > + } > > if (place->lpfn) { > spin_lock(&mgr->lock); > @@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct > ttm_resource_manager *man, > err_free: > ttm_resource_fini(man, &node->base.base); > kfree(node); > - > -err_out: > - if (!(place->flags & TTM_PL_FLAG_TEMPORARY)) > - atomic64_sub(num_pages, &mgr->used); > - > return r; > } > > @@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct > ttm_resource_manager *man, > drm_mm_remove_node(&node->base.mm_nodes[0]); > spin_unlock(&mgr->lock); > > - if (!(res->placement & TTM_PL_FLAG_TEMPORARY)) > - atomic64_sub(res->num_pages, &mgr->used); > - > ttm_resource_fini(man, res); > kfree(node); > } > > -/** > - * amdgpu_gtt_mgr_usage - return usage of GTT domain > - * > - * @mgr: amdgpu_gtt_mgr pointer > - * > - * Return how many bytes are used in the GTT domain > - */ > -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr) > -{ > - return atomic64_read(&mgr->used) * PAGE_SIZE; > -} > - > /** > * amdgpu_gtt_mgr_recover - re-init gart > * > @@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct > ttm_resource_manager *man, > spin_lock(&mgr->lock); > drm_mm_print(&mgr->mm, printer); > spin_unlock(&mgr->lock); > - > - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n", > - man->size, atomic64_read(&mgr->used)); > } > > static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = { > @@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, > uint64_t gtt_size) > man->use_tt = true; > man->func = &amdgpu_gtt_mgr_func; > > - ttm_resource_manager_init(man, &adev->mman.bdev, > - gtt_size >> PAGE_SHIFT); > + ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size); > > start = AMDGPU_GTT_MAX_TRANSFER_SIZE * > AMDGPU_GTT_NUM_TRANSFER_WINDOWS; > size = (adev->gmc.gart_size >> PAGE_SHIFT) - start; > drm_mm_init(&mgr->mm, start, size); > spin_lock_init(&mgr->lock); > - atomic64_set(&mgr->u
Re: [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`
On Mon, Feb 14, 2022 at 01:11:05PM +0200, Mika Westerberg wrote: > > > It is used to identify "tunneled" ports (whether PCIe, USB 3.x or > > > DisplayPort). Tunnels are created by software (in Linux it is the > > > Thunderbolt driver) and are dynamic in nature. The USB4 links go over > > > USB Type-C cable which also is something user can plug/unplug freely. > > > > > > I would say it is reasonable expectation that anything behind these > > > ports can be assumed as "removable". > > > > USB gadgets may be soldered to the mainboard. Those cannot be > > unplugged freely. It is common practice to solder USB Ethernet > > or USB FTDI serial ports and nothing's preventing a vendor to solder > > USB4/Thunderbolt gadgets. > > Right, that's why I say it is "reasonable expectation" that anything > behind these ports can be assumed "removable" :) Of course they don't > have to be but if we assume that in the driver where this actually > matters we should be on the safe side, no? Also the tunnels are not permanent anyway.
Re: [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting
On Mon, 14 Feb 2022 at 09:34, Christian König wrote: > > This is provided by TTM now. > > Signed-off-by: Christian König Reviewed-by: Matthew Auld
Re: [PATCH 08/11] drm/amdgpu: remove VRAM accounting v2
On Mon, 14 Feb 2022 at 09:34, Christian König wrote: > > This is provided by TTM now. > > Also switch man->size to bytes instead of pages and fix the double > printing of size and usage in debugfs. > > v2: fix size checking as well > > Signed-off-by: Christian König > Tested-by: Bas Nieuwenhuizen Reviewed-by: Matthew Auld
Re: [PATCH v15 2/7] drm/ingenic: Add dw-hdmi driver specialization for jz4780
Le lun., févr. 14 2022 at 12:02:53 +0100, H. Nikolaus Schaller a écrit : Hi Paul, Am 14.02.2022 um 11:24 schrieb Paul Cercueil : Hi, Le sam., févr. 12 2022 at 16:50:50 +0100, H. Nikolaus Schaller a écrit : +static void ingenic_dw_hdmi_cleanup(void *data) +{ + struct dw_hdmi *hdmi = (struct dw_hdmi *)data; + + dw_hdmi_remove(hdmi); +} + +static int ingenic_dw_hdmi_probe(struct platform_device *pdev) +{ + struct dw_hdmi *hdmi; + + hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data); + if (IS_ERR(hdmi)) + return PTR_ERR(hdmi); + + return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup, hdmi); I think I said it already, but in this driver you could use a .remove callback, there's not much point in using devm cleanups in such a simple setup. Well it was your suggestion after v8: https://lore.kernel.org/all/dia33r.qe29k7rkli...@crapouillou.net/ It made sense for your v8, not so much for your v15... So we now almost go back to RFC v1 almost 2 years ago: https://patchwork.kernel.org/project/linux-mips/patch/2c131e1fb19e19f958a612f7186bc83f4afb0b0a.1582744379.git@goldelico.com/ Of course there was a good reason to better handle the regulator AND the dw_hdmi_remove() by a single mechanism. Now the regulator has gone and been replaced by the hdmi connector and we can go back. In your probe you could just: return PTR_ERR_OR_ZERO(hdmi); No, this does not work since we need to platform_set_drvdata(). to be able to access the private struct in the remove callback. And checking errors after platform_set_drvdata() can be done but looks strange to me. Yeah, I guess it would look strange. Fine then. Then I guess just merge your current [6/7] with this one (and make sure it comes after your current [5/7]) and it looks mergeable to me. Cheers, -Paul It is up to you what you prefer. +} + +static struct platform_driver ingenic_dw_hdmi_driver = { + .probe = ingenic_dw_hdmi_probe, + .driver = { + .name = "dw-hdmi-ingenic", + .of_match_table = ingenic_dw_hdmi_dt_ids, + }, +}; +module_platform_driver(ingenic_dw_hdmi_driver); + +MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:dwhdmi-ingenic"); Should probably be "platform:dw-hdmi-ingenic"? Yes, indeed. Thanks for spotting! Was also good in v1. Probably someone deleted the hyphen unnoticed during editing of "jz4780" to "ingenic"...
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
Hi Am 14.02.22 um 11:38 schrieb Andy Shevchenko: On Mon, Feb 14, 2022 at 10:03:53AM +0100, Thomas Zimmermann wrote: Am 11.02.22 um 16:41 schrieb Andy Shevchenko: ... IMO *always* prefer a for loop over while or do-while. The for (i = 0; i < N; i++) is such a strong paradigm in C. You instantly know how many times you're going to loop, at a glance. Not so with with the alternatives, which should be used sparingly. while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. Naw, that's not true. In the section 3.5 "Loops - While and For" in "The C Programming Language" 2nd by K&R, the authors said: Year of publication: 1988 . It's not the most up-to-date reference for C programming. The for statement ... is equivalent to ... while..." They said that for is equivalent to while, and not otherwise. Even leaving readability aside, it's not equivalent. You can declare variables as part of the for statement. (I know it's not the kernel's style.) Also, 'continue' statements are not well-suited in for loops, because it's non-obvious if the loop's update statement is being executed. (It isn't.) Also, syntax sugar by definition declares something that can be written as a single line of code, which usually is done using more (not always). The discussion has entered the phase of hair splitting. Good. Best regards Thomas An idiomatic for loop, such as for (i = ...; i < N; ++i), is such a strong pattern that it's way better than the corresponding while loop. And yes, the do-while suggested above is buggy, and you actually need to stop and think to see why. It depends if pixels can be 0 or not and if it's not, then does it contain last or number. The do {} while (--pixels); might be buggy iff pixels may be 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
Re: [PATCH 1/2] edid: parse DRM VESA dsc bpp target
Subject prefix should be drm/edid. On Sun, 13 Feb 2022, Yaroslav Bolyukin wrote: > As per DisplayID v2.0 Errata E9 spec Please be more elaborate about the changes. You also need to update drm_reset_display_info(). BR, Jani. > Signed-off-by: Yaroslav Bolyukin > --- > drivers/gpu/drm/drm_edid.c | 31 --- > include/drm/drm_connector.h | 6 ++ > include/drm/drm_displayid.h | 4 > 3 files changed, 30 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index a7663f9a1..83ee685c8 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -5270,7 +5270,7 @@ static void drm_parse_vesa_mso_data(struct > drm_connector *connector, > if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI) > return; > > - if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { > + if (block->num_bytes < 5) { > drm_dbg_kms(connector->dev, "Unexpected VESA vendor block > size\n"); > return; > } > @@ -5290,20 +5290,29 @@ static void drm_parse_vesa_mso_data(struct > drm_connector *connector, > break; > } > > - if (!info->mso_stream_count) { > - info->mso_pixel_overlap = 0; > - return; > + info->mso_pixel_overlap = 0; > + > + if (info->mso_stream_count) { > + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, > vesa->mso); > + if (info->mso_pixel_overlap > 8) { > + drm_dbg_kms(connector->dev, "Reserved MSO pixel overlap > value %u\n", > + info->mso_pixel_overlap); > + info->mso_pixel_overlap = 8; > + } > + > + drm_dbg_kms(connector->dev, "MSO stream count %u, pixel overlap > %u\n", > + info->mso_stream_count, info->mso_pixel_overlap); > } > > - info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, > vesa->mso); > - if (info->mso_pixel_overlap > 8) { > - drm_dbg_kms(connector->dev, "Reserved MSO pixel overlap value > %u\n", > - info->mso_pixel_overlap); > - info->mso_pixel_overlap = 8; > + if (block->num_bytes < 7) { > + /* DSC bpp is optional */ > + return; > } > > - drm_dbg_kms(connector->dev, "MSO stream count %u, pixel overlap %u\n", > - info->mso_stream_count, info->mso_pixel_overlap); > + info->dp_dsc_bpp = FIELD_GET(DISPLAYID_VESA_DSC_BPP_INT, > vesa->dsc_bpp_int) * 16 + > + FIELD_GET(DISPLAYID_VESA_DSC_BPP_FRACT, vesa->dsc_bpp_fract); > + > + drm_dbg_kms(connector->dev, "DSC bits per pixel %u\n", > info->dp_dsc_bpp); > } > > static void drm_update_mso(struct drm_connector *connector, const struct > edid *edid) > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 5e36eb3df..04ef0e995 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -634,6 +634,12 @@ struct drm_display_info { >* @mso_pixel_overlap: eDP MSO segment pixel overlap, 0-8 pixels. >*/ > u8 mso_pixel_overlap; > + > + /** > + * @dp_dsc_bpp: DP Display-Stream-Compression (DSC) timing's target > + * DST bits per pixel in 6.4 fixed point format. 0 means undefined > + */ > + u16 dp_dsc_bpp; > }; > > int drm_display_info_set_bus_formats(struct drm_display_info *info, > diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h > index 7ffbd9f7b..1be6deddc 100644 > --- a/include/drm/drm_displayid.h > +++ b/include/drm/drm_displayid.h > @@ -131,12 +131,16 @@ struct displayid_detailed_timing_block { > > #define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0) > #define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5) > +#define DISPLAYID_VESA_DSC_BPP_INT GENMASK(5, 0) > +#define DISPLAYID_VESA_DSC_BPP_FRACT GENMASK(3, 0) > > struct displayid_vesa_vendor_specific_block { > struct displayid_block base; > u8 oui[3]; > u8 data_structure_type; > u8 mso; > + u8 dsc_bpp_int; > + u8 dsc_bpp_fract; > } __packed; > > /* DisplayID iteration */ > > base-commit: 1528038385c0a706aac9ac165eeb24044fef6825 -- Jani Nikula, Intel Open Source Graphics Center
Re: Regression from 3c196f056666 ("drm/amdgpu: always reset the asic in suspend (v2)") on suspend?
[TLDR: I'm adding the regression report below to regzbot, the Linux kernel regression tracking bot; all text you find below is compiled from a few templates paragraphs you might have encountered already already from similar mails.] Hi, this is your Linux kernel regression tracker speaking. CCing the regression mailing list, as it should be in the loop for all regressions, as explained here: https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html To be sure this issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression tracking bot: #regzbot ^introduced 3c196f05 #regzbot title amdgfx: suspend stopped working #regzbot ignore-activity #regzbot link: https://bugs.debian.org/1005005 Reminder for developers: when fixing the issue, please add a 'Link:' tags pointing to the report (the mail quoted above) using lore.kernel.org/r/, as explained in 'Documentation/process/submitting-patches.rst' and 'Documentation/process/5.Posting.rst'. This allows the bot to connect the report with any patches posted or committed to fix the issue; this again allows the bot to show the current status of regressions and automatically resolve the issue when the fix hits the right tree. I'm sending this to everyone that got the initial report, to make them aware of the tracking. I also hope that messages like this motivate people to directly get at least the regression mailing list and ideally even regzbot involved when dealing with regressions, as messages like this wouldn't be needed then. Don't worry, I'll send further messages wrt to this regression just to the lists (with a tag in the subject so people can filter them away), if they are relevant just for regzbot. With a bit of luck no such messages will be needed anyway. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) P.S.: As the Linux kernel's regression tracker I'm getting a lot of reports on my table. I can only look briefly into most of them and lack knowledge about most of the areas they concern. I thus unfortunately will sometimes get things wrong or miss something important. I hope that's not the case here; if you think it is, don't hesitate to tell me in a public reply, it's in everyone's interest to set the public record straight. On 12.02.22 19:23, Salvatore Bonaccorso wrote: > Hi Alex, hi all > > In Debian we got a regression report from Dominique Dumont, CC'ed in > https://bugs.debian.org/1005005 that afer an update to 5.15.15 based > kernel, his machine noe longer suspends correctly, after screen going > black as usual it comes back. The Debian bug above contians a trace. > > Dominique confirmed that this issue persisted after updating to 5.16.7 > furthermore he bisected the issue and found > > 3c196f0510912645c7c5d9107706003f67c3 is the first bad commit > commit 3c196f0510912645c7c5d9107706003f67c3 > Author: Alex Deucher > Date: Fri Nov 12 11:25:30 2021 -0500 > > drm/amdgpu: always reset the asic in suspend (v2) > > [ Upstream commit daf8de0874ab5b74b38a38726fdd3d07ef98a7ee ] > > If the platform suspend happens to fail and the power rail > is not turned off, the GPU will be in an unknown state on > resume, so reset the asic so that it will be in a known > good state on resume even if the platform suspend failed. > > v2: handle s0ix > > Acked-by: Luben Tuikov > Acked-by: Evan Quan > Signed-off-by: Alex Deucher > Signed-off-by: Sasha Levin > >drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 - >1 file changed, 4 insertions(+), 1 deletion(-) > > to be the first bad commit, see https://bugs.debian.org/1005005#34 . > > Does this ring any bell? Any idea on the problem? > > Regards, > Salvatore -- Additional information about regzbot: If you want to know more about regzbot, check out its web-interface, the getting start guide, and the references documentation: https://linux-regtracking.leemhuis.info/regzbot/ https://gitlab.com/knurd42/regzbot/-/blob/main/docs/getting_started.md https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md The last two documents will explain how you can interact with regzbot yourself if your want to. Hint for reporters: when reporting a regression it's in your interest to CC the regression list and tell regzbot about the issue, as that ensures the regression makes it onto the radar of the Linux kernel's regression tracker -- that's in your interest, as it ensures your report won't fall through the cracks unnoticed. Hint for developers: you normally don't need to care about regzbot once it's involved. Fix the issue as you normally would, just remember to include 'Link:' tag in the patch descriptions pointing to all reports about the issue. This has been expected from developers even before regzbot showed up for reasons explained in 'Documentation/process/subm
Re: [PATCH v15 1/7] drm/ingenic: Fix support for JZ4780 HDMI output
Hi, Le lun., févr. 14 2022 at 11:19:40 +0100, H. Nikolaus Schaller a écrit : Hi Paul, Am 14.02.2022 um 11:13 schrieb Paul Cercueil : Hi, Le sam., févr. 12 2022 at 16:50:49 +0100, H. Nikolaus Schaller a écrit : From: Paul Boddie We have to make sure that - JZ_LCD_OSDC_ALPHAEN is set - plane f0 is disabled and not seen from user-space Actually it will still be seen from user-space, but it won't be possible to use it. So before applying I'll change this to: "plane f0 is disabled as it's not working yet" If that's OK with you. Yes. You understand much better than me the implications... I reworded it to "plane f0 is disabled as it's not working yet", added a Fixes: tag, and pushed this patch to drm-misc-next. Cheers, -Paul
Re: [PATCH v15 1/7] drm/ingenic: Fix support for JZ4780 HDMI output
Hi Paul, > Am 14.02.2022 um 13:29 schrieb Paul Cercueil : > > Hi, > > Le lun., févr. 14 2022 at 11:19:40 +0100, H. Nikolaus Schaller > a écrit : >> Hi Paul, >>> Am 14.02.2022 um 11:13 schrieb Paul Cercueil : >>> Hi, >>> Le sam., févr. 12 2022 at 16:50:49 +0100, H. Nikolaus Schaller >>> a écrit : From: Paul Boddie We have to make sure that - JZ_LCD_OSDC_ALPHAEN is set - plane f0 is disabled and not seen from user-space >>> Actually it will still be seen from user-space, but it won't be possible to >>> use it. So before applying I'll change this to: >>> "plane f0 is disabled as it's not working yet" >>> If that's OK with you. >> Yes. You understand much better than me the implications... > > I reworded it to "plane f0 is disabled as it's not working yet", added a > Fixes: tag, and pushed this patch to drm-misc-next. great and thanks. So I drop it from v16. BR and thanks, Nikolaus
Re: [PATCH v15 1/7] drm/ingenic: Fix support for JZ4780 HDMI output
Hi, Le lun., févr. 14 2022 at 13:33:25 +0100, H. Nikolaus Schaller a écrit : Hi Paul, Am 14.02.2022 um 13:29 schrieb Paul Cercueil : Hi, Le lun., févr. 14 2022 at 11:19:40 +0100, H. Nikolaus Schaller a écrit : Hi Paul, Am 14.02.2022 um 11:13 schrieb Paul Cercueil : Hi, Le sam., févr. 12 2022 at 16:50:49 +0100, H. Nikolaus Schaller a écrit : From: Paul Boddie We have to make sure that - JZ_LCD_OSDC_ALPHAEN is set - plane f0 is disabled and not seen from user-space Actually it will still be seen from user-space, but it won't be possible to use it. So before applying I'll change this to: "plane f0 is disabled as it's not working yet" If that's OK with you. Yes. You understand much better than me the implications... I reworded it to "plane f0 is disabled as it's not working yet", added a Fixes: tag, and pushed this patch to drm-misc-next. great and thanks. So I drop it from v16. Neil told me he'd review patch [4/7] this week so maybe hold off v16 for a few days. Cheers, -Paul
Re: [PATCH 20/23] ASoC: codecs: wcd938x: Make use of the helper component_compare/release_of
On Mon, Feb 14, 2022 at 02:08:16PM +0800, Yong Wu wrote: > Use the common compare/release helpers from component. What's the story with dependencies here? I've just got this one patch with no cover letter... signature.asc Description: PGP signature
Re: [PATCH 20/23] ASoC: codecs: wcd938x: Make use of the helper component_compare/release_of
Il 14/02/22 13:40, Mark Brown ha scritto: On Mon, Feb 14, 2022 at 02:08:16PM +0800, Yong Wu wrote: Use the common compare/release helpers from component. What's the story with dependencies here? I've just got this one patch with no cover letter... Hello Mark, I agree, the cover letter should be sent to everyone; Yong, please add the proper Ccs to it next time. Anyway, context: https://patchwork.kernel.org/project/linux-mediatek/cover/20220214060819.7334-1-yong...@mediatek.com/ Cheers, Angelo
Re: [PATCH v7 1/4] drm/bridge: anx7625: send DPCD command to downstream
On Sun, 13 Feb 2022 at 11:34, Hsin-Yi Wang wrote: > > From: Xin Ji > > Send DPCD command to downstream before anx7625 power down, > let downstream monitor enter into standby mode. > > Signed-off-by: Xin Ji > Signed-off-by: Hsin-Yi Wang > Reviewed-by: Hsin-Yi Wang > --- > v3->v4: > Use common DP_AUX_NATIVE_READ/WRITE > > Previously in: > https://patchwork.kernel.org/project/dri-devel/patch/1f36f8bf0a48fb2bba17bacec23700e58c1d407d.1641891874.git@analogixsemi.com/ > --- > drivers/gpu/drm/bridge/analogix/anx7625.c | 42 +++ > drivers/gpu/drm/bridge/analogix/anx7625.h | 2 -- > 2 files changed, 35 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > b/drivers/gpu/drm/bridge/analogix/anx7625.c > index 76662fce4ce61d..17b23940549a42 100644 > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > @@ -129,6 +129,23 @@ static int anx7625_reg_write(struct anx7625_data *ctx, > return ret; > } > > +static int anx7625_reg_block_write(struct anx7625_data *ctx, > + struct i2c_client *client, > + u8 reg_addr, u8 len, u8 *buf) > +{ > + int ret; > + struct device *dev = &client->dev; > + > + i2c_access_workaround(ctx, client); > + > + ret = i2c_smbus_write_i2c_block_data(client, reg_addr, len, buf); > + if (ret < 0) > + dev_err(dev, "write i2c block failed id=%x\n:%x", > + client->addr, reg_addr); > + > + return ret; > +} > + > static int anx7625_write_or(struct anx7625_data *ctx, > struct i2c_client *client, > u8 offset, u8 mask) > @@ -214,8 +231,8 @@ static int wait_aux_op_finish(struct anx7625_data *ctx) > return 0; > } > > -static int anx7625_aux_dpcd_read(struct anx7625_data *ctx, > -u32 address, u8 len, u8 *buf) > +static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx, u8 op, > + u32 address, u8 len, u8 *buf) > { > struct device *dev = &ctx->client->dev; > int ret; > @@ -231,8 +248,7 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx, > addrm = (address >> 8) & 0xFF; > addrh = (address >> 16) & 0xFF; > > - cmd = DPCD_CMD(len, DPCD_READ); > - cmd = ((len - 1) << 4) | 0x09; > + cmd = DPCD_CMD(len, op); > > /* Set command and length */ > ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > @@ -246,6 +262,9 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx, > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > AP_AUX_ADDR_19_16, addrh); > > + if (op == DP_AUX_NATIVE_WRITE) > + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p0_client, > + AP_AUX_BUFF_START, len, buf); > /* Enable aux access */ > ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, > AP_AUX_CTRL_STATUS, AP_AUX_CTRL_OP_EN); > @@ -255,14 +274,17 @@ static int anx7625_aux_dpcd_read(struct anx7625_data > *ctx, > return -EIO; > } > > - usleep_range(2000, 2100); > - > ret = wait_aux_op_finish(ctx); > if (ret) { > dev_err(dev, "aux IO error: wait aux op finish.\n"); > return ret; > } > > + /* Write done */ > + if (op == DP_AUX_NATIVE_WRITE) > + return 0; > + > + /* Read done, read out dpcd data */ > ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client, > AP_AUX_BUFF_START, len, buf); > if (ret < 0) { > @@ -845,7 +867,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx) > } > > /* Read downstream capability */ > - anx7625_aux_dpcd_read(ctx, 0x68028, 1, &bcap); > + anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, &bcap); > if (!(bcap & 0x01)) { > pr_warn("downstream not support HDCP 1.4, cap(%x).\n", bcap); > return 0; > @@ -918,6 +940,7 @@ static void anx7625_dp_stop(struct anx7625_data *ctx) > { > struct device *dev = &ctx->client->dev; > int ret; > + u8 data; > > DRM_DEV_DEBUG_DRIVER(dev, "stop dp output\n"); > > @@ -929,6 +952,11 @@ static void anx7625_dp_stop(struct anx7625_data *ctx) > ret |= anx7625_write_and(ctx, ctx->i2c.tx_p2_client, 0x08, 0x7f); > > ret |= anx7625_video_mute_control(ctx, 1); > + > + dev_dbg(dev, "notify downstream enter into standby\n"); > + /* Downstream monitor enter into standby mode */ > + data = 2; > + ret |= anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_WRITE, 0x000600, 1, > &data); > if (ret < 0) > DRM_DEV_ERROR(dev, "IO error : mute video fail\n"); > >
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
On Mon, Feb 14, 2022 at 01:12:48PM +0100, Thomas Zimmermann wrote: > Hi > > Am 14.02.22 um 11:38 schrieb Andy Shevchenko: > > On Mon, Feb 14, 2022 at 10:03:53AM +0100, Thomas Zimmermann wrote: > >> Am 11.02.22 um 16:41 schrieb Andy Shevchenko: > > > > ... > > > IMO *always* prefer a for loop over while or do-while. > > The for (i = 0; i < N; i++) is such a strong paradigm in C. You > instantly know how many times you're going to loop, at a glance. Not so > with with the alternatives, which should be used sparingly. > >>> > >>> while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. > >> > >> Naw, that's not true. > > > > In the section 3.5 "Loops - While and For" in "The C Programming > > Language" 2nd by K&R, the authors said: > > Year of publication: 1988 . It's not the most up-to-date reference for C > programming. > > > > > The for statement ... is equivalent to ... while..." > > > > They said that for is equivalent to while, and not otherwise. > > Even leaving readability aside, it's not equivalent. You can declare > variables as part of the for statement. (I know it's not the kernel's > style.) Also, 'continue' statements are not well-suited in for loops, > because it's non-obvious if the loop's update statement is being > executed. (It isn't.) It is. 'continue' is just shorthand for 'goto end_of_loop_body'. -- Ville Syrjälä Intel
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
Hi Am 14.02.22 um 13:47 schrieb Ville Syrjälä: On Mon, Feb 14, 2022 at 01:12:48PM +0100, Thomas Zimmermann wrote: Hi Am 14.02.22 um 11:38 schrieb Andy Shevchenko: On Mon, Feb 14, 2022 at 10:03:53AM +0100, Thomas Zimmermann wrote: Am 11.02.22 um 16:41 schrieb Andy Shevchenko: ... IMO *always* prefer a for loop over while or do-while. The for (i = 0; i < N; i++) is such a strong paradigm in C. You instantly know how many times you're going to loop, at a glance. Not so with with the alternatives, which should be used sparingly. while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. Naw, that's not true. In the section 3.5 "Loops - While and For" in "The C Programming Language" 2nd by K&R, the authors said: Year of publication: 1988 . It's not the most up-to-date reference for C programming. The for statement ... is equivalent to ... while..." They said that for is equivalent to while, and not otherwise. Even leaving readability aside, it's not equivalent. You can declare variables as part of the for statement. (I know it's not the kernel's style.) Also, 'continue' statements are not well-suited in for loops, because it's non-obvious if the loop's update statement is being executed. (It isn't.) It is. 'continue' is just shorthand for 'goto end_of_loop_body'. Well, indeed. lol Fun fact: I actually had to look this up and still got it wrong. Let me just count it under proving-my-point: continue in a for statement is a bad idea and for isn't equivalent to while. Best regards Thomas -- 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: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
On Mon, Feb 14, 2022 at 01:54:59PM +0100, Thomas Zimmermann wrote: > Hi > > Am 14.02.22 um 13:47 schrieb Ville Syrjälä: > > On Mon, Feb 14, 2022 at 01:12:48PM +0100, Thomas Zimmermann wrote: > >> Hi > >> > >> Am 14.02.22 um 11:38 schrieb Andy Shevchenko: > >>> On Mon, Feb 14, 2022 at 10:03:53AM +0100, Thomas Zimmermann wrote: > Am 11.02.22 um 16:41 schrieb Andy Shevchenko: > >>> > >>> ... > >>> > >> IMO *always* prefer a for loop over while or do-while. > >> > >> The for (i = 0; i < N; i++) is such a strong paradigm in C. You > >> instantly know how many times you're going to loop, at a glance. Not so > >> with with the alternatives, which should be used sparingly. > > > > while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. > > Naw, that's not true. > >>> > >>> In the section 3.5 "Loops - While and For" in "The C Programming > >>> Language" 2nd by K&R, the authors said: > >> > >> Year of publication: 1988 . It's not the most up-to-date reference for C > >> programming. > >> > >>> > >>> The for statement ... is equivalent to ... while..." > >>> > >>> They said that for is equivalent to while, and not otherwise. > >> > >> Even leaving readability aside, it's not equivalent. You can declare > >> variables as part of the for statement. (I know it's not the kernel's > >> style.) Also, 'continue' statements are not well-suited in for loops, > >> because it's non-obvious if the loop's update statement is being > >> executed. (It isn't.) > > > > It is. > > > > 'continue' is just shorthand for 'goto end_of_loop_body'. > > Well, indeed. lol > > Fun fact: I actually had to look this up and still got it wrong. Let me > just count it under proving-my-point: continue in a for statement is a > bad idea and for isn't equivalent to while. Nah. We use 'continue' a *lot* in for loops in kms/atomic code. I'd be surprised if you can find many loops without a 'continue'. Looking at the loc stats I was a bit surprised to see more 'break' but then I realized switch() is bloating up those numbers quite a bit. -- Ville Syrjälä Intel
Re: [PATCH v15 1/7] drm/ingenic: Fix support for JZ4780 HDMI output
> Am 14.02.2022 um 13:36 schrieb Paul Cercueil : > > Hi, > > Le lun., févr. 14 2022 at 13:33:25 +0100, H. Nikolaus Schaller > a écrit : >> Hi Paul, >>> Am 14.02.2022 um 13:29 schrieb Paul Cercueil : >>> Hi, >>> Le lun., févr. 14 2022 at 11:19:40 +0100, H. Nikolaus Schaller >>> a écrit : Hi Paul, > Am 14.02.2022 um 11:13 schrieb Paul Cercueil : > Hi, > Le sam., févr. 12 2022 at 16:50:49 +0100, H. Nikolaus Schaller > a écrit : >> From: Paul Boddie >> We have to make sure that >> - JZ_LCD_OSDC_ALPHAEN is set >> - plane f0 is disabled and not seen from user-space > Actually it will still be seen from user-space, but it won't be possible > to use it. So before applying I'll change this to: > "plane f0 is disabled as it's not working yet" > If that's OK with you. Yes. You understand much better than me the implications... >>> I reworded it to "plane f0 is disabled as it's not working yet", added a >>> Fixes: tag, and pushed this patch to drm-misc-next. >> great and thanks. >> So I drop it from v16. > > Neil told me he'd review patch [4/7] this week so maybe hold off v16 for a > few days. Ok. BR and thanks, Nikolaus
Re: [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
Am 14.02.22 um 11:34 schrieb Matthew Auld: On Mon, 14 Feb 2022 at 09:34, Christian König wrote: It makes sense to have this in the common manager for debugging and accounting of how much resources are used. v2: cleanup kerneldoc a bit v3: drop the atomic, update counter under lock instead Signed-off-by: Christian König Reviewed-by: Huang Rui (v1) Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/ttm/ttm_resource.c | 30 ++ include/drm/ttm/ttm_resource.h | 11 +-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index ae40e144e728..bbb8a0f7aa14 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo, const struct ttm_place *place, struct ttm_resource *res) { + struct ttm_resource_manager *man; + res->start = 0; res->num_pages = PFN_UP(bo->base.size); res->mem_type = place->mem_type; @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo, res->bus.is_iomem = false; res->bus.caching = ttm_cached; res->bo = bo; + + man = ttm_manager_type(bo->bdev, place->mem_type); + spin_lock(&bo->bdev->lru_lock); + man->usage += bo->base.size; + spin_unlock(&bo->bdev->lru_lock); } EXPORT_SYMBOL(ttm_resource_init); @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init); void ttm_resource_fini(struct ttm_resource_manager *man, struct ttm_resource *res) { + spin_lock(&man->bdev->lru_lock); + man->usage -= res->bo->base.size; + spin_unlock(&man->bdev->lru_lock); } EXPORT_SYMBOL(ttm_resource_fini); @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man, spin_lock_init(&man->move_lock); man->bdev = bdev; man->size = size; + man->usage = 0; for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) INIT_LIST_HEAD(&man->lru[i]); @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev, } EXPORT_SYMBOL(ttm_resource_manager_evict_all); +/** + * ttm_resource_manager_usage + * + * @man: A memory manager object. + * + * Return how many resources are currently used. Maybe mention the units here? "Return how many resources are currently used, in bytes." Well exactly that's not correct. The whole idea here is that these are driver defined units. E.g. for the AMDGPU OA and GWS resources it's essentially a hardware block. Regards, Christian. Anyway, Reviewed-by: Matthew Auld + */ +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man) +{ + uint64_t usage; + + spin_lock(&man->bdev->lru_lock); + usage = man->usage; + spin_unlock(&man->bdev->lru_lock); + return usage; +} +EXPORT_SYMBOL(ttm_resource_manager_usage); + /** * ttm_resource_manager_debug * @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man, drm_printf(p, " use_type: %d\n", man->use_type); drm_printf(p, " use_tt: %d\n", man->use_tt); drm_printf(p, " size: %llu\n", man->size); + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man)); if (man->func->debug) man->func->debug(man, p); } diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index 555a11fb8a7f..323c14a30c6b 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -130,10 +131,15 @@ struct ttm_resource_manager { struct dma_fence *move; /* -* Protected by the global->lru_lock. +* Protected by the bdev->lru_lock. */ - struct list_head lru[TTM_MAX_BO_PRIORITY]; + + /** +* @usage: How much of the resources are used, protected by the +* bdev->lru_lock. +*/ + uint64_t usage; }; /** @@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man, int ttm_resource_manager_evict_all(struct ttm_device *bdev, struct ttm_resource_manager *man); +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man); void ttm_resource_manager_debug(struct ttm_resource_manager *man, struct drm_printer *p); -- 2.25.1
Re: [PATCH 2/2] fbdev: Don't sort deferred-I/O pages by default
Hi Am 14.02.22 um 10:05 schrieb Geert Uytterhoeven: Hi Thomas, On Mon, Feb 14, 2022 at 9:28 AM Thomas Zimmermann wrote: Am 14.02.22 um 09:05 schrieb Geert Uytterhoeven: On Thu, Feb 10, 2022 at 4:24 PM Thomas Zimmermann wrote: Fbdev's deferred I/O sorts all dirty pages by default, which incurs a significant overhead. Make the sorting step optional and update the few drivers that require it. Use a FIFO list by default. Sorting pages by memory offset for deferred I/O performs an implicit bubble-sort step on the list of dirty pages. The algorithm goes through the list of dirty pages and inserts each new page according to its index field. Even worse, list traversal always starts at the first entry. As video memory is most likely updated scanline by scanline, the algorithm traverses through the complete list for each updated page. For example, with 1024x768x32bpp a page covers exactly one scanline. Writing a single screen update from top to bottom requires updating 768 pages. With an average list length of 384 entries, a screen update creates (768 * 384 =) 294912 compare operation. What about using folios? If consecutive pages are merged into a single entry, there's much less (or nothing in the example above) to sort. How would the code know that? Calls to page_mkwrite happen pagefault-by-pagefault in any order AFAICT. fb_deferred_io_mkwrite() would still be called for a page, but an adjacent page can be merged with an existing entry while adding it to the list. I still don't understand how we'd use it to our advantage. Most drivers don't need sorted pages at all. A folio has strong alignment requirements for size and offset AFAICT. We might end up flushing way too much of the display memory. Best regards Thomas Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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: [PATCH 06/11] drm/amdgpu: remove GTT accounting v2
Am 14.02.22 um 12:10 schrieb Matthew Auld: On Mon, 14 Feb 2022 at 09:34, Christian König wrote: This is provided by TTM now. Also switch man->size to bytes instead of pages and fix the double printing of size and usage in debugfs. v2: fix size checking as well Signed-off-by: Christian König Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 + drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 - 4 files changed, 16 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index e0c7fbe01d93..3bcd27ae379d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev, struct ttm_resource_manager *man; man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT); - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE); + return sysfs_emit(buf, "%llu\n", man->size); } /** @@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev, { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = drm_to_adev(ddev); + struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager; - return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr)); + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man)); } static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO, @@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, struct amdgpu_gtt_node *node; int r; - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && - atomic64_add_return(num_pages, &mgr->used) > man->size) { - atomic64_sub(num_pages, &mgr->used); I guess this behaviour is now slightly different, since TEMPORARY will now get accounted like everything else. Hopefully that is not a concern. Yeah, that's correct but also unproblematic. See a few lines below. Otherwise seems mechanical, Reviewed-by: Matthew Auld - return -ENOSPC; - } - node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL); - if (!node) { - r = -ENOMEM; - goto err_out; - } + if (!node) + return -ENOMEM; node->tbo = tbo; ttm_resource_init(tbo, place, &node->base.base); + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && + ttm_resource_manager_usage(man) > man->size) { + r = -ENOSPC; + goto err_free; + } This here now takes care of TTM_PL_FLAG_TEMPORARY. Regards, Christian. if (place->lpfn) { spin_lock(&mgr->lock); @@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, err_free: ttm_resource_fini(man, &node->base.base); kfree(node); - -err_out: - if (!(place->flags & TTM_PL_FLAG_TEMPORARY)) - atomic64_sub(num_pages, &mgr->used); - return r; } @@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, drm_mm_remove_node(&node->base.mm_nodes[0]); spin_unlock(&mgr->lock); - if (!(res->placement & TTM_PL_FLAG_TEMPORARY)) - atomic64_sub(res->num_pages, &mgr->used); - ttm_resource_fini(man, res); kfree(node); } -/** - * amdgpu_gtt_mgr_usage - return usage of GTT domain - * - * @mgr: amdgpu_gtt_mgr pointer - * - * Return how many bytes are used in the GTT domain - */ -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr) -{ - return atomic64_read(&mgr->used) * PAGE_SIZE; -} - /** * amdgpu_gtt_mgr_recover - re-init gart * @@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man, spin_lock(&mgr->lock); drm_mm_print(&mgr->mm, printer); spin_unlock(&mgr->lock); - - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n", - man->size, atomic64_read(&mgr->used)); } static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = { @@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size) man->use_tt = true; man->func = &amdgpu_gtt_mgr_func; - ttm_resource_manager_init(man, &adev->mman.bdev, - gtt_size >> PAGE_SHIFT); + ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size); start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS; size = (adev->gmc.gart_size >> PAGE_SHIFT) - start; drm_mm_init(&mgr->mm, start, size); spin_lock_init(&mgr->lock); - atomic64_set(&mgr->used, 0); ttm_s
[PATCH v6 0/6] drm: Add driver for Solomon SSD130x OLED displays
This patch series adds a DRM driver for the Solomon OLED SSD1305, SSD1306, SSD1307 and SSD1309 displays. It is a port of the ssd1307fb fbdev driver. Using the DRM fbdev emulation, all the tests from Geert Uytterhoeven repo (https://git.kernel.org/pub/scm/linux/kernel/git/geert/fbtest.git) passes. I've also tested it using the display as a VT output and even though fbcon seems to work, it is mostly unusable on a 128x64 SSD1306 display. This is a v6 that addresses the issues pointed in v5. Thanks a lot to all reviewers that gave me feedback and comments. I didn't include the patch that adds the SPI support this time, because it will require changes in the existing Device Tree binding. And I wanted to avoid that bikesheeding for now, to focus on the core and I2C parts. Once this series land, I'll post patches for the SPI support. But the WIP patch posted in v3 should still apply cleanly on top of this v6: https://patchwork.kernel.org/project/dri-devel/patch/20220209091204.2513437-1-javi...@redhat.com/ Patch #1 splits per-line conversion logic in drm_fb_xrgb_to_gray8() to a separate drm_fb_xrgb_to_gray8_line() helper function. Patch #2 adds a new drm_fb_xrgb_to_mono_reversed() helper function to convert from XR24 to reversed monochrome. The latter internally converts each line first to 8-bit grayscale and then to 1-bit reversed monochrome. Patch #3 adds the driver. This only has the core support and doesn't have any bus specific code, separate drivers are needed for the transport used. Patch #4 adds a driver to use the I2C bus to communicate with the device. Patch #5 adds a MAINTAINERS entry for the DRM driver and patch #6 adds myself as co-maintainer of the existing DT binding for the ssd1307fb, since the same DT binding is used for both the fbdev and DRM drivers. Best regards, Javier Changes in v6: - Add Andy Shevchenko's Reviewed-by to patch #1. - Add Andy Shevchenko's Reviewed-by to patch #2. - Just return regmap_bulk_write() in ssd130x_write_data() (Andy Shevchenko) - Remove unnecessary cast in ssd130x_write_cmd() (Andy Shevchenko) - Return ERR_PTR(dev_err_probe(...)) everywhere in probe (Andy Shevchenko) Changes in v5: - Add Thomas Zimmermann's Reviewed-by to patch #1. - Use drm_WARN_ON* macros instead of deprecated ones (Thomas Zimmermann) - Include header (Andy Shevchenko) - Drop parenthesis for command options macros (Andy Shevchenko) - Explain in ssd130x_write_cmd() comment how commands are sent (Andy Shevchenko) - The pwm_*() functions check for NULL already (Andy Shevchenko) - Remove unnecesary blank line (Andy Shevchenko) - Simplify error handling for backlight registration failure (Geert Uytterhoeven) - Don't clear screen on enable, instead send the full buffer (Thomas Zimmermann) - Add Andy Shevchenko's Reviewed-by tag to patch #4. - Add Andy Shevchenko's Reviewed-by tag to patch #5. - Add Andy Shevchenko's Reviewed-by tag to patch #6. Changes in v4: - Rename end_offset to end_len (Thomas Zimmermann) - Warn once if dst_pitch is not a multiple of 8 (Thomas Zimmermann) - Drop drm_fb_gray8_to_mono_reversed() that's not used (Thomas Zimmermann) - Allocate single buffer for both copy cma memory and gray8 (Thomas Zimmermann) - Add Thomas Zimmermann Reviewed-by tag to patch adding XR24 -> mono helper. - Rename vbat supply to vcc since is how's labeled in the device (Mark Brown) - Don't make the regulator option since is always needed (Mark Brown) - Add solomon Kconfig source and directory inclusion sorted (Andy Shevchenko) - Use SSD130x instead of SSD130X to denote is not a model name (Andy Shevchenko) - Check if there's a reset pin in the callee and not the caller (Andy Shevchenko) - Define missing commands instead of using magic numbers (Andy Shevchenko) - Use GENMASK() and FIELD_PREP() macros when possible (Andy Shevchenko) - Avoid using ternary operators to ease code readablity (Andy Shevchenko) - Use i++ instead of --i on some for loops (Andy Shevchenko) - Remove redundant blank lines (Andy Shevchenko) - Rename power_off label to out_power_off (Andy Shevchenko) - Use dev_err_probe() even if no -EPROBE_DEFER (Andy Shevchenko) - Don't use plural Authors if there's only one (Andy Shevchenko) - Remove unnecessary casting (Geert Uytterhoeven) - Remove redundant blank lines (Andy Shevchenko) - Remove comma after of_device_id table terminator (Andy Shevchenko) - Add Rob Herring Acked-by tag to patch adding as DT binding co-maintainer. Changes in v3: - Add a drm_fb_xrgb_to_gray8_line() helper function (Thomas Zimmermann) - Also add a drm_fb_xrgb_to_mono_reversed() helper (Thomas Zimmermann) - Split lines copy to drm_fb_gray8_to_mono_reversed_line() (Thomas Zimmermann) - Handle case where the source buffer is not aligned to 8 (Thomas Zimmermann) - Move driver from tiny sub-dir to drivers/gpu/drm/solomon (Sam Ravnborg) - Split driver in a bus agnostic core and bus specific (Andy Shevchenko) - Use regmap to access the chip registers (Andy Shevchenko) - Remove unnecessar
[PATCH v6 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
Pull the per-line conversion logic into a separate helper function. This will allow to do line-by-line conversion in other helpers that convert to a gray8 format. Suggested-by: Thomas Zimmermann Signed-off-by: Javier Martinez Canillas Reviewed-by: Thomas Zimmermann Reviewed-by: Andy Shevchenko --- Changes in v6: - Add Andy Shevchenko's Reviewed-by to patch #1. Changes in v5: - Add Thomas Zimmermann's Reviewed-by to patch #1. Changes in v3: - Add a drm_fb_xrgb_to_gray8_line() helper function (Thomas Zimmermann) drivers/gpu/drm/drm_format_helper.c | 31 ++--- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 0f28dd2bdd72..b981712623d3 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -464,6 +464,21 @@ void drm_fb_xrgb_to_xrgb2101010_toio(void __iomem *dst, } EXPORT_SYMBOL(drm_fb_xrgb_to_xrgb2101010_toio); +static void drm_fb_xrgb_to_gray8_line(u8 *dst, const u32 *src, unsigned int pixels) +{ + unsigned int x; + + for (x = 0; x < pixels; x++) { + u8 r = (*src & 0x00ff) >> 16; + u8 g = (*src & 0xff00) >> 8; + u8 b = *src & 0x00ff; + + /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */ + *dst++ = (3 * r + 6 * g + b) / 10; + src++; + } +} + /** * drm_fb_xrgb_to_gray8 - Convert XRGB to grayscale * @dst: 8-bit grayscale destination buffer @@ -484,8 +499,9 @@ EXPORT_SYMBOL(drm_fb_xrgb_to_xrgb2101010_toio); 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; + unsigned int linepixels = clip->x2 - clip->x1; + unsigned int len = linepixels * sizeof(u32); + unsigned int y; void *buf; u8 *dst8; u32 *src32; @@ -508,16 +524,7 @@ void drm_fb_xrgb_to_gray8(void *dst, unsigned int dst_pitch, const void *vad for (y = clip->y1; y < clip->y2; y++) { dst8 = dst; src32 = memcpy(buf, vaddr, len); - for (x = clip->x1; x < clip->x2; x++) { - 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 */ - *dst8++ = (3 * r + 6 * g + b) / 10; - src32++; - } - + drm_fb_xrgb_to_gray8_line(dst8, src32, linepixels); vaddr += fb->pitches[0]; dst += dst_pitch; } -- 2.34.1
[PATCH v6 4/6] drm/solomon: Add SSD130x OLED displays I2C support
The ssd130x driver only provides the core support for these devices but it does not have any bus transport logic. Add a driver to interface over I2C. Signed-off-by: Javier Martinez Canillas Reviewed-by: Andy Shevchenko --- (no changes since v5) Changes in v5: - Add Andy Shevchenko's Reviewed-by tag to patch #4. Changes in v4: - Remove unnecessary casting (Geert Uytterhoeven) - Remove redundant blank lines (Andy Shevchenko) - Remove comma after of_device_id table terminator (Andy Shevchenko) Changes in v3: - Add a separate driver for SSD130X chips I2C support (Andy Shevchenko) drivers/gpu/drm/solomon/Kconfig | 9 ++ drivers/gpu/drm/solomon/Makefile | 1 + drivers/gpu/drm/solomon/ssd130x-i2c.c | 116 ++ 3 files changed, 126 insertions(+) create mode 100644 drivers/gpu/drm/solomon/ssd130x-i2c.c diff --git a/drivers/gpu/drm/solomon/Kconfig b/drivers/gpu/drm/solomon/Kconfig index 7720a7039e8d..5861c3ab7c45 100644 --- a/drivers/gpu/drm/solomon/Kconfig +++ b/drivers/gpu/drm/solomon/Kconfig @@ -10,3 +10,12 @@ config DRM_SSD130X the appropriate bus transport in your chip also must be selected. If M is selected the module will be called ssd130x. + +config DRM_SSD130X_I2C + tristate "DRM support for Solomon SSD130x OLED displays (I2C bus)" + depends on DRM_SSD130X && I2C + select REGMAP_I2C + help + Say Y here if the SSD130x OLED display is connected via I2C bus. + + If M is selected the module will be called ssd130x-i2c. diff --git a/drivers/gpu/drm/solomon/Makefile b/drivers/gpu/drm/solomon/Makefile index f685addb19fe..4bfc5acb0447 100644 --- a/drivers/gpu/drm/solomon/Makefile +++ b/drivers/gpu/drm/solomon/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_DRM_SSD130X) += ssd130x.o +obj-$(CONFIG_DRM_SSD130X_I2C) += ssd130x-i2c.o diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd130x-i2c.c new file mode 100644 index ..3126aeda4ced --- /dev/null +++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * DRM driver for Solomon SSD130x OLED displays (I2C bus) + * + * Copyright 2022 Red Hat Inc. + * Author: Javier Martinez Canillas + * + * Based on drivers/video/fbdev/ssd1307fb.c + * Copyright 2012 Free Electrons + */ +#include +#include + +#include "ssd130x.h" + +#define DRIVER_NAME"ssd130x-i2c" +#define DRIVER_DESC"DRM driver for Solomon SSD130x OLED displays (I2C)" + +static const struct regmap_config ssd130x_i2c_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +static int ssd130x_i2c_probe(struct i2c_client *client) +{ + struct ssd130x_device *ssd130x; + struct regmap *regmap; + + regmap = devm_regmap_init_i2c(client, &ssd130x_i2c_regmap_config); + if (IS_ERR(regmap)) + return PTR_ERR(regmap); + + ssd130x = ssd130x_probe(&client->dev, regmap); + if (IS_ERR(ssd130x)) + return PTR_ERR(ssd130x); + + i2c_set_clientdata(client, ssd130x); + + return 0; +} + +static int ssd130x_i2c_remove(struct i2c_client *client) +{ + struct ssd130x_device *ssd130x = i2c_get_clientdata(client); + + return ssd130x_remove(ssd130x); +} + +static void ssd130x_i2c_shutdown(struct i2c_client *client) +{ + struct ssd130x_device *ssd130x = i2c_get_clientdata(client); + + ssd130x_shutdown(ssd130x); +} + +static struct ssd130x_deviceinfo ssd130x_ssd1305_deviceinfo = { + .default_vcomh = 0x34, + .default_dclk_div = 1, + .default_dclk_frq = 7, +}; + +static struct ssd130x_deviceinfo ssd130x_ssd1306_deviceinfo = { + .default_vcomh = 0x20, + .default_dclk_div = 1, + .default_dclk_frq = 8, + .need_chargepump = 1, +}; + +static struct ssd130x_deviceinfo ssd130x_ssd1307_deviceinfo = { + .default_vcomh = 0x20, + .default_dclk_div = 2, + .default_dclk_frq = 12, + .need_pwm = 1, +}; + +static struct ssd130x_deviceinfo ssd130x_ssd1309_deviceinfo = { + .default_vcomh = 0x34, + .default_dclk_div = 1, + .default_dclk_frq = 10, +}; + +static const struct of_device_id ssd130x_of_match[] = { + { + .compatible = "solomon,ssd1305fb-i2c", + .data = &ssd130x_ssd1305_deviceinfo, + }, + { + .compatible = "solomon,ssd1306fb-i2c", + .data = &ssd130x_ssd1306_deviceinfo, + }, + { + .compatible = "solomon,ssd1307fb-i2c", + .data = &ssd130x_ssd1307_deviceinfo, + }, + { + .compatible = "solomon,ssd1309fb-i2c", + .data = &ssd130x_ssd1309_deviceinfo, + }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, ssd130x_of_match); + +static struct i2c_driver ssd130x_i2c_driver = { + .driver = { + .name = DRIVER_NAME, + .of_match_table = ssd130x_of_match, + }, + .probe_ne
[PATCH v6 2/6] drm/format-helper: Add drm_fb_xrgb8888_to_mono_reversed()
Add support to convert from XR24 to reversed monochrome for drivers that control monochromatic display panels, that only have 1 bit per pixel. The function does a line-by-line conversion doing an intermediate step first from XR24 to 8-bit grayscale and then to reversed monochrome. The drm_fb_gray8_to_mono_reversed_line() helper was based on code from drivers/gpu/drm/tiny/repaper.c driver. Signed-off-by: Javier Martinez Canillas Reviewed-by: Thomas Zimmermann Reviewed-by: Andy Shevchenko --- Changes in v6: - Add Andy Shevchenko's Reviewed-by to patch #2. Changes in v5: - Use drm_WARN_ON* macros instead of deprecated ones (Thomas Zimmermann) Changes in v4: - Rename end_offset to end_len (Thomas Zimmermann) - Warn once if dst_pitch is not a multiple of 8 (Thomas Zimmermann) - Drop drm_fb_gray8_to_mono_reversed() that's not used (Thomas Zimmermann) - Allocate single buffer for both copy cma memory and gray8 (Thomas Zimmermann) - Add Thomas Zimmermann Reviewed-by tag to patch adding XR24 -> mono helper. Changes in v3: - Also add a drm_fb_xrgb_to_mono_reversed() helper (Thomas Zimmermann) - Split lines copy to drm_fb_gray8_to_mono_reversed_line() (Thomas Zimmermann) - Handle case where the source buffer is not aligned to 8 (Thomas Zimmermann) drivers/gpu/drm/drm_format_helper.c | 110 include/drm/drm_format_helper.h | 4 + 2 files changed, 114 insertions(+) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index b981712623d3..bc0f49773868 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -12,9 +12,11 @@ #include #include +#include #include #include #include +#include #include static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp) @@ -591,3 +593,111 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for return -EINVAL; } EXPORT_SYMBOL(drm_fb_blit_toio); + +static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src, unsigned int pixels, + unsigned int start_offset, unsigned int end_len) +{ + unsigned int xb, i; + + for (xb = 0; xb < pixels; xb++) { + unsigned int start = 0, end = 8; + u8 byte = 0x00; + + if (xb == 0 && start_offset) + start = start_offset; + + if (xb == pixels - 1 && end_len) + end = end_len; + + for (i = start; i < end; i++) { + unsigned int x = xb * 8 + i; + + byte >>= 1; + if (src[x] >> 7) + byte |= BIT(7); + } + *dst++ = byte; + } +} + +/** + * drm_fb_xrgb_to_mono_reversed - Convert XRGB to reversed monochrome + * @dst: reversed monochrome 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 + * + * DRM doesn't have native monochrome support. + * Such drivers can announce the commonly supported XR24 format to userspace + * and use this function to convert to the native format. + * + * This function uses drm_fb_xrgb_to_gray8() to convert to grayscale and + * then the result is converted from grayscale to reversed monohrome. + */ +void drm_fb_xrgb_to_mono_reversed(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip) +{ + unsigned int linepixels = drm_rect_width(clip); + unsigned int lines = clip->y2 - clip->y1; + unsigned int cpp = fb->format->cpp[0]; + unsigned int len_src32 = linepixels * cpp; + struct drm_device *dev = fb->dev; + unsigned int start_offset, end_len; + unsigned int y; + u8 *mono = dst, *gray8; + u32 *src32; + + if (drm_WARN_ON(dev, fb->format->format != DRM_FORMAT_XRGB)) + return; + + /* +* The reversed mono destination buffer contains 1 bit per pixel +* and destination scanlines have to be in multiple of 8 pixels. +*/ + if (!dst_pitch) + dst_pitch = DIV_ROUND_UP(linepixels, 8); + + drm_WARN_ONCE(dev, dst_pitch % 8 != 0, "dst_pitch is not a multiple of 8\n"); + + /* +* The cma memory is write-combined so reads are uncached. +* Speed up by fetching one line at a time. +* +* Also, format conversion from XR24 to reversed monochrome +* are done line-by-line but are converted to 8-bit grayscale +* as an intermediate step. +* +* Allocate a buffer to be used for both copying from the cma +* memory and to store the intermediate grayscale line pixels. +*/ + src32 = km
[PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
This adds a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon OLED display controllers. It's only the core part of the driver and a bus specific driver is needed for each transport interface supported by the display controllers. Signed-off-by: Javier Martinez Canillas --- Changes in v6: - Just return regmap_bulk_write() in ssd130x_write_data() (Andy Shevchenko) - Remove unnecessary cast in ssd130x_write_cmd() (Andy Shevchenko) - Return ERR_PTR(dev_err_probe(...)) everywhere in probe (Andy Shevchenko) Changes in v5: - Include header (Andy Shevchenko) - Drop parenthesis for command options macros (Andy Shevchenko) - Explain in ssd130x_write_cmd() comment how commands are sent (Andy Shevchenko) - The pwm_*() functions check for NULL already (Andy Shevchenko) - Remove unnecesary blank line (Andy Shevchenko) - Simplify error handling for backlight registration failure (Geert Uytterhoeven) - Don't clear screen on enable, instead send the full buffer (Thomas Zimmermann) Changes in v4: - Rename vbat supply to vcc since is how's labeled in the device (Mark Brown) - Don't make the regulator option since is always needed (Mark Brown) - Add solomon Kconfig source and directory inclusion sorted (Andy Shevchenko) - Use SSD130x instead of SSD130X to denote is not a model name (Andy Shevchenko) - Check if there's a reset pin in the callee and not the caller (Andy Shevchenko) - Define missing commands instead of using magic numbers (Andy Shevchenko) - Use GENMASK() and FIELD_PREP() macros when possible (Andy Shevchenko) - Avoid using ternary operators to ease code readablity (Andy Shevchenko) - Use i++ instead of --i on some for loops (Andy Shevchenko) - Remove redundant blank lines (Andy Shevchenko) - Rename power_off label to out_power_off (Andy Shevchenko) - Use dev_err_probe() even if no -EPROBE_DEFER (Andy Shevchenko) - Don't use plural Authors if there's only one (Andy Shevchenko) Changes in v3: - Move driver from tiny sub-dir to drivers/gpu/drm/solomon (Sam Ravnborg) - Split driver in a bus agnostic core and bus specific (Andy Shevchenko) - Use regmap to access the chip registers (Andy Shevchenko) - Remove unnecessary blank lines (Andy Shevchenko) - Remove unneeded inline specifier in functions (Andy Shevchenko) - Add a comment about always returning a single mode (Andy Shevchenko) - Change write command logic to use do while loop (Andy Shevchenko) - Use "firmware description" instead of "device tree" (Andy Shevchenko) - Use return foo() instead of returning the return value (Andy Shevchenko) - Don't split lines longer than 80 chars if makes less readable (Andy Shevchenko) - Remove redundant else statements in .mode_valid callback (Andy Shevchenko) - Rename powero{n,ff}() functions to power_o{n,ff)() (Andy Shevchenko) - Use dev_err_probe() to prevent spam logs on probe deferral (Andy Shevchenko) - Remove ',' after sentinel terminator in array (Andy Shevchenko) - Fix a bug when doing partial updates (Geert Uytterhoeven) Changes in v2: - Drop patch that was adding a DRM_MODE_CONNECTOR_I2C type. - Invert order of backlight {en,dis}able and display {on,off} (Sam Ravnborg) - Don't clear the screen and turn on display on probe (Sam Ravnborg) - Use backlight_get_brightness() macro to get BL brightness (Sam Ravnborg) - Use dev managed version of devm_backlight_device_register() (Sam Ravnborg) - Use dev_name(dev) for backlight name instead of an array (Sam Ravnborg) - Drop the .get_brightness callback since isn't needed (Sam Ravnborg) - Rename driver to ssd130x since supports a display family (Thomas Zimmermann) - Drop the TINY prefix from the Kconfig symbol (Thomas Zimmermann) - Sort the Kconfig symbol dependencies alphabetically (Thomas Zimmermann) - Rename struct ssd130x_array to struct ssd130x_i2c_msg (Thomas Zimmermann) - Rename struct ssd130x_i2c_msg .type member to .cmd (Thomas Zimmermann) - Use sizeof(*foo) instead of sizeof(struct foo) (Thomas Zimmermann) - Use struct_size() macro to calculate sizeof(*foo) + len (Thomas Zimmermann) - Use kcalloc() instead of kmalloc_array() + memset() (Thomas Zimmermann) - Use shadow plane helpers virtual screen support (Thomas Zimmermann) - Remove unused goto label in ssd1307_fb_blit_rect() (Thomas Zimmermann) - Use drm_set_preferred_mode() inset of manually set (Thomas Zimmermann) - Use shadow plane helpers virtual screen support (Thomas Zimmermann) - Remove unused goto label in ssd1307_fb_blit_rect() (Thomas Zimmermann) - Use drm_set_preferred_mode() inset of manually set (Thomas Zimmermann) - Reorganize code in probe to make it more legible (Thomas Zimmermann) - ssd130x_write_cmd() uses varargs to simplify I2C code (Thomas Zimmermann) - Move regulator/pwm init logic to display pipe enable callback. drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/solomon/Kconfig | 12 + drivers/gpu/drm/solomon/Makefile | 1 + drivers/gpu/drm/solomon/ssd130x.c | 843 ++ drivers/gpu/drm
[PATCH v6 5/6] MAINTAINERS: Add entry for Solomon SSD130x OLED displays DRM driver
To make sure that tools like the get_maintainer.pl script will suggest to Cc me if patches are posted for this driver. Also include the Device Tree binding for the old ssd1307fb fbdev driver since the new DRM driver was made compatible with the existing binding. Signed-off-by: Javier Martinez Canillas Acked-by: Sam Ravnborg Reviewed-by: Andy Shevchenko --- (no changes since v5) Changes in v5: - Add Andy Shevchenko's Reviewed-by tag to patch #5. Changes in v3: - Adapt MAINTAINERS entry to point to the new drivers/gpu/drm/solomon directory. Changes in v2: - Add Sam Ravnborg's acked-by to patch adding a MAINTAINERS entry (Sam Ravnborg) MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index d03ad8da1f36..05c306986ab0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6102,6 +6102,13 @@ T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/devicetree/bindings/display/repaper.txt F: drivers/gpu/drm/tiny/repaper.c +DRM DRIVER FOR SOLOMON SSD130X OLED DISPLAYS +M: Javier Martinez Canillas +S: Maintained +T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml +F: drivers/gpu/drm/solomon/ssd130x* + DRM DRIVER FOR QEMU'S CIRRUS DEVICE M: Dave Airlie M: Gerd Hoffmann -- 2.34.1
[PATCH v6 6/6] dt-bindings: display: ssd1307fb: Add myself as binding co-maintainer
The ssd130x DRM driver also makes use of this Device Tree binding to allow existing users of the fbdev driver to migrate without the need to change their Device Trees. Add myself as another maintainer of the binding, to make sure that I will be on Cc when patches are proposed for it. Suggested-by: Sam Ravnborg Signed-off-by: Javier Martinez Canillas Acked-by: Rob Herring Reviewed-by: Andy Shevchenko --- (no changes since v5) Changes in v5: - Add Andy Shevchenko's Reviewed-by tag to patch #6. Changes in v4: - Add Rob Herring Acked-by tag to patch adding as DT binding co-maintainer. Changes in v2: - Add myself as co-maintainer of the ssd1370fb DT binding (Sam Ravnborg). Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml index 2ed2a7d0ca2f..9baafd0c42dd 100644 --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml @@ -8,6 +8,7 @@ title: Solomon SSD1307 OLED Controller Framebuffer maintainers: - Maxime Ripard + - Javier Martinez Canillas properties: compatible: -- 2.34.1
Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()
On Mon, Feb 14, 2022 at 01:12:48PM +0100, Thomas Zimmermann wrote: > Am 14.02.22 um 11:38 schrieb Andy Shevchenko: > > On Mon, Feb 14, 2022 at 10:03:53AM +0100, Thomas Zimmermann wrote: > > > Am 11.02.22 um 16:41 schrieb Andy Shevchenko: ... > > > > > IMO *always* prefer a for loop over while or do-while. > > > > > > > > > > The for (i = 0; i < N; i++) is such a strong paradigm in C. You > > > > > instantly know how many times you're going to loop, at a glance. Not > > > > > so > > > > > with with the alternatives, which should be used sparingly. > > > > > > > > while () {} _is_ a paradigm, for-loop is syntax sugar on top of it. > > > > > > Naw, that's not true. > > > > In the section 3.5 "Loops - While and For" in "The C Programming > > Language" 2nd by K&R, the authors said: > > Year of publication: 1988 . It's not the most up-to-date reference for C > programming. Yet this makes your above remark invalid, i.e. `for` _is_ syntax sugar despite what you think it's idiomatic _nowadays_. > > The for statement ... is equivalent to ... while..." > > > > They said that for is equivalent to while, and not otherwise. > > Even leaving readability aside, it's not equivalent. You can declare > variables as part of the for statement. (I know it's not the kernel's > style.) Also, 'continue' statements are not well-suited in for loops, > because it's non-obvious if the loop's update statement is being executed. > (It isn't.) It's also written in the book :-) > > Also, syntax sugar by definition declares something that can be written as > > a single line of code, which usually is done using more (not always). > > The discussion has entered the phase of hair splitting. Good. I don't know why we are adding an oil into the flames... -- With Best Regards, Andy Shevchenko
Re: [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
On Mon, 14 Feb 2022 at 13:23, Christian König wrote: > > Am 14.02.22 um 11:34 schrieb Matthew Auld: > > On Mon, 14 Feb 2022 at 09:34, Christian König > > wrote: > >> It makes sense to have this in the common manager for debugging and > >> accounting of how much resources are used. > >> > >> v2: cleanup kerneldoc a bit > >> v3: drop the atomic, update counter under lock instead > >> > >> Signed-off-by: Christian König > >> Reviewed-by: Huang Rui (v1) > >> Tested-by: Bas Nieuwenhuizen > >> --- > >> drivers/gpu/drm/ttm/ttm_resource.c | 30 ++ > >> include/drm/ttm/ttm_resource.h | 11 +-- > >> 2 files changed, 39 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c > >> b/drivers/gpu/drm/ttm/ttm_resource.c > >> index ae40e144e728..bbb8a0f7aa14 100644 > >> --- a/drivers/gpu/drm/ttm/ttm_resource.c > >> +++ b/drivers/gpu/drm/ttm/ttm_resource.c > >> @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo, > >> const struct ttm_place *place, > >> struct ttm_resource *res) > >> { > >> + struct ttm_resource_manager *man; > >> + > >> res->start = 0; > >> res->num_pages = PFN_UP(bo->base.size); > >> res->mem_type = place->mem_type; > >> @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo, > >> res->bus.is_iomem = false; > >> res->bus.caching = ttm_cached; > >> res->bo = bo; > >> + > >> + man = ttm_manager_type(bo->bdev, place->mem_type); > >> + spin_lock(&bo->bdev->lru_lock); > >> + man->usage += bo->base.size; > >> + spin_unlock(&bo->bdev->lru_lock); > >> } > >> EXPORT_SYMBOL(ttm_resource_init); > >> > >> @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init); > >> void ttm_resource_fini(struct ttm_resource_manager *man, > >> struct ttm_resource *res) > >> { > >> + spin_lock(&man->bdev->lru_lock); > >> + man->usage -= res->bo->base.size; > >> + spin_unlock(&man->bdev->lru_lock); > >> } > >> EXPORT_SYMBOL(ttm_resource_fini); > >> > >> @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct > >> ttm_resource_manager *man, > >> spin_lock_init(&man->move_lock); > >> man->bdev = bdev; > >> man->size = size; > >> + man->usage = 0; > >> > >> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) > >> INIT_LIST_HEAD(&man->lru[i]); > >> @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device > >> *bdev, > >> } > >> EXPORT_SYMBOL(ttm_resource_manager_evict_all); > >> > >> +/** > >> + * ttm_resource_manager_usage > >> + * > >> + * @man: A memory manager object. > >> + * > >> + * Return how many resources are currently used. > > Maybe mention the units here? > > > > "Return how many resources are currently used, in bytes." > > Well exactly that's not correct. The whole idea here is that these are > driver defined units. Ok, I was assuming bo->base.size to operate in bytes(the kernel-doc seems to indicate that) and ttm_resource_{init, fini} are using this to track the man->usage. > > E.g. for the AMDGPU OA and GWS resources it's essentially a hardware block. > > Regards, > Christian. > > > > > Anyway, > > Reviewed-by: Matthew Auld > > > >> + */ > >> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man) > >> +{ > >> + uint64_t usage; > >> + > >> + spin_lock(&man->bdev->lru_lock); > >> + usage = man->usage; > >> + spin_unlock(&man->bdev->lru_lock); > >> + return usage; > >> +} > >> +EXPORT_SYMBOL(ttm_resource_manager_usage); > >> + > >> /** > >>* ttm_resource_manager_debug > >>* > >> @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct > >> ttm_resource_manager *man, > >> drm_printf(p, " use_type: %d\n", man->use_type); > >> drm_printf(p, " use_tt: %d\n", man->use_tt); > >> drm_printf(p, " size: %llu\n", man->size); > >> + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man)); > >> if (man->func->debug) > >> man->func->debug(man, p); > >> } > >> diff --git a/include/drm/ttm/ttm_resource.h > >> b/include/drm/ttm/ttm_resource.h > >> index 555a11fb8a7f..323c14a30c6b 100644 > >> --- a/include/drm/ttm/ttm_resource.h > >> +++ b/include/drm/ttm/ttm_resource.h > >> @@ -27,6 +27,7 @@ > >> > >> #include > >> #include > >> +#include > >> #include > >> #include > >> #include > >> @@ -130,10 +131,15 @@ struct ttm_resource_manager { > >> struct dma_fence *move; > >> > >> /* > >> -* Protected by the global->lru_lock. > >> +* Protected by the bdev->lru_lock. > >> */ > >> - > >> struct list_head lru[TTM_MAX_BO_PRIORITY]; > >> + > >> + /** > >> +* @usage: How much of the resources are used, protected by the > >> +* bdev->lru_lock. > >> +*/ > >
Re: [PATCH v2] drm/bridge: anx7625: Fix overflow issue on reading EDID
On Thu, 10 Feb 2022 at 11:38, Pin-Yen Lin wrote: > > The length of EDID block can be longer than 256 bytes, so we should use > `int` instead of `u8` for the `edid_pos` variable. > > Fixes: 8bdfc5dae4e3 ("drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP") > Signed-off-by: Pin-Yen Lin > Reviewed-by: Jernej Skrabec > --- > > Changes in v2: > - add "Fixes" tag > - collect review tag > > drivers/gpu/drm/bridge/analogix/anx7625.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > b/drivers/gpu/drm/bridge/analogix/anx7625.c > index 2346dbcc505f..e596cacce9e3 100644 > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > @@ -846,7 +846,8 @@ static int segments_edid_read(struct anx7625_data *ctx, > static int sp_tx_edid_read(struct anx7625_data *ctx, >u8 *pedid_blocks_buf) > { > - u8 offset, edid_pos; > + u8 offset; > + int edid_pos; > int count, blocks_num; > u8 pblock_buf[MAX_DPCD_BUFFER_SIZE]; > u8 i, j; > -- > 2.35.0.263.gb82422642f-goog > Applied to drm-misc-next.
Re: [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
Am 14.02.22 um 15:29 schrieb Matthew Auld: On Mon, 14 Feb 2022 at 13:23, Christian König wrote: Am 14.02.22 um 11:34 schrieb Matthew Auld: On Mon, 14 Feb 2022 at 09:34, Christian König wrote: It makes sense to have this in the common manager for debugging and accounting of how much resources are used. v2: cleanup kerneldoc a bit v3: drop the atomic, update counter under lock instead Signed-off-by: Christian König Reviewed-by: Huang Rui (v1) Tested-by: Bas Nieuwenhuizen --- drivers/gpu/drm/ttm/ttm_resource.c | 30 ++ include/drm/ttm/ttm_resource.h | 11 +-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index ae40e144e728..bbb8a0f7aa14 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo, const struct ttm_place *place, struct ttm_resource *res) { + struct ttm_resource_manager *man; + res->start = 0; res->num_pages = PFN_UP(bo->base.size); res->mem_type = place->mem_type; @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo, res->bus.is_iomem = false; res->bus.caching = ttm_cached; res->bo = bo; + + man = ttm_manager_type(bo->bdev, place->mem_type); + spin_lock(&bo->bdev->lru_lock); + man->usage += bo->base.size; + spin_unlock(&bo->bdev->lru_lock); } EXPORT_SYMBOL(ttm_resource_init); @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init); void ttm_resource_fini(struct ttm_resource_manager *man, struct ttm_resource *res) { + spin_lock(&man->bdev->lru_lock); + man->usage -= res->bo->base.size; + spin_unlock(&man->bdev->lru_lock); } EXPORT_SYMBOL(ttm_resource_fini); @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man, spin_lock_init(&man->move_lock); man->bdev = bdev; man->size = size; + man->usage = 0; for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) INIT_LIST_HEAD(&man->lru[i]); @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev, } EXPORT_SYMBOL(ttm_resource_manager_evict_all); +/** + * ttm_resource_manager_usage + * + * @man: A memory manager object. + * + * Return how many resources are currently used. Maybe mention the units here? "Return how many resources are currently used, in bytes." Well exactly that's not correct. The whole idea here is that these are driver defined units. Ok, I was assuming bo->base.size to operate in bytes(the kernel-doc seems to indicate that) and ttm_resource_{init, fini} are using this to track the man->usage. Yeah, good point. That one is the next on my TODO list to fix. Christian. E.g. for the AMDGPU OA and GWS resources it's essentially a hardware block. Regards, Christian. Anyway, Reviewed-by: Matthew Auld + */ +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man) +{ + uint64_t usage; + + spin_lock(&man->bdev->lru_lock); + usage = man->usage; + spin_unlock(&man->bdev->lru_lock); + return usage; +} +EXPORT_SYMBOL(ttm_resource_manager_usage); + /** * ttm_resource_manager_debug * @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man, drm_printf(p, " use_type: %d\n", man->use_type); drm_printf(p, " use_tt: %d\n", man->use_tt); drm_printf(p, " size: %llu\n", man->size); + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man)); if (man->func->debug) man->func->debug(man, p); } diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index 555a11fb8a7f..323c14a30c6b 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -130,10 +131,15 @@ struct ttm_resource_manager { struct dma_fence *move; /* -* Protected by the global->lru_lock. +* Protected by the bdev->lru_lock. */ - struct list_head lru[TTM_MAX_BO_PRIORITY]; + + /** +* @usage: How much of the resources are used, protected by the +* bdev->lru_lock. +*/ + uint64_t usage; }; /** @@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man, int ttm_resource_manager_evict_all(struct ttm_device *bdev, struct ttm_resource_manager *man); +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man); void ttm_resource_manager_debug(struct ttm_resource_manager *man, struct drm_printer *p); -- 2.25.1
[PATCH] drm/nouveau/bios: Use HWSQ entry 1 for PowerBook6,1
On PowerBook6,1 (PowerBook G4 867 12") HWSQ entry 0 (which is currently always used by nouveau) fails, but the BIOS declares 2 HWSQ entries and entry 1 works. Add a quirk to use HWSQ entry 1. Signed-off-by: Icenowy Zheng --- drivers/gpu/drm/nouveau/nouveau_bios.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c b/drivers/gpu/drm/nouveau/nouveau_bios.c index e8c445eb11004..2691d0e0cf9f1 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bios.c +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c @@ -1977,6 +1977,13 @@ static int load_nv17_hw_sequencer_ucode(struct drm_device *dev, if (!hwsq_offset) return 0; +#ifdef __powerpc__ + /* HWSQ entry 0 fails on PowerBook G4 867 12" (Al) */ + if (of_machine_is_compatible("PowerBook6,1")) + return load_nv17_hwsq_ucode_entry(dev, bios, + hwsq_offset + sz, 1); +#endif + /* always use entry 0? */ return load_nv17_hwsq_ucode_entry(dev, bios, hwsq_offset + sz, 0); } -- 2.30.2
Re: [PATCH] drm/nouveau/bios: Use HWSQ entry 1 for PowerBook6,1
I'm not saying this is wrong, but could you file a bug at gitlab.freedesktop.org/drm/nouveau/-/issues and include the VBIOS (/sys/kernel/debug/dri/0/vbios.rom)? That would make it easier to review the full situation. On Mon, Feb 14, 2022 at 11:03 AM Icenowy Zheng wrote: > > On PowerBook6,1 (PowerBook G4 867 12") HWSQ entry 0 (which is currently > always used by nouveau) fails, but the BIOS declares 2 HWSQ entries and > entry 1 works. > > Add a quirk to use HWSQ entry 1. > > Signed-off-by: Icenowy Zheng > --- > drivers/gpu/drm/nouveau/nouveau_bios.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c > b/drivers/gpu/drm/nouveau/nouveau_bios.c > index e8c445eb11004..2691d0e0cf9f1 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bios.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c > @@ -1977,6 +1977,13 @@ static int load_nv17_hw_sequencer_ucode(struct > drm_device *dev, > if (!hwsq_offset) > return 0; > > +#ifdef __powerpc__ > + /* HWSQ entry 0 fails on PowerBook G4 867 12" (Al) */ > + if (of_machine_is_compatible("PowerBook6,1")) > + return load_nv17_hwsq_ucode_entry(dev, bios, > + hwsq_offset + sz, 1); > +#endif > + > /* always use entry 0? */ > return load_nv17_hwsq_ucode_entry(dev, bios, hwsq_offset + sz, 0); > } > -- > 2.30.2 >
Re: [PATCH 04/11] drm/ttm: add resource iterator v2
Am 2022-02-14 um 04:34 schrieb Christian König: Instead of duplicating that at different places add an iterator over all the resources in a resource manager. v2: add lockdep annotation and kerneldoc Signed-off-by: Christian König Tested-by: Bas Nieuwenhuizen Reviewed-by: Daniel Vetter --- drivers/gpu/drm/ttm/ttm_bo.c | 41 ++--- drivers/gpu/drm/ttm/ttm_device.c | 26 +++- drivers/gpu/drm/ttm/ttm_resource.c | 49 ++ include/drm/ttm/ttm_resource.h | 23 ++ 4 files changed, 99 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index cb0fa932d495..599be3dda8a9 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -579,38 +579,29 @@ int ttm_mem_evict_first(struct ttm_device *bdev, struct ww_acquire_ctx *ticket) { struct ttm_buffer_object *bo = NULL, *busy_bo = NULL; + struct ttm_resource_cursor cursor; struct ttm_resource *res; bool locked = false; - unsigned i; int ret; spin_lock(&bdev->lru_lock); - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { - list_for_each_entry(res, &man->lru[i], lru) { - bool busy; - - bo = res->bo; - if (!ttm_bo_evict_swapout_allowable(bo, ctx, place, - &locked, &busy)) { - if (busy && !busy_bo && ticket != - dma_resv_locking_ctx(bo->base.resv)) - busy_bo = bo; - continue; - } - - if (!ttm_bo_get_unless_zero(bo)) { - if (locked) - dma_resv_unlock(bo->base.resv); - continue; - } - break; + ttm_resource_manager_for_each_res(man, &cursor, res) { + bool busy; + + if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place, + &locked, &busy)) { + if (busy && !busy_bo && ticket != + dma_resv_locking_ctx(bo->base.resv)) + busy_bo = res->bo; + continue; } - /* If the inner loop terminated early, we have our candidate */ - if (&res->lru != &man->lru[i]) - break; - - bo = NULL; + if (!ttm_bo_get_unless_zero(res->bo)) { + if (locked) + dma_resv_unlock(res->bo->base.resv); + continue; + } + bo = res->bo; } if (!bo) { diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index ba35887147ba..a0562ab386f5 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -142,10 +142,10 @@ EXPORT_SYMBOL(ttm_global_swapout); int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { + struct ttm_resource_cursor cursor; struct ttm_resource_manager *man; - struct ttm_buffer_object *bo; struct ttm_resource *res; - unsigned i, j; + unsigned i; int ret; spin_lock(&bdev->lru_lock); @@ -154,20 +154,16 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx, if (!man || !man->use_tt) continue; - for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) { - list_for_each_entry(res, &man->lru[j], lru) { - uint32_t num_pages; - - bo = res->bo; - num_pages = PFN_UP(bo->base.size); + ttm_resource_manager_for_each_res(man, &cursor, res) { + struct ttm_buffer_object *bo = res->bo; + uint32_t num_pages = PFN_UP(bo->base.size); -ret = ttm_bo_swapout(bo, ctx, gfp_flags); - /* ttm_bo_swapout has dropped the lru_lock */ - if (!ret) - return num_pages; - if (ret != -EBUSY) - return ret; - } + ret = ttm_bo_swapout(bo, ctx, gfp_flags); + /* ttm_bo_swapout has dropped the lru_lock */ + if (!ret) + return num_pages; + if (ret != -EBUSY) + return ret; } } spin_unlock(&bdev->lru_lock); diff --git
Re: [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting
Am 2022-02-14 um 04:34 schrieb Christian König: This is provided by TTM now. Signed-off-by: Christian König This patch is Reviewed-by: Felix Kuehling --- .../gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 62 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +-- 2 files changed, 6 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c index 0d85c2096ab5..e8adfd0a570a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c @@ -25,12 +25,6 @@ #include "amdgpu.h" -static inline struct amdgpu_preempt_mgr * -to_preempt_mgr(struct ttm_resource_manager *man) -{ - return container_of(man, struct amdgpu_preempt_mgr, manager); -} - /** * DOC: mem_info_preempt_used * @@ -45,10 +39,9 @@ static ssize_t mem_info_preempt_used_show(struct device *dev, { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = drm_to_adev(ddev); - struct ttm_resource_manager *man; + struct ttm_resource_manager *man = &adev->mman.preempt_mgr; - man = ttm_manager_type(&adev->mman.bdev, AMDGPU_PL_PREEMPT); - return sysfs_emit(buf, "%llu\n", amdgpu_preempt_mgr_usage(man)); + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man)); } static DEVICE_ATTR_RO(mem_info_preempt_used); @@ -68,16 +61,12 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager *man, const struct ttm_place *place, struct ttm_resource **res) { - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); - *res = kzalloc(sizeof(**res), GFP_KERNEL); if (!*res) return -ENOMEM; ttm_resource_init(tbo, place, *res); (*res)->start = AMDGPU_BO_INVALID_OFFSET; - - atomic64_add((*res)->num_pages, &mgr->used); return 0; } @@ -92,49 +81,13 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager *man, static void amdgpu_preempt_mgr_del(struct ttm_resource_manager *man, struct ttm_resource *res) { - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); - - atomic64_sub(res->num_pages, &mgr->used); ttm_resource_fini(man, res); kfree(res); } -/** - * amdgpu_preempt_mgr_usage - return usage of PREEMPT domain - * - * @man: TTM memory type manager - * - * Return how many bytes are used in the GTT domain - */ -uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man) -{ - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); - s64 result = atomic64_read(&mgr->used); - - return (result > 0 ? result : 0) * PAGE_SIZE; -} - -/** - * amdgpu_preempt_mgr_debug - dump VRAM table - * - * @man: TTM memory type manager - * @printer: DRM printer to use - * - * Dump the table content using printk. - */ -static void amdgpu_preempt_mgr_debug(struct ttm_resource_manager *man, -struct drm_printer *printer) -{ - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); - - drm_printf(printer, "man size:%llu pages, preempt used:%lld pages\n", - man->size, (u64)atomic64_read(&mgr->used)); -} - static const struct ttm_resource_manager_func amdgpu_preempt_mgr_func = { .alloc = amdgpu_preempt_mgr_new, .free = amdgpu_preempt_mgr_del, - .debug = amdgpu_preempt_mgr_debug }; /** @@ -146,8 +99,7 @@ static const struct ttm_resource_manager_func amdgpu_preempt_mgr_func = { */ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev) { - struct amdgpu_preempt_mgr *mgr = &adev->mman.preempt_mgr; - struct ttm_resource_manager *man = &mgr->manager; + struct ttm_resource_manager *man = &adev->mman.preempt_mgr; int ret; man->use_tt = true; @@ -155,16 +107,13 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev) ttm_resource_manager_init(man, &adev->mman.bdev, (1 << 30)); - atomic64_set(&mgr->used, 0); - ret = device_create_file(adev->dev, &dev_attr_mem_info_preempt_used); if (ret) { DRM_ERROR("Failed to create device file mem_info_preempt_used\n"); return ret; } - ttm_set_driver_manager(&adev->mman.bdev, AMDGPU_PL_PREEMPT, - &mgr->manager); + ttm_set_driver_manager(&adev->mman.bdev, AMDGPU_PL_PREEMPT, man); ttm_resource_manager_set_used(man, true); return 0; } @@ -179,8 +128,7 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev) */ void amdgpu_preempt_mgr_fini(struct amdgpu_device *adev) { - struct amdgpu_preempt_mgr *mgr = &adev->mman.preempt_mgr; - struct ttm_resource_manager *man = &mgr->manager; + struct ttm_resource_manager *man = &adev->mman.preempt_mgr; int ret; ttm_resource_manager_s
Re: [PATCH v8 00/10] vgaarb: Rework default VGA device selection
On Tue, Feb 01, 2022 at 04:46:33PM +0100, Maarten Lankhorst wrote: > Hey, > > Op 31-01-2022 om 23:23 schreef Bjorn Helgaas: > > [+to Maarten, Maxime, Thomas; beginning of thread: > > https://lore.kernel.org/r/20220106000658.243509-1-helg...@kernel.org] > > > > On Wed, Jan 05, 2022 at 06:06:48PM -0600, Bjorn Helgaas wrote: > >> From: Bjorn Helgaas > >> > >> Current default VGA device selection fails in some cases because > >> part of it is done in the vga_arb_device_init() subsys_initcall, > >> and some arches enumerate PCI devices in pcibios_init(), which > >> runs *after* that. > > > > Where are we at with this series? Is there anything I can do to > > move it forward? > > I'm afraid that I don't understand the vga arbiter or the vga code > well enough to review. > > Could you perhaps find someone who could review? > > I see Chen wrote some patches and tested, so perhaps they could? Hi Maarten, Huacai Chen did provide his Reviewed-by (although as he noted, the content initially came from him anyway and my contribution was mainly rearranging things into separate patches for each specific case). Anything else we can to do help here? Bjorn
Re: [PATCH 1/9] dt-bindings: Add arm,mali-valhall compatible
On 11/02/2022 20:27, alyssa.rosenzw...@collabora.com wrote: > From: Alyssa Rosenzweig > > From the kernel's perspective, pre-CSF Valhall is more or less > compatible with Bifrost, although they differ to userspace. Add a > compatible for Valhall to the existing Bifrost bindings documentation. > > Signed-off-by: Alyssa Rosenzweig > Cc: devicet...@vger.kernel.org > --- > Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > index 63a08f3f321d..48aeabd2ed68 100644 > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > @@ -23,6 +23,7 @@ properties: >- rockchip,px30-mali >- rockchip,rk3568-mali >- const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully > discoverable > + - const: arm,mali-valhall # Mali Valhall GPU model/revision is fully > discoverable It might be worth spelling out here that this is *pre-CSF* Valhall. I'm pretty sure we're going to need different bindings for CSF GPUs. Steve > >reg: > maxItems: 1
Re: [PATCH 2/9] drm/panfrost: Handle HW_ISSUE_TTRX_2968_TTRX_3162
On 11/02/2022 20:27, alyssa.rosenzw...@collabora.com wrote: > From: Alyssa Rosenzweig > > Add handling for the HW_ISSUE_TTRX_2968_TTRX_3162 quirk. Logic ported > from kbase. kbase lists this workaround as used on Mali-G57. > > Signed-off-by: Alyssa Rosenzweig Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_gpu.c| 3 +++ > drivers/gpu/drm/panfrost/panfrost_issues.h | 3 +++ > drivers/gpu/drm/panfrost/panfrost_regs.h | 1 + > 3 files changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c > b/drivers/gpu/drm/panfrost/panfrost_gpu.c > index 50c8922694d7..1c1e2017aa80 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c > @@ -108,6 +108,9 @@ static void panfrost_gpu_init_quirks(struct > panfrost_device *pfdev) > quirks |= SC_LS_ALLOW_ATTR_TYPES; > } > > + if (panfrost_has_hw_issue(pfdev, HW_ISSUE_TTRX_2968_TTRX_3162)) > + quirks |= SC_VAR_ALGORITHM; > + > if (panfrost_has_hw_feature(pfdev, HW_FEATURE_TLS_HASHING)) > quirks |= SC_TLS_HASH_ENABLE; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h > b/drivers/gpu/drm/panfrost/panfrost_issues.h > index 8e59d765bf19..3af7d723377e 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_issues.h > +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h > @@ -125,6 +125,9 @@ enum panfrost_hw_issue { >* kernel must fiddle with L2 caches to prevent data leakage */ > HW_ISSUE_TGOX_R1_1234, > > + /* Must set SC_VAR_ALGORITHM */ > + HW_ISSUE_TTRX_2968_TTRX_3162, > + > HW_ISSUE_END > }; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h > b/drivers/gpu/drm/panfrost/panfrost_regs.h > index 16e776cc82ea..fa1e1af56e17 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_regs.h > +++ b/drivers/gpu/drm/panfrost/panfrost_regs.h > @@ -195,6 +195,7 @@ > #define SC_TLS_HASH_ENABLE BIT(17) > #define SC_LS_ATTR_CHECK_DISABLE BIT(18) > #define SC_ENABLE_TEXGRD_FLAGS BIT(25) > +#define SC_VAR_ALGORITHM BIT(29) > /* End SHADER_CONFIG register */ > > /* TILER_CONFIG register */
Re: [PATCH 3/9] drm/panfrost: Constify argument to has_hw_issue
On 11/02/2022 20:27, alyssa.rosenzw...@collabora.com wrote: > From: Alyssa Rosenzweig > > Logically, this function is free of side effects, so any pointers it > takes should be const. Needed to avoid a warning in the next patch. > > Signed-off-by: Alyssa Rosenzweig Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_issues.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h > b/drivers/gpu/drm/panfrost/panfrost_issues.h > index 3af7d723377e..a66692663833 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_issues.h > +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h > @@ -251,7 +251,7 @@ enum panfrost_hw_issue { > > #define hw_issues_g76 0 > > -static inline bool panfrost_has_hw_issue(struct panfrost_device *pfdev, > +static inline bool panfrost_has_hw_issue(const struct panfrost_device *pfdev, >enum panfrost_hw_issue issue) > { > return test_bit(issue, pfdev->features.hw_issues);
Re: [PATCH 4/9] drm/panfrost: Handle HW_ISSUE_TTRX_3076
On 11/02/2022 20:27, alyssa.rosenzw...@collabora.com wrote: > From: Alyssa Rosenzweig > > Some Valhall GPUs require resets when encountering bus faults due to > occlusion query writes. Add the issue bit for this and handle it. > > Signed-off-by: Alyssa Rosenzweig Reviewed-by: Steven Price (although one nit below) Just in case any one is wondering - these bus faults occur when switching the GPU's MMU to unmapped - it's not a normal "bus fault" from the external bus. This is triggered by an attempt to read unmapped memory which is completed by the driver by switching the entire MMU to unmapped. > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 9 +++-- > drivers/gpu/drm/panfrost/panfrost_issues.h | 4 > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index 7f51a4682ccb..ee612303f076 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -11,6 +11,7 @@ > #include "panfrost_device.h" > #include "panfrost_devfreq.h" > #include "panfrost_features.h" > +#include "panfrost_issues.h" > #include "panfrost_gpu.h" > #include "panfrost_job.h" > #include "panfrost_mmu.h" > @@ -380,9 +381,13 @@ const char *panfrost_exception_name(u32 exception_code) > bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev, > u32 exception_code) > { > - /* Right now, none of the GPU we support need a reset, but this > - * might change. > + /* If an occlusion query write causes a bus fault on affected GPUs, > + * future fragment jobs may hang. Reset to workaround. >*/ > + if (exception_code == DRM_PANFROST_EXCEPTION_JOB_BUS_FAULT) > + return panfrost_has_hw_issue(pfdev, HW_ISSUE_TTRX_3076); > + > + /* No other GPUs we support need a reset */ > return false; > } > > diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h > b/drivers/gpu/drm/panfrost/panfrost_issues.h > index a66692663833..058f6a4c8435 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_issues.h > +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h > @@ -128,6 +128,10 @@ enum panfrost_hw_issue { > /* Must set SC_VAR_ALGORITHM */ > HW_ISSUE_TTRX_2968_TTRX_3162, > > + /* Bus fault from occlusion query write may cause future fragment jobs > + * to hang */ NIT: Kernel comment style has the "/*" and "*/" on lines by themselves for multi-line comments. checkpatch will complain! > + HW_ISSUE_TTRX_3076, > + > HW_ISSUE_END > }; >