Re: [PATCH] drm/display: fix typo
On 1/19/24 02:22, Oleksandr Natalenko wrote: > While studying the code I've bumped into a small typo within the > kernel-doc for two functions, apparently, due to copy-paste. > > This commit fixes "sizo" word to be "size". > > Signed-off-by: Oleksandr Natalenko Acked-by: Randy Dunlap Thanks. > --- > drivers/gpu/drm/display/drm_dp_dual_mode_helper.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c > b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c > index bd61e20770a5b..14a2a8473682b 100644 > --- a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c > @@ -52,7 +52,7 @@ > * @adapter: I2C adapter for the DDC bus > * @offset: register offset > * @buffer: buffer for return data > - * @size: sizo of the buffer > + * @size: size of the buffer > * > * Reads @size bytes from the DP dual mode adaptor registers > * starting at @offset. > @@ -116,7 +116,7 @@ EXPORT_SYMBOL(drm_dp_dual_mode_read); > * @adapter: I2C adapter for the DDC bus > * @offset: register offset > * @buffer: buffer for write data > - * @size: sizo of the buffer > + * @size: size of the buffer > * > * Writes @size bytes to the DP dual mode adaptor registers > * starting at @offset. -- #Randy
Re: [PATCH 13/14] drm/msm/dp: move next_bridge handling to dp_display
On Fri, 19 Jan 2024 at 23:14, Kuogee Hsieh wrote: > > Dmitry, > > I am testing this patch serial with msm-next branch. > > This patch cause system crash during booting up for me. > > Is this patch work for you? Yes, tested on top of linux-next. However I only tested it with DP-over-USBC. What is your testcase? Could you please share the crash log? > On 12/29/2023 2:56 PM, Dmitry Baryshkov wrote: > > Remove two levels of indirection and fetch next bridge directly in > > dp_display_probe_tail(). > > > > Signed-off-by: Dmitry Baryshkov > > --- > > drivers/gpu/drm/msm/dp/dp_display.c | 42 + > > drivers/gpu/drm/msm/dp/dp_parser.c | 14 -- > > drivers/gpu/drm/msm/dp/dp_parser.h | 14 -- > > 3 files changed, 13 insertions(+), 57 deletions(-) -- With best wishes Dmitry
Re: [PATCH v4 1/4] usb: gadget: Support already-mapped DMA SGs
On Sat, Jan 20, 2024 at 01:14:52AM +0100, Paul Cercueil wrote: > Hi Frank, > > Le vendredi 19 janvier 2024 à 16:49 -0500, Frank Li a écrit : > > On Wed, Jan 17, 2024 at 01:26:43PM +0100, Paul Cercueil wrote: > > > Add a new 'sg_was_mapped' field to the struct usb_request. This > > > field > > > can be used to indicate that the scatterlist associated to the USB > > > transfer has already been mapped into the DMA space, and it does > > > not > > > have to be done internally. > > > > > > Signed-off-by: Paul Cercueil > > > --- > > > drivers/usb/gadget/udc/core.c | 7 ++- > > > include/linux/usb/gadget.h | 2 ++ > > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/gadget/udc/core.c > > > b/drivers/usb/gadget/udc/core.c > > > index d59f94464b87..9d4150124fdb 100644 > > > --- a/drivers/usb/gadget/udc/core.c > > > +++ b/drivers/usb/gadget/udc/core.c > > > @@ -903,6 +903,11 @@ int usb_gadget_map_request_by_dev(struct > > > device *dev, > > > if (req->length == 0) > > > return 0; > > > > > > + if (req->sg_was_mapped) { > > > + req->num_mapped_sgs = req->num_sgs; > > > + return 0; > > > + } > > > + > > > if (req->num_sgs) { > > > int mapped; > > > > > > @@ -948,7 +953,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_map_request); > > > void usb_gadget_unmap_request_by_dev(struct device *dev, > > > struct usb_request *req, int is_in) > > > { > > > - if (req->length == 0) > > > + if (req->length == 0 || req->sg_was_mapped) > > > return; > > > > > > if (req->num_mapped_sgs) { > > > diff --git a/include/linux/usb/gadget.h > > > b/include/linux/usb/gadget.h > > > index a771ccc038ac..c529e4e06997 100644 > > > --- a/include/linux/usb/gadget.h > > > +++ b/include/linux/usb/gadget.h > > > @@ -52,6 +52,7 @@ struct usb_ep; > > > * @short_not_ok: When reading data, makes short packets be > > > * treated as errors (queue stops advancing till cleanup). > > > * @dma_mapped: Indicates if request has been mapped to DMA > > > (internal) > > > + * @sg_was_mapped: Set if the scatterlist has been mapped before > > > the request > > > * @complete: Function called when request completes, so this > > > request and > > > * its buffer may be re-used. The function will always be > > > called with > > > * interrupts disabled, and it must not sleep. > > > @@ -111,6 +112,7 @@ struct usb_request { > > > unsignedzero:1; > > > unsignedshort_not_ok:1; > > > unsigneddma_mapped:1; > > > + unsignedsg_was_mapped:1; > > > > why not use dma_mapped direclty? > > Because of the unmap case. We want to know whether we should unmap or > not. I see, Thanks Frank > > > > > Frank > > Cheers, > -Paul > > > > > > > > > void(*complete)(struct usb_ep *ep, > > > struct usb_request *req); > > > -- > > > 2.43.0 > > > >
Re: REGRESSION: no console on current -git
On 1/19/24 5:27 PM, Helge Deller wrote: > On 1/19/24 22:22, Jens Axboe wrote: >> On 1/19/24 2:14 PM, Helge Deller wrote: >>> On 1/19/24 22:01, Jens Axboe wrote: On 1/19/24 1:55 PM, Helge Deller wrote: > Adding Mirsad Todorovac (who reported a similar issue). > > On 1/19/24 19:39, Jens Axboe wrote: >> My trusty R7525 test box is failing to show a console, or in fact >> anything, >> on current -git. There's no output after: >> >> Loading Linux 6.7.0+ ... >> Loading initial ramdisk ... >> >> and I don't get a console up. I went through the bisection pain and >> found this was the culprit: >> >> commit df67699c9cb0ceb70f6cc60630ca938c06773eda >> Author: Thomas Zimmermann >> Date: Wed Jan 3 11:15:11 2024 +0100 >> >>firmware/sysfb: Clear screen_info state after consuming it >> >> Reverting this commit, and everything is fine. Looking at dmesg with a >> buggy kernel, I get no frame or fb messages. On a good kernel, it looks >> ilke this: >> >> [1.416486] efifb: probing for efifb >> [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k >> [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 >> [1.416607] efifb: scrolling: redraw >> [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 >> [1.449746] fb0: EFI VGA frame buffer device >> >> Happy to test a fix, or barring that, can someone just revert this >> commit please? > > I've temporarily added a revert patch into the fbdev for-next tree for > now, > so people should not face the issue in the for-next series: > https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next > I'd like to wait for Thomas to return on monday to check the issue > as there are some other upcoming patches in this area from him. Given the issue (and that I'm not the only one reporting it), can we please just get that pushed so it'll make -rc1? It can always get re-introduced in a fixed fashion. I don't run -next here, I rely on mainline working for my testing. >>> >>> I agree, it would be good to get it fixed for -rc1. >>> So, it's ok for me, but I won't be able to test the revert short time right >>> now. >>> If you can assure that the revert fixes it, and builds in git-head, >>> I can now prepare the pull request for Linus now (or he just reverts >>> commit df67699c9cb0 manually). >> >> I already tested a revert on top of the current tree, and it builds just >> fine and boots with a working console. So reverting it does work and >> solves the issue. > > I sent a pull request with the revert. Thanks! You forgot the Reported-by, but no big deal. -- Jens Axboe
Re: REGRESSION: no console on current -git
On 1/19/24 22:22, Jens Axboe wrote: On 1/19/24 2:14 PM, Helge Deller wrote: On 1/19/24 22:01, Jens Axboe wrote: On 1/19/24 1:55 PM, Helge Deller wrote: Adding Mirsad Todorovac (who reported a similar issue). On 1/19/24 19:39, Jens Axboe wrote: My trusty R7525 test box is failing to show a console, or in fact anything, on current -git. There's no output after: Loading Linux 6.7.0+ ... Loading initial ramdisk ... and I don't get a console up. I went through the bisection pain and found this was the culprit: commit df67699c9cb0ceb70f6cc60630ca938c06773eda Author: Thomas Zimmermann Date: Wed Jan 3 11:15:11 2024 +0100 firmware/sysfb: Clear screen_info state after consuming it Reverting this commit, and everything is fine. Looking at dmesg with a buggy kernel, I get no frame or fb messages. On a good kernel, it looks ilke this: [1.416486] efifb: probing for efifb [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 [1.416607] efifb: scrolling: redraw [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 [1.449746] fb0: EFI VGA frame buffer device Happy to test a fix, or barring that, can someone just revert this commit please? I've temporarily added a revert patch into the fbdev for-next tree for now, so people should not face the issue in the for-next series: https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next I'd like to wait for Thomas to return on monday to check the issue as there are some other upcoming patches in this area from him. Given the issue (and that I'm not the only one reporting it), can we please just get that pushed so it'll make -rc1? It can always get re-introduced in a fixed fashion. I don't run -next here, I rely on mainline working for my testing. I agree, it would be good to get it fixed for -rc1. So, it's ok for me, but I won't be able to test the revert short time right now. If you can assure that the revert fixes it, and builds in git-head, I can now prepare the pull request for Linus now (or he just reverts commit df67699c9cb0 manually). I already tested a revert on top of the current tree, and it builds just fine and boots with a working console. So reverting it does work and solves the issue. I sent a pull request with the revert. Thanks! Helge
[GIT PULL] One fbdev regression fix for v6.8-rc1
Hi Linus, there were various reports from people without any graphics output on the screen and it turns out one commit triggers the problem. Please pull a single revert for now for -rc1 to fix this regression. We will work out how to fix it correctly later. Thanks! Helge The following changes since commit 556e2d17cae620d549c5474b1ece053430cd50bc: Merge tag 'ceph-for-6.8-rc1' of https://github.com/ceph/ceph-client (2024-01-19 09:58:55 -0800) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.8-rc1-2 for you to fetch changes up to 2bebc3cd48701607e38e8258ab9692de9b1a718b: Revert "firmware/sysfb: Clear screen_info state after consuming it" (2024-01-19 22:22:26 +0100) fbdev fix for 6.8-rc1: - Revert "firmware/sysfb: Clear screen_info state after consuming it" Helge Deller (1): Revert "firmware/sysfb: Clear screen_info state after consuming it" drivers/firmware/sysfb.c | 14 +- 1 file changed, 1 insertion(+), 13 deletions(-)
Re: [PATCH v4 1/4] usb: gadget: Support already-mapped DMA SGs
Hi Frank, Le vendredi 19 janvier 2024 à 16:49 -0500, Frank Li a écrit : > On Wed, Jan 17, 2024 at 01:26:43PM +0100, Paul Cercueil wrote: > > Add a new 'sg_was_mapped' field to the struct usb_request. This > > field > > can be used to indicate that the scatterlist associated to the USB > > transfer has already been mapped into the DMA space, and it does > > not > > have to be done internally. > > > > Signed-off-by: Paul Cercueil > > --- > > drivers/usb/gadget/udc/core.c | 7 ++- > > include/linux/usb/gadget.h | 2 ++ > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/gadget/udc/core.c > > b/drivers/usb/gadget/udc/core.c > > index d59f94464b87..9d4150124fdb 100644 > > --- a/drivers/usb/gadget/udc/core.c > > +++ b/drivers/usb/gadget/udc/core.c > > @@ -903,6 +903,11 @@ int usb_gadget_map_request_by_dev(struct > > device *dev, > > if (req->length == 0) > > return 0; > > > > + if (req->sg_was_mapped) { > > + req->num_mapped_sgs = req->num_sgs; > > + return 0; > > + } > > + > > if (req->num_sgs) { > > int mapped; > > > > @@ -948,7 +953,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_map_request); > > void usb_gadget_unmap_request_by_dev(struct device *dev, > > struct usb_request *req, int is_in) > > { > > - if (req->length == 0) > > + if (req->length == 0 || req->sg_was_mapped) > > return; > > > > if (req->num_mapped_sgs) { > > diff --git a/include/linux/usb/gadget.h > > b/include/linux/usb/gadget.h > > index a771ccc038ac..c529e4e06997 100644 > > --- a/include/linux/usb/gadget.h > > +++ b/include/linux/usb/gadget.h > > @@ -52,6 +52,7 @@ struct usb_ep; > > * @short_not_ok: When reading data, makes short packets be > > * treated as errors (queue stops advancing till cleanup). > > * @dma_mapped: Indicates if request has been mapped to DMA > > (internal) > > + * @sg_was_mapped: Set if the scatterlist has been mapped before > > the request > > * @complete: Function called when request completes, so this > > request and > > * its buffer may be re-used. The function will always be > > called with > > * interrupts disabled, and it must not sleep. > > @@ -111,6 +112,7 @@ struct usb_request { > > unsignedzero:1; > > unsignedshort_not_ok:1; > > unsigneddma_mapped:1; > > + unsignedsg_was_mapped:1; > > why not use dma_mapped direclty? Because of the unmap case. We want to know whether we should unmap or not. > > Frank Cheers, -Paul > > > > > void(*complete)(struct usb_ep *ep, > > struct usb_request *req); > > -- > > 2.43.0 > >
Re: [PATCH 01/10] pci: add new set of devres functions
On Wed, Jan 17, 2024 at 09:54:47AM +0100, Philipp Stanner wrote: > On Tue, 2024-01-16 at 12:44 -0600, Bjorn Helgaas wrote: > > On Mon, Jan 15, 2024 at 03:46:12PM +0100, Philipp Stanner wrote: > > > PCI's devres API is not extensible to ranged mappings and has > > > bug-provoking features. Improve that by providing better > > > alternatives. > > > > I guess "ranged mappings" means a mapping that doesn't cover an > > entire BAR? Maybe there's a way to clarify? > > That's what it's supposed to mean, yes. We could give it the longer > title "mappings smaller than the whole BAR" or something, I guess. "partial BAR mappings"? > > > to the creation of a set of "pural functions" such as s/pural/plural/ (I missed this before). > > > c) The iomap-table mechanism is over-engineered, > > > complicated and > > > can by definition not perform bounds checks, thus, > > > provoking > > > memory faults: pcim_iomap_table(pdev)[42] > > > > Not sure what "pcim_iomap_table(pdev)[42]" means. > > That function currently is implemented with this prototype: > void __iomem * const *pcim_iomap_table(struct pci_dev *pdev); > > And apparently, it's intended to index directly over the function. And > that's how at least part of the users use it indeed. > > Here in drivers/crypto/inside-secure/safexcel.c, L.1919 for example: > > priv->base = pcim_iomap_table(pdev)[0]; > > I've never seen something that wonderful in C ever before, so it's not > surprising that you weren't sure what I mean > > pcim_iomap_table() can not and does not perform any bounds check. If > you do > > void __iomem *mappy_map_mapface = pcim_iomap_table(pdev)[42]; > > then it will just return random garbage, or it faults. No -EINVAL or > anything. You won't even get NULL. > > That's why this function must die. No argument except that this example only makes sense after one looks up the prototype and connects the dots. Bjorn
Re: [PATCH v4 1/4] usb: gadget: Support already-mapped DMA SGs
On Wed, Jan 17, 2024 at 01:26:43PM +0100, Paul Cercueil wrote: > Add a new 'sg_was_mapped' field to the struct usb_request. This field > can be used to indicate that the scatterlist associated to the USB > transfer has already been mapped into the DMA space, and it does not > have to be done internally. > > Signed-off-by: Paul Cercueil > --- > drivers/usb/gadget/udc/core.c | 7 ++- > include/linux/usb/gadget.h| 2 ++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index d59f94464b87..9d4150124fdb 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -903,6 +903,11 @@ int usb_gadget_map_request_by_dev(struct device *dev, > if (req->length == 0) > return 0; > > + if (req->sg_was_mapped) { > + req->num_mapped_sgs = req->num_sgs; > + return 0; > + } > + > if (req->num_sgs) { > int mapped; > > @@ -948,7 +953,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_map_request); > void usb_gadget_unmap_request_by_dev(struct device *dev, > struct usb_request *req, int is_in) > { > - if (req->length == 0) > + if (req->length == 0 || req->sg_was_mapped) > return; > > if (req->num_mapped_sgs) { > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index a771ccc038ac..c529e4e06997 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -52,6 +52,7 @@ struct usb_ep; > * @short_not_ok: When reading data, makes short packets be > * treated as errors (queue stops advancing till cleanup). > * @dma_mapped: Indicates if request has been mapped to DMA (internal) > + * @sg_was_mapped: Set if the scatterlist has been mapped before the request > * @complete: Function called when request completes, so this request and > * its buffer may be re-used. The function will always be called with > * interrupts disabled, and it must not sleep. > @@ -111,6 +112,7 @@ struct usb_request { > unsignedzero:1; > unsignedshort_not_ok:1; > unsigneddma_mapped:1; > + unsignedsg_was_mapped:1; why not use dma_mapped direclty? Frank > > void(*complete)(struct usb_ep *ep, > struct usb_request *req); > -- > 2.43.0 >
Re: Implement per-key keyboard backlight as auxdisplay?
Hi! > > And while I would personally hate it, you can imagine a use case where > > you'd like a keypress to have a visual effect around the key you > > pressed. A kind of force feedback, if you will. I don't actually know, > > and correct me if I'm wrong, but feels like implementing that outside of > > the input subsystem would be non-trivial. > > Actually I think it does not belong to the input subsystem as it is, > where the goal is to deliver keystrokes and gestures to userspace. The > "force feedback" kind of fits, but not really practical, again because > of lack of geometry info. It is also not really essential to be fully > and automatically handled by the kernel. So I think the best way is > > to So that's actually big question. If the common usage is "run bad apple demo on keyboard" than pretty clearly it should be display. If the common usage is "computer is asking yes/no question, so highlight yes and no buttons", then there are good arguments why input should handle that (as it does capslock led, for example). Actually I could imagine "real" use when shift / control /alt backlight would indicate sticky-shift keys for handicapped. It seems they are making mice with backlit buttons. If the main use is highlight this key whereever it is, then it should be input. But I suspect may use is just fancy colors and it should be display. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH 1/2] drm/atomic: Allow drivers to write their own plane check for async flips
Hi André, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-tip/drm-tip linus/master next-20240119] [cannot apply to drm-intel/for-linux-next-fixes v6.7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Andr-Almeida/drm-atomic-Allow-drivers-to-write-their-own-plane-check-for-async-flips/20240116-125406 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20240116045159.1015510-2-andrealmeid%40igalia.com patch subject: [PATCH 1/2] drm/atomic: Allow drivers to write their own plane check for async flips config: hexagon-randconfig-002-20240119 (https://download.01.org/0day-ci/archive/20240120/202401200526.xggix2de-...@intel.com/config) compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project d92ce344bf641e6bb025b41b3f1a77dd25e2b3e9) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240120/202401200526.xggix2de-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202401200526.xggix2de-...@intel.com/ All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/drm_atomic_uapi.c:31: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:32: In file included from include/drm/drm_modes.h:33: In file included from include/drm/drm_connector.h:32: In file included from include/drm/drm_util.h:35: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:337: include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 547 | val = __raw_readb(PCI_IOBASE + addr); | ~~ ^ include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); | ~~ ^ include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu' 37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x)) | ^ In file included from drivers/gpu/drm/drm_atomic_uapi.c:31: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:32: In file included from include/drm/drm_modes.h:33: In file included from include/drm/drm_connector.h:32: In file included from include/drm/drm_util.h:35: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:337: include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); | ~~ ^ include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu' 35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) | ^ In file included from drivers/gpu/drm/drm_atomic_uapi.c:31: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:32: In file included from include/drm/drm_modes.h:33: In file included from include/drm/drm_connector.h:32: In file included from include/drm/drm_util.h:35: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file in
RE: [PATCH 3/4] drm: xlnx: zynqmp_dpsub: Don't generate vblank in live mode
Hi Laurent, > -Original Message- > From: Laurent Pinchart > Sent: Friday, January 19, 2024 4:30 AM > To: Klymenko, Anatoliy > Cc: Tomi Valkeinen ; > aarten.lankho...@linux.intel.com; mrip...@kernel.org; tzimmerm...@suse.de; > airl...@gmail.com; dan...@ffwll.ch; Simek, Michal ; > dri-devel@lists.freedesktop.org; linux-arm-ker...@lists.infradead.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH 3/4] drm: xlnx: zynqmp_dpsub: Don't generate vblank in > live > mode > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > Hi Anatoliy, > > On Fri, Jan 19, 2024 at 05:53:30AM +, Klymenko, Anatoliy wrote: > > On Wed, 17 Jan 2024 16:20:10 +0200, Tomi Valkeinen wrote: > > > On 13/01/2024 01:42, Anatoliy Klymenko wrote: > > > > Filter out status register against interrupts' mask. > > > > Some events are being reported via DP status register, even if > > > > corresponding interrupts have been disabled. Avoid processing of > > > > such events in interrupt handler context. > > > > > > The subject talks about vblank and live mode, the the description > > > doesn't. Can you elaborate in the desc a bit about when this is an > > > issue and why it wasn't before? > > > > Yes, I should make patch comment more consistent and elaborate. I will fix > > it in > the next version. Thank you. > > > > > > > > > Signed-off-by: Anatoliy Klymenko > > > > --- > > > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 11 +-- > > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > > b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > > index d60b7431603f..571c5dbc97e5 100644 > > > > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > > @@ -1624,8 +1624,16 @@ static irqreturn_t zynqmp_dp_irq_handler(int > irq, void *data) > > > > u32 status, mask; > > > > > > > > status = zynqmp_dp_read(dp, ZYNQMP_DP_INT_STATUS); > > > > + zynqmp_dp_write(dp, ZYNQMP_DP_INT_STATUS, status); > > > > mask = zynqmp_dp_read(dp, ZYNQMP_DP_INT_MASK); > > > > - if (!(status & ~mask)) > > > > + > > > > + /* > > > > + * Status register may report some events, which corresponding > interrupts > > > > + * have been disabled. Filter out those events against > > > > interrupts' mask. > > > > + */ > > > > + status &= ~mask; > > > > + > > > > + if (!status) > > > > return IRQ_NONE; > > > > > > > > /* dbg for diagnostic, but not much that the driver can do > > > > */ @@ -1634,7 +1642,6 @@ static irqreturn_t zynqmp_dp_irq_handler(int > irq, void *data) > > > > if (status & ZYNQMP_DP_INT_CHBUF_OVERFLW_MASK) > > > > dev_dbg_ratelimited(dp->dev, "overflow > > > > interrupt\n"); > > > > > > > > - zynqmp_dp_write(dp, ZYNQMP_DP_INT_STATUS, status); > > > > > > > > if (status & ZYNQMP_DP_INT_VBLANK_START) > > > > zynqmp_dpsub_drm_handle_vblank(dp->dpsub); > > > > > > Moving the zynqmp_dp_write() is not related to this fix, is it? I > > > think it should be in a separate patch. > > > > The sole purpose of zynqmp_dp_write() is to clear status register. The > > sooner we do it the better (after reading status in the local variable > > of course). > > No disagreement about that. Tomi's point is that it's a good change, but it > should > be done in a separate patch, by itself, not bundled with other changes. The > usual > rule in the kernel is "one change, one commit". > Sure, I will move this into a separate commit in the next version. Thank you. > > We can also reuse status variable for in-place filtering against the > > interrupt mask, which makes this change dependent on > > zynqmp_dp_write() reordering. I will add a comment explaining this. Is > > it ok? > > -- > Regards, > > Laurent Pinchart Thank you, Anatoliy
Re: [PATCH v3 0/3] Convert Microchip's HLCDC Text based DT bindings to JSON schema
Hi Dharma, On Fri, Jan 19, 2024 at 08:41:04AM +, dharm...@microchip.com wrote: > Hi Sam, > On 19/01/24 1:00 am, Sam Ravnborg wrote: > > [You don't often get email from s...@ravnborg.org. Learn why this is > > important at https://aka.ms/LearnAboutSenderIdentification ] > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > Hi Dharma et al. > > > > On Thu, Jan 18, 2024 at 02:56:09PM +0530, Dharma Balasubiramani wrote: > >> Converted the text bindings to YAML and validated them individually using > >> following commands > >> > >> $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ > >> $ make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ > >> > >> changelogs are available in respective patches. > >> > >> Dharma Balasubiramani (3): > >>dt-bindings: display: convert Atmel's HLCDC to DT schema > >>dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema > >>dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format > > > > I know this is a bit late to ask - sorry in advance. > > > > The binding describes the single IP block as a multi functional device, > > but it is a single IP block that includes the display controller and a > > simple pwm that can be used for contrast or backlight. > yes. > > > > If we ignore the fact that the current drivers for hlcdc uses an mfd > > abstraction, is this then the optimal way to describe the HW? > > > > > > In one of my stale git tree I converted atmel lcdc to DT, and here > Are you referring the "bindings/display/atmel,lcdc.txt"? Correct. > > I used: > > > > + "#pwm-cells": > > +description: > > + This PWM chip use the default 3 cells bindings > > + defined in ../../pwm/pwm.yaml. > > +const: 3 > > + > > + clocks: > > +maxItems: 2 > > + > > + clock-names: > > +maxItems: 2 > > +items: > > + - const: lcdc_clk > > + - const: hclk > > > > This proved to be a simple way to describe the HW. > > > > To make the DT binding backward compatible you likely need to add a few > > compatible that otherwise would have been left out - but that should do > > the trick. > again you mean the compatibles from atmel,lcdc binding? If the new binding describes the full IP, as I suggest, then I assume you need to add the compatible "atmel,hlcdc-pwm" to be backward compatible. Otherwise users assuming the old binding will fail to find the pwm info. I am not sure how important this is - but at least then the device trees can be updated out of sync with the current users. I hope this explains what I tried to say, otherwise do not hesitate to get back to me. Sam
Re: [git pull] drm fixes for 6.8-rc1 (part two)
The pull request you sent on Fri, 19 Jan 2024 16:58:59 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-next-2024-01-19 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/e08b5758153981ca812c5991209a6133c732e799 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: REGRESSION: no console on current -git
On 1/19/24 2:14 PM, Helge Deller wrote: > On 1/19/24 22:01, Jens Axboe wrote: >> On 1/19/24 1:55 PM, Helge Deller wrote: >>> Adding Mirsad Todorovac (who reported a similar issue). >>> >>> On 1/19/24 19:39, Jens Axboe wrote: My trusty R7525 test box is failing to show a console, or in fact anything, on current -git. There's no output after: Loading Linux 6.7.0+ ... Loading initial ramdisk ... and I don't get a console up. I went through the bisection pain and found this was the culprit: commit df67699c9cb0ceb70f6cc60630ca938c06773eda Author: Thomas Zimmermann Date: Wed Jan 3 11:15:11 2024 +0100 firmware/sysfb: Clear screen_info state after consuming it Reverting this commit, and everything is fine. Looking at dmesg with a buggy kernel, I get no frame or fb messages. On a good kernel, it looks ilke this: [1.416486] efifb: probing for efifb [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 [1.416607] efifb: scrolling: redraw [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 [1.449746] fb0: EFI VGA frame buffer device Happy to test a fix, or barring that, can someone just revert this commit please? >>> >>> I've temporarily added a revert patch into the fbdev for-next tree for now, >>> so people should not face the issue in the for-next series: >>> https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next >>> I'd like to wait for Thomas to return on monday to check the issue >>> as there are some other upcoming patches in this area from him. >> >> Given the issue (and that I'm not the only one reporting it), can we >> please just get that pushed so it'll make -rc1? It can always get >> re-introduced in a fixed fashion. I don't run -next here, I rely on >> mainline working for my testing. > > I agree, it would be good to get it fixed for -rc1. > So, it's ok for me, but I won't be able to test the revert short time right > now. > If you can assure that the revert fixes it, and builds in git-head, > I can now prepare the pull request for Linus now (or he just reverts > commit df67699c9cb0 manually). I already tested a revert on top of the current tree, and it builds just fine and boots with a working console. So reverting it does work and solves the issue. -- Jens Axboe
Re: REGRESSION: no console on current -git
On 1/19/24 22:01, Jens Axboe wrote: On 1/19/24 1:55 PM, Helge Deller wrote: Adding Mirsad Todorovac (who reported a similar issue). On 1/19/24 19:39, Jens Axboe wrote: My trusty R7525 test box is failing to show a console, or in fact anything, on current -git. There's no output after: Loading Linux 6.7.0+ ... Loading initial ramdisk ... and I don't get a console up. I went through the bisection pain and found this was the culprit: commit df67699c9cb0ceb70f6cc60630ca938c06773eda Author: Thomas Zimmermann Date: Wed Jan 3 11:15:11 2024 +0100 firmware/sysfb: Clear screen_info state after consuming it Reverting this commit, and everything is fine. Looking at dmesg with a buggy kernel, I get no frame or fb messages. On a good kernel, it looks ilke this: [1.416486] efifb: probing for efifb [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 [1.416607] efifb: scrolling: redraw [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 [1.449746] fb0: EFI VGA frame buffer device Happy to test a fix, or barring that, can someone just revert this commit please? I've temporarily added a revert patch into the fbdev for-next tree for now, so people should not face the issue in the for-next series: https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next I'd like to wait for Thomas to return on monday to check the issue as there are some other upcoming patches in this area from him. Given the issue (and that I'm not the only one reporting it), can we please just get that pushed so it'll make -rc1? It can always get re-introduced in a fixed fashion. I don't run -next here, I rely on mainline working for my testing. I agree, it would be good to get it fixed for -rc1. So, it's ok for me, but I won't be able to test the revert short time right now. If you can assure that the revert fixes it, and builds in git-head, I can now prepare the pull request for Linus now (or he just reverts commit df67699c9cb0 manually). Helge
Re: [PATCH 13/14] drm/msm/dp: move next_bridge handling to dp_display
Dmitry, I am testing this patch serial with msm-next branch. This patch cause system crash during booting up for me. Is this patch work for you? On 12/29/2023 2:56 PM, Dmitry Baryshkov wrote: Remove two levels of indirection and fetch next bridge directly in dp_display_probe_tail(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dp/dp_display.c | 42 + drivers/gpu/drm/msm/dp/dp_parser.c | 14 -- drivers/gpu/drm/msm/dp/dp_parser.h | 14 -- 3 files changed, 13 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 4de0857c31ce..923df47efcc9 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1195,16 +1195,24 @@ static const struct msm_dp_desc *dp_display_get_desc(struct platform_device *pde return NULL; } -static int dp_display_get_next_bridge(struct msm_dp *dp); - static int dp_display_probe_tail(struct device *dev) { struct msm_dp *dp = dev_get_drvdata(dev); int ret; - ret = dp_display_get_next_bridge(dp); - if (ret) - return ret; + /* +* External bridges are mandatory for eDP interfaces: one has to +* provide at least an eDP panel (which gets wrapped into panel-bridge). +* +* For DisplayPort interfaces external bridges are optional, so +* silently ignore an error if one is not present (-ENODEV). +*/ + dp->next_bridge = devm_drm_of_get_bridge(>pdev->dev, dp->pdev->dev.of_node, 1, 0); + if (IS_ERR(dp->next_bridge)) { + ret = PTR_ERR(dp->next_bridge); + if (dp->is_edp || ret != -ENODEV) + return ret; + } ret = component_add(dev, _display_comp_ops); if (ret) @@ -1397,30 +1405,6 @@ void dp_display_debugfs_init(struct msm_dp *dp_display, struct dentry *root, boo } } -static int dp_display_get_next_bridge(struct msm_dp *dp) -{ - int rc; - struct dp_display_private *dp_priv; - - dp_priv = container_of(dp, struct dp_display_private, dp_display); - - /* -* External bridges are mandatory for eDP interfaces: one has to -* provide at least an eDP panel (which gets wrapped into panel-bridge). -* -* For DisplayPort interfaces external bridges are optional, so -* silently ignore an error if one is not present (-ENODEV). -*/ - rc = devm_dp_parser_find_next_bridge(>pdev->dev, dp_priv->parser); - if (!dp->is_edp && rc == -ENODEV) - return 0; - - if (!rc) - dp->next_bridge = dp_priv->parser->next_bridge; - - return rc; -} - int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, struct drm_encoder *encoder) { diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c index aa135d5cedbd..f95ab3c5c72c 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.c +++ b/drivers/gpu/drm/msm/dp/dp_parser.c @@ -24,20 +24,6 @@ static int dp_parser_ctrl_res(struct dp_parser *parser) return 0; } -int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser) -{ - struct platform_device *pdev = parser->pdev; - struct drm_bridge *bridge; - - bridge = devm_drm_of_get_bridge(dev, pdev->dev.of_node, 1, 0); - if (IS_ERR(bridge)) - return PTR_ERR(bridge); - - parser->next_bridge = bridge; - - return 0; -} - static int dp_parser_parse(struct dp_parser *parser) { int rc = 0; diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h index bc56e0e8c446..2b39b1c394ae 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.h +++ b/drivers/gpu/drm/msm/dp/dp_parser.h @@ -22,7 +22,6 @@ struct dp_parser { struct platform_device *pdev; struct phy *phy; - struct drm_bridge *next_bridge; }; /** @@ -38,17 +37,4 @@ struct dp_parser { */ struct dp_parser *dp_parser_get(struct platform_device *pdev); -/** - * devm_dp_parser_find_next_bridge() - find an additional bridge to DP - * - * @dev: device to tie bridge lifetime to - * @parser: dp_parser data from client - * - * This function is used to find any additional bridge attached to - * the DP controller. The eDP interface requires a panel bridge. - * - * Return: 0 if able to get the bridge, otherwise negative errno for failure. - */ -int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser); - #endif
Re: REGRESSION: no console on current -git
On 1/19/24 1:55 PM, Helge Deller wrote: > Adding Mirsad Todorovac (who reported a similar issue). > > On 1/19/24 19:39, Jens Axboe wrote: >> My trusty R7525 test box is failing to show a console, or in fact anything, >> on current -git. There's no output after: >> >> Loading Linux 6.7.0+ ... >> Loading initial ramdisk ... >> >> and I don't get a console up. I went through the bisection pain and >> found this was the culprit: >> >> commit df67699c9cb0ceb70f6cc60630ca938c06773eda >> Author: Thomas Zimmermann >> Date: Wed Jan 3 11:15:11 2024 +0100 >> >> firmware/sysfb: Clear screen_info state after consuming it >> >> Reverting this commit, and everything is fine. Looking at dmesg with a >> buggy kernel, I get no frame or fb messages. On a good kernel, it looks >> ilke this: >> >> [1.416486] efifb: probing for efifb >> [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k >> [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 >> [1.416607] efifb: scrolling: redraw >> [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 >> [1.449746] fb0: EFI VGA frame buffer device >> >> Happy to test a fix, or barring that, can someone just revert this >> commit please? > > I've temporarily added a revert patch into the fbdev for-next tree for now, > so people should not face the issue in the for-next series: > https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next > I'd like to wait for Thomas to return on monday to check the issue > as there are some other upcoming patches in this area from him. Given the issue (and that I'm not the only one reporting it), can we please just get that pushed so it'll make -rc1? It can always get re-introduced in a fixed fashion. I don't run -next here, I rely on mainline working for my testing. -- Jens Axboe
Re: REGRESSION: no console on current -git
Adding Mirsad Todorovac (who reported a similar issue). On 1/19/24 19:39, Jens Axboe wrote: My trusty R7525 test box is failing to show a console, or in fact anything, on current -git. There's no output after: Loading Linux 6.7.0+ ... Loading initial ramdisk ... and I don't get a console up. I went through the bisection pain and found this was the culprit: commit df67699c9cb0ceb70f6cc60630ca938c06773eda Author: Thomas Zimmermann Date: Wed Jan 3 11:15:11 2024 +0100 firmware/sysfb: Clear screen_info state after consuming it Reverting this commit, and everything is fine. Looking at dmesg with a buggy kernel, I get no frame or fb messages. On a good kernel, it looks ilke this: [1.416486] efifb: probing for efifb [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 [1.416607] efifb: scrolling: redraw [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 [1.449746] fb0: EFI VGA frame buffer device Happy to test a fix, or barring that, can someone just revert this commit please? I've temporarily added a revert patch into the fbdev for-next tree for now, so people should not face the issue in the for-next series: https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next I'd like to wait for Thomas to return on monday to check the issue as there are some other upcoming patches in this area from him. Helge
Re: Implement per-key keyboard backlight as auxdisplay?
Hi! > > > And then storing RGB in separate bytes, so userspace will then > > > always send a buffer of 192 bytes per line (64x3) x 14 rows > > > = 3072 bytes. With the kernel driver ignoring parts of > > > the buffer where there are no actual keys. > > That's really really weird interface. If you are doing RGB888 64x14, > > lets make it a ... display? :-). > > > > ioctl always sending 3072 bytes is really a hack. > > > > Small displays exist and are quite common, surely we'd handle this as > > a display: > > https://pajenicko.cz/displeje/graficky-oled-displej-0-66-64x48-i2c-bily-wemos-d1-mini > > It is 64x48. > > > > And then there's this: > > https://pajenicko.cz/displeje/maticovy-8x8-led-displej-s-radicem-max7219 > > and this: > > https://pajenicko.cz/displeje/maticovy-8x32-led-displej-s-radicem-max7219 > > > > One of them is 8x8. > > > > Surely those should be displays, too? > > But what about a light bar with, lets say, 3 zones. Is that a 3x1 display? > > And what about a mouse having lit mousebuttons and a single led light bar at > the wrist: a 2x2 display, but one is thin but long and one is not used? So indeed LEDs can arranged into various shapes. Like a ring, or this: * * * * * * * https://pajenicko.cz/led-moduly?page=2 Dunno. Sounds like a display is still a best match for them. Some of modules are RGB, some are single-color only, I'm sure there will be various bit depths. I guess we can do 3x1 and 2x2 displays. Or we could try to solve keyboards and ignore those for now. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Mesa >= 23.3.x and python 2.6 ...
Hi Jordan Thanks for digging into this! On Fri, Jan 19, 2024 at 12:10:37PM -0800, Jordan Justen wrote: > On 2024-01-18 04:37:52, Stefan Dirsch wrote: > > Hi > > > > I noticed that with version 23.3.x Mesa no longer can be built with python > > 2.6. It still worked with Mesa 23.2.1. > > As mentioned in other emails, this was typo where 3.6 was intended. > > > > > It fails with > > > > [ 95s] Traceback (most recent call last): > > [ 95s] File "../src/intel/genxml/gen_bits_header.py", line 23, in > > > > [ 95s] import intel_genxml > > [ 95s] File > > "/home/abuild/rpmbuild/BUILD/mesa-23.3.3/src/intel/genxml/intel_ > > genxml.py", line 5 > > [ 95s] from __future__ import annotations > > [ 95s] ^ > > [ 95s] SyntaxError: future feature annotations is not defined > > > > I guess this code first appeared in Dylan's: > > 4fd2e15855d ("intel/genxml: add type annotations to gen_sort_tags.py") > > and then became part of the standard tests a few commits later in: > > 1f0a0a46d97 ("meson: run genxml sort tests") > > back in Oct 2022. So, I guess at that point 'ninja test' would have > failed with Python 3.6. > > Then, I suppose I propagated this to being used on every build in: > > 0495f952d48 ("intel/genxml: Add genxml_import.py script") > > in Sept 2023. Thanks. This explains why I've found this code already in older releases, but it didn't fail for me yet. You said tests. Is this just a test, I could disable (as dirty hack)? I was assuming it would generate code ... > Maybe Dylan knows how we might make this compatible with Python 3.6, > assuming we want to. :) > > https://devguide.python.org/versions/ Yes, I know. It's EOL since a long time now ... sigh Thanks, Stefan Public Key available -- Stefan Dirsch (Res. & Dev.) SUSE Software Solutions Germany GmbH Tel: 0911-740 53 0Frankenstraße 146 FAX: 0911-740 53 479 D-90461 Nürnberg http://www.suse.deGermany Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman (HRB 36809, AG Nürnberg)
Re: Implement per-key keyboard backlight as auxdisplay?
Hi, Am 19.01.24 um 21:15 schrieb Pavel Machek: Hi! 2. Implement per-key keyboards as auxdisplay - Pro: - Already has a concept for led positions - Is conceptually closer to "multiple leds forming a singular entity" - Con: - No preexisting UPower support - No concept for special hardware lightning modes - No support for arbitrary led outlines yet (e.g. ISO style enter-key) Please do this one. Ok, so based on the discussion so far and Pavel's feedback lets try to design a custom userspace API for this. I do not believe that auxdisplay is a good fit because: Ok, so lets call this a "display". These days, framebuffers and drm handles displays. My proposal is to use similar API as other displays. So my proposal would be an ioctl interface (ioctl only no r/w) using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev. For per key controllable rgb LEDs we need to discuss a coordinate system. I propose using a fixed size of 16 rows of 64 keys, so 64x16 in standard WxH notation. And then storing RGB in separate bytes, so userspace will then always send a buffer of 192 bytes per line (64x3) x 14 rows = 3072 bytes. With the kernel driver ignoring parts of the buffer where there are no actual keys. That's really really weird interface. If you are doing RGB888 64x14, lets make it a ... display? :-). ioctl always sending 3072 bytes is really a hack. Small displays exist and are quite common, surely we'd handle this as a display: https://pajenicko.cz/displeje/graficky-oled-displej-0-66-64x48-i2c-bily-wemos-d1-mini It is 64x48. And then there's this: https://pajenicko.cz/displeje/maticovy-8x8-led-displej-s-radicem-max7219 and this: https://pajenicko.cz/displeje/maticovy-8x32-led-displej-s-radicem-max7219 One of them is 8x8. Surely those should be displays, too? But what about a light bar with, lets say, 3 zones. Is that a 3x1 display? And what about a mouse having lit mousebuttons and a single led light bar at the wrist: a 2x2 display, but one is thin but long and one is not used? Regards, Werner And yes, we'd probably want some extra ioctls on top, for example to map from input device to this and back, and maybe for various effects, too. And yes, I realize that display with holes in it and with some pixels bigger than others is weird, but it still looks like a display to me. (And phones have high-res displays with rounded corners and holes in them, so... we'll need to deal with weird displays anyway). Best regards, Pavel
Re: Implement per-key keyboard backlight as auxdisplay?
Hi! > >> 2. Implement per-key keyboards as auxdisplay > >> > >> - Pro: > >> > >> - Already has a concept for led positions > >> > >> - Is conceptually closer to "multiple leds forming a singular > >> entity" > >> > >> - Con: > >> > >> - No preexisting UPower support > >> > >> - No concept for special hardware lightning modes > >> > >> - No support for arbitrary led outlines yet (e.g. ISO style > >> enter-key) > > > > Please do this one. > > Ok, so based on the discussion so far and Pavel's feedback lets try to > design a custom userspace API for this. I do not believe that auxdisplay > is a good fit because: Ok, so lets call this a "display". These days, framebuffers and drm handles displays. My proposal is to use similar API as other displays. > So my proposal would be an ioctl interface (ioctl only no r/w) > using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev. > > For per key controllable rgb LEDs we need to discuss a coordinate > system. I propose using a fixed size of 16 rows of 64 keys, > so 64x16 in standard WxH notation. > > And then storing RGB in separate bytes, so userspace will then > always send a buffer of 192 bytes per line (64x3) x 14 rows > = 3072 bytes. With the kernel driver ignoring parts of > the buffer where there are no actual keys. That's really really weird interface. If you are doing RGB888 64x14, lets make it a ... display? :-). ioctl always sending 3072 bytes is really a hack. Small displays exist and are quite common, surely we'd handle this as a display: https://pajenicko.cz/displeje/graficky-oled-displej-0-66-64x48-i2c-bily-wemos-d1-mini It is 64x48. And then there's this: https://pajenicko.cz/displeje/maticovy-8x8-led-displej-s-radicem-max7219 and this: https://pajenicko.cz/displeje/maticovy-8x32-led-displej-s-radicem-max7219 One of them is 8x8. Surely those should be displays, too? And yes, we'd probably want some extra ioctls on top, for example to map from input device to this and back, and maybe for various effects, too. And yes, I realize that display with holes in it and with some pixels bigger than others is weird, but it still looks like a display to me. (And phones have high-res displays with rounded corners and holes in them, so... we'll need to deal with weird displays anyway). Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Mesa >= 23.3.x no longer supporting python 3.6 ...
On Thu, Jan 18, 2024 at 01:37:52PM +0100, Stefan Dirsch wrote: > Hi > > I noticed that with version 23.3.x Mesa no longer can be built with python > 2.6. It still worked with Mesa 23.2.1. Of course I've meant python 3.6. Sorry for the confusion! > It fails with > > [ 95s] Traceback (most recent call last): > [ 95s] File "../src/intel/genxml/gen_bits_header.py", line 23, in > [ 95s] import intel_genxml > [ 95s] File > "/home/abuild/rpmbuild/BUILD/mesa-23.3.3/src/intel/genxml/intel_ > genxml.py", line 5 > [ 95s] from __future__ import annotations > [ 95s] ^ > [ 95s] SyntaxError: future feature annotations is not defined > > When removing __future__ line like this > > --- mesa-23.3.3/src/intel/genxml/intel_genxml.py.orig 2024-01-12 > 10:26:26.314070540 +0100 > +++ mesa-23.3.3/src/intel/genxml/intel_genxml.py2024-01-12 > 10:26:38.682317490 +0100 > @@ -2,7 +2,6 @@ > # Copyright © 2019, 2022 Intel Corporation > # SPDX-License-Identifier: MIT > > -from __future__ import annotations > from collections import OrderedDict > import copy > import io > > this results in the following failure. > > [ 113s] Traceback (most recent call last): > [ 113s] File "../src/intel/genxml/gen_bits_header.py", line 23, in > [ 113s] import intel_genxml > [ 113s] File > "/home/abuild/rpmbuild/BUILD/mesa-23.3.3/src/intel/genxml/intel_ > genxml.py", line 51, in > [ 113s] def add_struct_refs(items: typing.OrderedDict[str, bool], node: > et. > Element) -> None: > [ 113s] AttributeError: module 'typing' has no attribute 'OrderedDict' > > I'm wondering if Mesa developers are interested in still supporting python > 3.6? > > Unfortunately currently it's not an option for SUSE's enterprise product to > update to a newer python (would be probably 3.12) (due to many customers still > relying on python 3.6 and we lack the ressources of adding and maintaining a > (full!) second python development stack). :-( > > If the answer to the question above is a no. :-( How hard would it be to > adjust the code to python 3.6? Any suggestions to how it could be done? Or is > there any other workaround available? > > I had a quick look between 23.2.1 and 23.3.3 and I've seen that > > from __future__ import annotations > > and > > typing.OrderedDict > > have already been used in 23.2.1 (although it was in gen_sort_tags.py and > now has been moved to new intel_genxml.py). So not sure why this fails now > or was working before ... > > Any help here would be appreciated. SUSE is definitely interested in shipping > the latest Mesa with our latest enterprise product. Thanks, Stefan Public Key available -- Stefan Dirsch (Res. & Dev.) SUSE Software Solutions Germany GmbH Tel: 0911-740 53 0Frankenstraße 146 FAX: 0911-740 53 479 D-90461 Nürnberg http://www.suse.deGermany Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman (HRB 36809, AG Nürnberg)
Re: Mesa >= 23.3.x and python 2.6 ...
On 2024-01-18 04:37:52, Stefan Dirsch wrote: > Hi > > I noticed that with version 23.3.x Mesa no longer can be built with python > 2.6. It still worked with Mesa 23.2.1. As mentioned in other emails, this was typo where 3.6 was intended. > > It fails with > > [ 95s] Traceback (most recent call last): > [ 95s] File "../src/intel/genxml/gen_bits_header.py", line 23, in > [ 95s] import intel_genxml > [ 95s] File > "/home/abuild/rpmbuild/BUILD/mesa-23.3.3/src/intel/genxml/intel_ > genxml.py", line 5 > [ 95s] from __future__ import annotations > [ 95s] ^ > [ 95s] SyntaxError: future feature annotations is not defined > I guess this code first appeared in Dylan's: 4fd2e15855d ("intel/genxml: add type annotations to gen_sort_tags.py") and then became part of the standard tests a few commits later in: 1f0a0a46d97 ("meson: run genxml sort tests") back in Oct 2022. So, I guess at that point 'ninja test' would have failed with Python 3.6. Then, I suppose I propagated this to being used on every build in: 0495f952d48 ("intel/genxml: Add genxml_import.py script") in Sept 2023. Maybe Dylan knows how we might make this compatible with Python 3.6, assuming we want to. :) https://devguide.python.org/versions/ -Jordan
Re: [PATCH v3 0/3] Convert Microchip's HLCDC Text based DT bindings to JSON schema
On Thu, Jan 18, 2024 at 08:30:40PM +0100, Sam Ravnborg wrote: > Hi Dharma et al. > > On Thu, Jan 18, 2024 at 02:56:09PM +0530, Dharma Balasubiramani wrote: > > Converted the text bindings to YAML and validated them individually using > > following commands > > > > $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ > > $ make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ > > > > changelogs are available in respective patches. > > > > Dharma Balasubiramani (3): > > dt-bindings: display: convert Atmel's HLCDC to DT schema > > dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema > > dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format > > I know this is a bit late to ask - sorry in advance. > > The binding describes the single IP block as a multi functional device, > but it is a single IP block that includes the display controller and a > simple pwm that can be used for contrast or backlight. > > If we ignore the fact that the current drivers for hlcdc uses an mfd > abstraction, is this then the optimal way to describe the HW? > > > In one of my stale git tree I converted atmel lcdc to DT, and here > I used: > > + "#pwm-cells": > +description: > + This PWM chip use the default 3 cells bindings > + defined in ../../pwm/pwm.yaml. > +const: 3 > + > + clocks: > +maxItems: 2 > + > + clock-names: > +maxItems: 2 > +items: > + - const: lcdc_clk > + - const: hclk > > This proved to be a simple way to describe the HW. > > To make the DT binding backward compatible you likely need to add a few > compatible that otherwise would have been left out - but that should do > the trick. > > The current atmel hlcdc driver that is split in three is IMO an > over-engineering, and the driver could benefit merging it all in one. > And the binding should not prevent this. I agree on all this, but a conversion is not really the time to redesign things. Trust me, I've wanted to on lots of conversions. It should be possible to simplify the driver side while keeping the DT as-is. Just make the display driver bind to the MFD node instead. After that, then one could look at flattening everything to 1 node. Rob
Re: [PATCH v3 2/3] dt-bindings: atmel, hlcdc: convert pwm bindings to json-schema
On Thu, Jan 18, 2024 at 02:56:11PM +0530, Dharma Balasubiramani wrote: > Convert device tree bindings for Atmel's HLCDC PWM controller to YAML > format. > > Signed-off-by: Dharma Balasubiramani > Reviewed-by: Conor Dooley > --- > changelog > v2 -> v3 > - Remove '|' in description, as there is no formatting to preserve. > - Delete the description for pwm-cells. > - Drop the label for pwm node as it not used. > v1 -> v2 > - Remove the explicit copyrights. > - Modify title (not include words like binding/driver). > - Modify description actually describing the hardware and not the driver. > - Remove pinctrl properties which aren't required. > - Drop parent node and it's other sub-device node which are not related here. > --- > .../bindings/pwm/atmel,hlcdc-pwm.yaml | 44 +++ > .../bindings/pwm/atmel-hlcdc-pwm.txt | 29 > 2 files changed, 44 insertions(+), 29 deletions(-) > create mode 100644 Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml > delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt > > diff --git a/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml > b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml > new file mode 100644 > index ..4f4cc21fe4f7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml > @@ -0,0 +1,44 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pwm/atmel,hlcdc-pwm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Atmel's HLCDC's PWM controller > + > +maintainers: > + - Nicolas Ferre > + - Alexandre Belloni > + - Claudiu Beznea > + > +description: > + The LCDC integrates a Pulse Width Modulation (PWM) Controller. This block > + generates the LCD contrast control signal (LCD_PWM) that controls the > + display's contrast by software. LCDC_PWM is an 8-bit PWM signal that can be > + converted to an analog voltage with a simple passive filter. LCD display > + panels have different backlight specifications in terms of minimum/maximum > + values for PWM frequency. If the LCDC PWM frequency range does not match > the > + LCD display panel, it is possible to use the standalone PWM Controller to > + drive the backlight. > + > +properties: > + compatible: > +const: atmel,hlcdc-pwm > + > + "#pwm-cells": > +const: 3 > + > +required: > + - compatible > + - "#pwm-cells" > + > +additionalProperties: false > + > +examples: > + - | > +pwm { > + compatible = "atmel,hlcdc-pwm"; > + pinctrl-names = "default"; > + pinctrl-0 = <_lcd_pwm>; > + #pwm-cells = <3>; > +}; Move the example to the MFD schema. Or just drop if already there. Rob
[PATCH v2] drm/i915/mtl: Wake GT before sending H2G message
Instead of waiting until the interrupt reaches GuC, we can grab a forcewake while triggering the H2G interrupt. GEN11_GUC_HOST_INTERRUPT is inside sgunit and is not affected by forcewakes. However, there could be some delays when platform is entering/exiting some higher level platform sleep states and a H2G is triggered. A forcewake ensures those sleep states have been fully exited and further processing occurs as expected. The hysteresis timers for C6 and higher sleep states will ensure there is no unwanted race between the wake and processing of the interrupts by GuC. This will have an official WA soon so adding a FIXME in the comments. v2: Make the new ranges watertight to address BAT failures and update commit message (Matt R). Reviewed-by: Matt Roper Signed-off-by: Vinay Belgaumkar --- drivers/gpu/drm/i915/intel_uncore.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index dfefad5a5fec..76400e9c40f0 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1800,7 +1800,10 @@ static const struct intel_forcewake_range __mtl_fw_ranges[] = { GEN_FW_RANGE(0x24000, 0x2, 0), /* 0x24000 - 0x2407f: always on 0x24080 - 0x2: reserved */ - GEN_FW_RANGE(0x3, 0x3, FORCEWAKE_GT) + GEN_FW_RANGE(0x3, 0x3, FORCEWAKE_GT), + GEN_FW_RANGE(0x4, 0x1901ef, 0), + GEN_FW_RANGE(0x1901f0, 0x1901f3, FORCEWAKE_GT) + /* FIXME: WA to wake GT while triggering H2G */ }; /* -- 2.38.1
Re: Mesa >= 23.3.x and python 2.6 ...
On Fri, Jan 19, 2024 at 12:35:58PM -0500, Matt Turner wrote: > On Thu, Jan 18, 2024 at 10:22 AM Stefan Dirsch wrote: > > I noticed that with version 23.3.x Mesa no longer can be built with python > > 2.6. It still worked with Mesa 23.2.1. > > For anyone who got this far and was completely incredulous... this > (and the subject) is typo'd -- the problem is about Python 3.6, not > 2.6. I'm pretty sure you were the first and only one. :-( I've corrected it in the body by doing a reply to my own message, but how do I correct a typo in the subject ... I'll try to send the message again with also the correct subject. Then I'll be blamed for bringing up the same topic twice and spamming the list. Sigh. Thanks for having a look at the message nevertheless! Thanks, Stefan Public Key available -- Stefan Dirsch (Res. & Dev.) SUSE Software Solutions Germany GmbH Tel: 0911-740 53 0Frankenstraße 146 FAX: 0911-740 53 479 D-90461 Nürnberg http://www.suse.deGermany Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman (HRB 36809, AG Nürnberg)
[PATCH] drm/rockchip: vop2: add a missing unlock in vop2_crtc_atomic_enable()
Unlock before returning on the error path. Fixes: 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588") Signed-off-by: Harshit Mogalapalli --- This is based on static analysis. Only compile tested. Note: Smatch found this. --- drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c index 85b3b4871a1d..fdd768bbd487 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c @@ -1985,8 +1985,10 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc, clock = vop2_set_intf_mux(vp, rkencoder->crtc_endpoint_id, polflags); } - if (!clock) + if (!clock) { + vop2_unlock(vop2); return; + } if (vcstate->output_mode == ROCKCHIP_OUT_MODE_ && !(vp_data->feature & VOP2_VP_FEATURE_OUTPUT_10BIT)) -- 2.39.3
Re: Implement per-key keyboard backlight as auxdisplay?
On Fri, Jan 19, 2024 at 12:51:21PM +0200, Jani Nikula wrote: > On Fri, 19 Jan 2024, Hans de Goede wrote: > > For per key controllable rgb LEDs we need to discuss a coordinate > > system. I propose using a fixed size of 16 rows of 64 keys, > > so 64x16 in standard WxH notation. > > > > And then storing RGB in separate bytes, so userspace will then > > always send a buffer of 192 bytes per line (64x3) x 14 rows > > = 3072 bytes. With the kernel driver ignoring parts of > > the buffer where there are no actual keys. > > > > I would then like the map the standard 105 key layout onto this, > > starting at x.y (column.row) coordinates of 16.6 (with 0.0 being > > the top left). Leaving plenty of space on the left top and right > > (and some on the bottom) for extra media key rows, macro keys, etc. > > > > The idea to have the standard layout at a fixed place is to allow > > userspace to have a database of preset patterns which will work > > everywhere. > > > > Note I say standard 105 key layout, but in reality for > > defining the standardized part of the buffer we should > > use the maximum amount of keys per row of all the standard layouts, > > so for row 6 (the ESC row) and for extra keys on the right outside > > the main block we use the standard layout as shown here: > > Doesn't the input stack already have to have pretty much all of this > already covered? I can view the keyboard layout in my desktop > environment, and it's a reasonably accurate match, even if unlikely to > be pixel perfect. But crucially, it has to have all the possible layouts > covered already. The kernel actually is not aware of the keyboard geometry, it had no idea if you are dealing with a standard full 101/102 keys keyboard, TKL or even smaller one, if it is split or not, maybe something like Kinesis Advantage360. Arguably, it could potentially know about 101/TLK if vendors would program accurate descriptors into their devices, but nobody does... And geometry is not a part of HID interface at all. So your desktop environment makes an [un]educated guess. > > And while I would personally hate it, you can imagine a use case where > you'd like a keypress to have a visual effect around the key you > pressed. A kind of force feedback, if you will. I don't actually know, > and correct me if I'm wrong, but feels like implementing that outside of > the input subsystem would be non-trivial. Actually I think it does not belong to the input subsystem as it is, where the goal is to deliver keystrokes and gestures to userspace. The "force feedback" kind of fits, but not really practical, again because of lack of geometry info. It is also not really essential to be fully and automatically handled by the kernel. So I think the best way is to have an API that is flexible enough for the userspace solution to control, and that is not restricted by the input core design. The hardware drivers are not restricted to using a single API, they can implement both an input device and whatever new "rgbled" and userspace can associate them by topology/sysfs. > > Cc: Dmitry, could we at least have some input from the input subsystem > POV on this? AFAICT we have received none. Sorry, I was not CCed and I missed this on the mainling list. Thanks. -- Dmitry
Re: [PATCH v2 2/2] drm/amdgpu: Implement check_async_props for planes
On Fri, Jan 19, 2024 at 03:12:35PM -0300, André Almeida wrote: > AMD GPUs can do async flips with changes on more properties than just > the FB ID, so implement a custom check_async_props for AMD planes. > > Allow amdgpu to do async flips with IN_FENCE_ID and FB_DAMAGE_CLIPS > properties. For userspace to check if a driver support this two > properties, the strategy for now is to use TEST_ONLY commits. > > Signed-off-by: André Almeida > --- > v2: Drop overlay plane option for now > > .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 29 +++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c > index 116121e647ca..7afe8c1b62d4 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c > @@ -25,6 +25,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -1430,6 +1431,33 @@ static void > amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane, > drm_atomic_helper_plane_destroy_state(plane, state); > } > > +static int amdgpu_dm_plane_check_async_props(struct drm_property *prop, > + struct drm_plane *plane, > + struct drm_plane_state *plane_state, > + struct drm_mode_object *obj, > + u64 prop_value, u64 old_val) > +{ > + struct drm_mode_config *config = >dev->mode_config; > + int ret; > + > + if (prop != config->prop_fb_id && > + prop != config->prop_in_fence_fd && IN_FENCE should just be allowed always. > + prop != config->prop_fb_damage_clips) { This seems a bit dubious to me. How is amdgpu using the damage information during async flips? > + ret = drm_atomic_plane_get_property(plane, plane_state, > + prop, _val); > + return drm_atomic_check_prop_changes(ret, old_val, prop_value, > prop); > + } > + > + if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) { > + drm_dbg_atomic(prop->dev, > +"[OBJECT:%d] Only primary planes can be changed > during async flip\n", > +obj->id); > + return -EINVAL; > + } > + > + return 0; > +} > + > static const struct drm_plane_funcs dm_plane_funcs = { > .update_plane = drm_atomic_helper_update_plane, > .disable_plane = drm_atomic_helper_disable_plane, > @@ -1438,6 +1466,7 @@ static const struct drm_plane_funcs dm_plane_funcs = { > .atomic_duplicate_state = amdgpu_dm_plane_drm_plane_duplicate_state, > .atomic_destroy_state = amdgpu_dm_plane_drm_plane_destroy_state, > .format_mod_supported = amdgpu_dm_plane_format_mod_supported, > + .check_async_props = amdgpu_dm_plane_check_async_props, > }; > > int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, > -- > 2.43.0 -- Ville Syrjälä Intel
[PATCH v2 2/2] drm/amdgpu: Implement check_async_props for planes
AMD GPUs can do async flips with changes on more properties than just the FB ID, so implement a custom check_async_props for AMD planes. Allow amdgpu to do async flips with IN_FENCE_ID and FB_DAMAGE_CLIPS properties. For userspace to check if a driver support this two properties, the strategy for now is to use TEST_ONLY commits. Signed-off-by: André Almeida --- v2: Drop overlay plane option for now .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 29 +++ 1 file changed, 29 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c index 116121e647ca..7afe8c1b62d4 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c @@ -25,6 +25,7 @@ */ #include +#include #include #include #include @@ -1430,6 +1431,33 @@ static void amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane, drm_atomic_helper_plane_destroy_state(plane, state); } +static int amdgpu_dm_plane_check_async_props(struct drm_property *prop, + struct drm_plane *plane, + struct drm_plane_state *plane_state, + struct drm_mode_object *obj, + u64 prop_value, u64 old_val) +{ + struct drm_mode_config *config = >dev->mode_config; + int ret; + + if (prop != config->prop_fb_id && + prop != config->prop_in_fence_fd && + prop != config->prop_fb_damage_clips) { + ret = drm_atomic_plane_get_property(plane, plane_state, + prop, _val); + return drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); + } + + if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) { + drm_dbg_atomic(prop->dev, + "[OBJECT:%d] Only primary planes can be changed during async flip\n", + obj->id); + return -EINVAL; + } + + return 0; +} + static const struct drm_plane_funcs dm_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, @@ -1438,6 +1466,7 @@ static const struct drm_plane_funcs dm_plane_funcs = { .atomic_duplicate_state = amdgpu_dm_plane_drm_plane_duplicate_state, .atomic_destroy_state = amdgpu_dm_plane_drm_plane_destroy_state, .format_mod_supported = amdgpu_dm_plane_format_mod_supported, + .check_async_props = amdgpu_dm_plane_check_async_props, }; int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, -- 2.43.0
[PATCH v2 0/2] drm/atomic: Allow drivers to write their own plane check for async
Hi, AMD hardware can do more on the async flip path than just the primary plane, so to lift up the current restrictions, this patchset allows drivers to write their own check for planes for async flips. For now this patchset only allow for async commits with IN_FENCE_ID and FB_DAMAGE_CLIPS. Userspace can query if a driver supports this with TEST_ONLY commits. I will left overlay planes for a next iteration. Changes from v1: - Drop overlay planes option for now v1: https://lore.kernel.org/dri-devel/20240116045159.1015510-1-andrealm...@igalia.com/ André André Almeida (2): drm/atomic: Allow drivers to write their own plane check for async flips drm/amdgpu: Implement check_async_props for planes .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 29 + drivers/gpu/drm/drm_atomic_uapi.c | 62 ++- include/drm/drm_atomic_uapi.h | 12 include/drm/drm_plane.h | 5 ++ 4 files changed, 91 insertions(+), 17 deletions(-) -- 2.43.0
[PATCH v2 1/2] drm/atomic: Allow drivers to write their own plane check for async flips
Some hardware are more flexible on what they can flip asynchronously, so rework the plane check so drivers can implement their own check, lifting up some of the restrictions. Signed-off-by: André Almeida --- drivers/gpu/drm/drm_atomic_uapi.c | 62 ++- include/drm/drm_atomic_uapi.h | 12 ++ include/drm/drm_plane.h | 5 +++ 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index aee4a65d4959..6d5b9fec90c7 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -620,7 +620,7 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, return 0; } -static int +int drm_atomic_plane_get_property(struct drm_plane *plane, const struct drm_plane_state *state, struct drm_property *property, uint64_t *val) @@ -683,6 +683,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane, return 0; } +EXPORT_SYMBOL(drm_atomic_plane_get_property); static int drm_atomic_set_writeback_fb_for_connector( struct drm_connector_state *conn_state, @@ -1026,18 +1027,54 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state, return ret; } -static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value, +int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value, struct drm_property *prop) { if (ret != 0 || old_val != prop_value) { drm_dbg_atomic(prop->dev, - "[PROP:%d:%s] No prop can be changed during async flip\n", + "[PROP:%d:%s] This prop cannot be changed during async flip\n", prop->base.id, prop->name); return -EINVAL; } return 0; } +EXPORT_SYMBOL(drm_atomic_check_prop_changes); + +/* plane changes may have exceptions, so we have a special function for them */ +static int drm_atomic_check_plane_changes(struct drm_property *prop, + struct drm_plane *plane, + struct drm_plane_state *plane_state, + struct drm_mode_object *obj, + u64 prop_value, u64 old_val) +{ + struct drm_mode_config *config = >dev->mode_config; + int ret; + + if (plane->funcs->check_async_props) + return plane->funcs->check_async_props(prop, plane, plane_state, +obj, prop_value, old_val); + + /* +* if you are trying to change something other than the FB ID, your +* change will be either rejected or ignored, so we can stop the check +* here +*/ + if (prop != config->prop_fb_id) { + ret = drm_atomic_plane_get_property(plane, plane_state, + prop, _val); + return drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); + } + + if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) { + drm_dbg_atomic(prop->dev, + "[OBJECT:%d] Only primary planes can be changed during async flip\n", + obj->id); + return -EINVAL; + } + + return 0; +} int drm_atomic_set_property(struct drm_atomic_state *state, struct drm_file *file_priv, @@ -1100,7 +1137,6 @@ int drm_atomic_set_property(struct drm_atomic_state *state, case DRM_MODE_OBJECT_PLANE: { struct drm_plane *plane = obj_to_plane(obj); struct drm_plane_state *plane_state; - struct drm_mode_config *config = >dev->mode_config; plane_state = drm_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) { @@ -1108,19 +1144,11 @@ int drm_atomic_set_property(struct drm_atomic_state *state, break; } - if (async_flip && prop != config->prop_fb_id) { - ret = drm_atomic_plane_get_property(plane, plane_state, - prop, _val); - ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); - break; - } - - if (async_flip && plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) { - drm_dbg_atomic(prop->dev, - "[OBJECT:%d] Only primary planes can be changed during async flip\n", - obj->id); - ret = -EINVAL; - break; + if (async_flip) { + ret =
Re: [PATCH 02/11] dt-bindings: gpu: Add PowerVR Series5 SGX GPUs
On Wed, Jan 10, 2024 at 10:38:57AM +0200, Tony Lindgren wrote: > * Krzysztof Kozlowski [240109 19:53]: > > On 09/01/2024 18:19, Andrew Davis wrote: > > > The Imagination PowerVR Series5 "SGX" GPU is part of several SoCs from > > > multiple vendors. Describe how the SGX GPU is integrated in these SoC, > > > including register space and interrupts. Clocks, reset, and power domain > > > information is SoC specific. > > > > > > Signed-off-by: Andrew Davis > > > Reviewed-by: Javier Martinez Canillas > > > > > > > + clock-names: > > > +minItems: 1 > > > +items: > > > + - const: core > > > + - const: mem > > > + - const: sys > > > > There are no devices currently using third clock, but I assume it is > > expected or possible. > > I think the third clock is typically merged with one of the two clocks but > yeah possibly it's a separate clocke in some cases. > > > Reviewed-by: Krzysztof Kozlowski > > Looks good to me too. > > So for merging these, as many of the changes touch the omap variants, I > could set up an immutable branch with all the changes after -rc1. Or I can > ack the patches too if somebody has better ideas. Just take all but patches 10 and 11. I don't think it matters if the binding is there for them as long as it is all there in next. No one is paying that close attention to the warnings I think. Rob
Re: Mesa >= 23.3.x and python 2.6 ...
On Thu, Jan 18, 2024 at 10:22 AM Stefan Dirsch wrote: > I noticed that with version 23.3.x Mesa no longer can be built with python > 2.6. It still worked with Mesa 23.2.1. For anyone who got this far and was completely incredulous... this (and the subject) is typo'd -- the problem is about Python 3.6, not 2.6.
Re: [PATCH v2 2/2] backlight: Add Kinetic KTD2801 driver
On Friday, January 19, 2024 6:29:21 PM CET Christophe JAILLET wrote: > Le 18/01/2024 à 18:32, Duje Mihanović a écrit : > > Add driver for the Kinetic KTD2801 backlight driver. > > > > Signed-off-by: Duje Mihanović > > > > --- > > ... > > > + ktd2801->gpiod = devm_gpiod_get(dev, "ctrl", GPIOD_OUT_HIGH); > > + if (IS_ERR(ktd2801->gpiod)) > > + return dev_err_probe(dev, PTR_ERR(dev), > > PTR_ERR(ktd2801->gpiod); ? Good catch, I'll fix it in v3. Regards, -- Duje
Re: [PATCH v2 2/2] backlight: Add Kinetic KTD2801 driver
Le 18/01/2024 à 18:32, Duje Mihanović a écrit : Add driver for the Kinetic KTD2801 backlight driver. Signed-off-by: Duje Mihanović --- ... + ktd2801->gpiod = devm_gpiod_get(dev, "ctrl", GPIOD_OUT_HIGH); + if (IS_ERR(ktd2801->gpiod)) + return dev_err_probe(dev, PTR_ERR(dev), PTR_ERR(ktd2801->gpiod); ? CJ
Re: [PATCH v7 2/9] drm/panic: Add a drm panic handler
On 12/01/2024 14:50, Daniel Vetter wrote: On Thu, Jan 04, 2024 at 05:00:46PM +0100, Jocelyn Falempe wrote: +/** + * drm_panic_init() - Initialize drm-panic subsystem + * + * register the panic notifier + */ +void drm_panic_init(void) +{ + atomic_notifier_chain_register(_notifier_list, + _panic_notifier); Ok I've found another one after checking core panic code. This is the wrong hook, we want to be a sttruct kmsg_dumper and use kmsg_dump_register. And again once for each drm_panic_device so that we can rely on core locking, as I've explained in the other reply. Also because it trashes buffers from userspace I think by default we want to only dump on panic, so KMS_DUMP_PANIC. -Sima I've tested this change and I don't think kmsg_dumper is the right callback. At least with panic_notifier, you get one line on why the panic occurs. With kmsg_dumper you get the whole kmsg buffer, but I don't want to throw that at the user. And it's not possible to extract just the panic reason from the log. I think the debug information should go in a QRCode, so you can actually report the crash somewhere, and copy/paste the backtrace and other info to the bug. I've a PoC for that, but I prefer to have the main drm_panic merged before working further on this. Anyway it's pretty easy to change from one to the other, since the API are quite similar. So if we need the complete kmsg log someday, it should be easy to switch. Best regards, -- Jocelyn
Re: [PATCH v3 3/3] dt-bindings: soc: mediatek: Change mediatek,gce-events to refernece
On Fri, Jan 19, 2024 at 02:32:24PM +0800, Jason-JH.Lin wrote: > Change mediatek,gce-events property to reference mediatek,gce-props.yaml > instead of defining itself. > > Signed-off-by: Jason-JH.Lin Reviewed-by: Conor Dooley Cheers, Conor. signature.asc Description: PGP signature
Re: [PATCH] drm/doc/rfc: Removing missing reference to xe.rst
On Fri, Jan 19, 2024 at 04:29:50PM +, Matthew Auld wrote: > On 19/01/2024 16:25, Rodrigo Vivi wrote: > > On Tue, Jan 16, 2024 at 05:03:31PM -0500, Rodrigo Vivi wrote: > > > The file has already been deleted as the tasks were completed. > > > However the index reference was missed behind. > > > > Gentle ping on this one. > > I should have mentioned here that this fixes a doc build warning: > > > > Documentation/gpu/rfc/index.rst:35: WARNING: toctree contains reference to > > nonexisting document 'gpu/rfc/xe' > > > > > > > > Fixes: d11dc7aa98e5 ("drm/doc/rfc: Remove Xe's pre-merge plan") > > > Cc: Lucas De Marchi > > > Signed-off-by: Rodrigo Vivi > Reviewed-by: Matthew Auld Thank you for the prompt request. pushed to drm-misc-next
Re: [PATCH v3 2/3] dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to reference
On Fri, Jan 19, 2024 at 02:32:23PM +0800, Jason-JH.Lin wrote: > Change mediatek,gce-events property to reference mediatek,gce-props.yaml > instead of defining itself. > > Signed-off-by: Jason-JH.Lin Reviewed-by: Conor Dooley Cheers, Conor. signature.asc Description: PGP signature
Re: [PATCH v3 1/3] dt-bindings: mailbox: Add mediatek,gce-props.yaml
Rob, On Fri, Jan 19, 2024 at 02:32:22PM +0800, Jason-JH.Lin wrote: > Add mediatek,gce-props.yaml for common GCE properties that is used for > both mailbox providers and consumers. We place the common property > "mediatek,gce-events" in this binding currently. > > The property "mediatek,gce-events" is used for GCE event ID corresponding > to a hardware event signal sent by the hardware or a sofware driver. > If the mailbox providers or consumers want to manipulate the value of > the event ID, they need to know the specific event ID. > > Signed-off-by: Jason-JH.Lin > --- > .../bindings/mailbox/mediatek,gce-props.yaml | 52 +++ Is bindings/mailbox the correct directory to put this in? > 1 file changed, 52 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml > > diff --git > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml > new file mode 100644 > index ..68b519ff089f > --- /dev/null > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml > @@ -0,0 +1,52 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mailbox/mediatek,gce-props.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek Global Command Engine Common Propertes > + > +maintainers: > + - Houlong Wei > + > +description: > + The Global Command Engine (GCE) is an instruction based, multi-threaded, > + single-core command dispatcher for MediaTek hardware. The Command Queue > + (CMDQ) mailbox driver is a driver for GCE, implemented using the Linux > + mailbox framework. It is used to receive messages from mailbox consumers > + and configure GCE to execute the specified instruction set in the message. > + We use mediatek,gce-mailbox.yaml to define the properties for CMDQ mailbox > + driver. A device driver that uses the CMDQ driver to configure its hardware > + registers is a mailbox consumer. The mailbox consumer can request a mailbox > + channel corresponding to a GCE hardware thread to send a message, > specifying > + that the GCE thread to configure its hardware. The mailbox provider can > also > + reserved a mailbox channel to configure GCE hardware register by the > spcific > + GCE thread. This binding defines the common GCE properties for both mailbox > + provider and consumers. > + > +properties: > + mediatek,gce-events: > +description: > + GCE has an event table in SRAM, consisting of 1024 event IDs (0~1023). > + Each event ID has a boolean event value with the default value 0. > + The property mediatek,gce-events is used to obtain the event IDs. > + Some gce-events are hardware-bound and cannot be changed by software. > + For instance, in MT8195, when VDO0_MUTEX is stream done, VDO_MUTEX will > + send an event signal to GCE, setting the value of event ID 597 to 1. > + Similarly, in MT8188, the value of event ID 574 will be set to 1 when > + VOD0_MUTEX is stream done. > + On the other hand, some gce-events are not hardware-bound and can be > + changed by software. For example, in MT8188, we can set the value of > + event ID 855, which is not bound to any hardware, to 1 when the driver > + in the secure world completes a task. However, in MT8195, event ID 855 > + is already bound to VDEC_LAT1, so we need to select another event ID to > + achieve the same purpose. This event ID can be any ID that is not bound > + to any hardware and is not yet used in any software driver. > + To determine if the event ID is bound to the hardware or used by a > + software driver, refer to the GCE header > + include/dt-bindings/gce/-gce.h of each chip. > +$ref: /schemas/types.yaml#/definitions/uint32-array > +minItems: 1 > +maxItems: 1024 > + > +additionalProperties: true > -- > 2.18.0 > signature.asc Description: PGP signature
Re: [PATCH v2 2/2] backlight: Add Kinetic KTD2801 driver
On Friday, January 19, 2024 11:07:09 AM CET Daniel Thompson wrote: > On Thu, Jan 18, 2024 at 06:32:39PM +0100, Duje Mihanović wrote: > > Add driver for the Kinetic KTD2801 backlight driver. > > > > Signed-off-by: Duje Mihanović > > > > --- > > Shared ExpressWire handling code and preemption watchdogs haven't been > > implemented in this version as my questions regarding these two weren't > > answered. > > --- > > The last mail I saw on this topic was of the "do you have any better > ideas?" variety. I (mis)read that as "unless you have any > better ideas" and didn't realize you were waiting for anything. > > I didn't have any better ideas! My apologies, I'll write the library as I proposed in that email. Regards, -- Duje
Re: [PATCH v2 2/2] backlight: Add Kinetic KTD2801 driver
On Friday, January 19, 2024 10:02:33 AM CET Linus Walleij wrote: > Hi Duje, > > thanks for your patch! > > On Thu, Jan 18, 2024 at 6:33 PM Duje Mihanović wrote: > > Add driver for the Kinetic KTD2801 backlight driver.> > > Signed-off-by: Duje Mihanović > > Add some commit message? Besides the usual short explanation of the hardware I'd also add a link to the datasheet in the commit message if that's appropriate. > > +#include > > +#include > > +#include > > +#include > > I don't think you need , the compatible table works without > that (is in the device driver core). Can confirm it compiles without. > > +/* These values have been extracted from Samsung's driver. */ > > +#define KTD2801_EXPRESSWIRE_DETECT_DELAY_US150 > > +#define KTD2801_EXPRESSWIRE_DETECT_US 270 > > +#define KTD2801_LOW_BIT_HIGH_TIME_US 5 > > +#define KTD2801_LOW_BIT_LOW_TIME_US(4 * > > KTD2801_HIGH_BIT_LOW_TIME_US) +#define KTD2801_HIGH_BIT_LOW_TIME_US > > 5 > > +#define KTD2801_HIGH_BIT_HIGH_TIME_US (4 * > > KTD2801_HIGH_BIT_LOW_TIME_US) +#define KTD2801_DATA_START_US > > 5 > > +#define KTD2801_END_OF_DATA_LOW_US 10 > > +#define KTD2801_END_OF_DATA_HIGH_US350 > > +#define KTD2801_PWR_DOWN_DELAY_US 2600 > > + > > +#define KTD2801_DEFAULT_BRIGHTNESS 100 > > +#define KTD2801_MAX_BRIGHTNESS 255 > > + > > +struct ktd2801_backlight { > > + struct backlight_device *bd; > > + struct gpio_desc *gpiod; > > + bool was_on; > > +}; > > + > > +static int ktd2801_update_status(struct backlight_device *bd) > > +{ > > + struct ktd2801_backlight *ktd2801 = bl_get_data(bd); > > + u8 brightness = (u8) backlight_get_brightness(bd); > > + > > + if (backlight_is_blank(bd)) { > > + gpiod_set_value(ktd2801->gpiod, 0); > > + udelay(KTD2801_PWR_DOWN_DELAY_US); > > That's 2600 us, a pretty long delay in a hard loop or delay timer! > > Can you use usleep_range() instead, at least for this one? Sounds like a good idea. Should I also make that GPIO pulldown _cansleep while at it? > > + for (int i = 0; i < 8; i++) { > > + u8 next_bit = (brightness & 0x80) >> 7; > > I would just: > > #include > > bool next_bit = !!(brightness & BIT(7)); Will do. Regards, -- Duje
[PATCH v2 3/3] drm/syncobj: call might_sleep before waiting for fence submission
If either the DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT or DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE flags are passed to drm_syncobj_array_wait_timeout, the function might sleep if the fence at one of the given timeline points has not yet been submitted. Therefore, we should call might_sleep in that case to catch potential bugs. Signed-off-by: Erik Kurzinger --- drivers/gpu/drm/drm_syncobj.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index c59bb02e2c07..e04965878a08 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1062,8 +1062,10 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, uint32_t signaled_count, i; if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT | -DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) +DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) { + might_sleep(); lockdep_assert_none_held_once(); + } points = kmalloc_array(count, sizeof(*points), GFP_KERNEL); if (points == NULL) -- 2.43.0
[PATCH v2 2/3] drm/syncobj: reject invalid flags in drm_syncobj_find_fence
The only flag that is meaningful to drm_syncobj_find_fence is DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT. It should return -EINVAL for any other flag bits. Signed-off-by: Erik Kurzinger --- drivers/gpu/drm/drm_syncobj.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 97be8b140599..c59bb02e2c07 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -448,6 +448,9 @@ int drm_syncobj_find_fence(struct drm_file *file_private, u64 timeout = nsecs_to_jiffies64(DRM_SYNCOBJ_WAIT_FOR_SUBMIT_TIMEOUT); int ret; + if (flags & ~DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) + return -EINVAL; + if (!syncobj) return -ENOENT; -- 2.43.0
[PATCH v2 1/3] drm/syncobj: call drm_syncobj_fence_add_wait when WAIT_AVAILABLE flag is set
When waiting for a syncobj timeline point whose fence has not yet been submitted with the WAIT_FOR_SUBMIT flag, a callback is registered using drm_syncobj_fence_add_wait and the thread is put to sleep until the timeout expires. If the fence is submitted before then, drm_syncobj_add_point will wake up the sleeping thread immediately which will proceed to wait for the fence to be signaled. However, if the WAIT_AVAILABLE flag is used instead, drm_syncobj_fence_add_wait won't get called, meaning the waiting thread will always sleep for the full timeout duration, even if the fence gets submitted earlier. If it turns out that the fence *has* been submitted by the time it eventually wakes up, it will still indicate to userspace that the wait completed successfully (it won't return -ETIME), but it will have taken much longer than it should have. To fix this, we must call drm_syncobj_fence_add_wait if *either* the WAIT_FOR_SUBMIT flag or the WAIT_AVAILABLE flag is set. The only difference being that with WAIT_FOR_SUBMIT we will also wait for the fence to be signaled after it has been submitted while with WAIT_AVAILABLE we will return immediately. IGT test patch: https://lists.freedesktop.org/archives/igt-dev/2024-January/067537.html v1 -> v2: adjust lockdep_assert_none_held_once condition Fixes: 01d6c3578379 ("drm/syncobj: add support for timeline point wait v8") Signed-off-by: Erik Kurzinger --- drivers/gpu/drm/drm_syncobj.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 94ebc71e5be5..97be8b140599 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1058,7 +1058,8 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, uint64_t *points; uint32_t signaled_count, i; - if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) + if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT | +DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) lockdep_assert_none_held_once(); points = kmalloc_array(count, sizeof(*points), GFP_KERNEL); @@ -1127,7 +1128,8 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, * fallthough and try a 0 timeout wait! */ - if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { + if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT | +DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) { for (i = 0; i < count; ++i) drm_syncobj_fence_add_wait(syncobjs[i], [i]); } -- 2.43.0
Re: [PATCH] drm/doc/rfc: Removing missing reference to xe.rst
On 19/01/2024 16:25, Rodrigo Vivi wrote: On Tue, Jan 16, 2024 at 05:03:31PM -0500, Rodrigo Vivi wrote: The file has already been deleted as the tasks were completed. However the index reference was missed behind. Gentle ping on this one. I should have mentioned here that this fixes a doc build warning: Documentation/gpu/rfc/index.rst:35: WARNING: toctree contains reference to nonexisting document 'gpu/rfc/xe' Fixes: d11dc7aa98e5 ("drm/doc/rfc: Remove Xe's pre-merge plan") Cc: Lucas De Marchi Signed-off-by: Rodrigo Vivi Reviewed-by: Matthew Auld
Re: [PATCH] drm/doc/rfc: Removing missing reference to xe.rst
On Tue, Jan 16, 2024 at 05:03:31PM -0500, Rodrigo Vivi wrote: > The file has already been deleted as the tasks were completed. > However the index reference was missed behind. Gentle ping on this one. I should have mentioned here that this fixes a doc build warning: Documentation/gpu/rfc/index.rst:35: WARNING: toctree contains reference to nonexisting document 'gpu/rfc/xe' > > Fixes: d11dc7aa98e5 ("drm/doc/rfc: Remove Xe's pre-merge plan") > Cc: Lucas De Marchi > Signed-off-by: Rodrigo Vivi > --- > Documentation/gpu/rfc/index.rst | 4 > 1 file changed, 4 deletions(-) > > diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst > index e4f7b005138d..476719771eef 100644 > --- a/Documentation/gpu/rfc/index.rst > +++ b/Documentation/gpu/rfc/index.rst > @@ -31,7 +31,3 @@ host such documentation: > .. toctree:: > > i915_vm_bind.rst > - > -.. toctree:: > - > - xe.rst > -- > 2.43.0 >
Re: drm/loongson: Error out if no VRAM detected
Hi, Thanks a lot for contribution. On 2024/1/19 18:40, Huacai Chen wrote: If there is no VRAM (it is true if there is a discreted card), Why the dedicated VRAM is gone whenthere is a discrete card? As far as I know, this is only possible on Loongson 3C5000 + aspeed BMC server hardware platform where the dedicated VRAM chip of Loongson Graphics is NOT soldered on the motherboard. Probably for cost reason, but then, the platform BIOS(either UEFI or PMON) should turn off the Loongson integrated graphics. Because without dedicated VRAM, this driver can not work correctly. Or carve out part of system RAM as VRAM, and write the base address and size to the BAR 2 of the GPU PCI device. This is NOT true for Loongson 3A5000/3A6000 desktop hardware, because I have do a lot test on various platform[1] before this driver was merged. It never happens on a sane hardware configuration. Please update the commit message and limit the scope. [1] https://github.com/loongson-gfx/loongson_boards we get such an error and Xorg fails to start: Yeah, If there is no dedicated VRAM, the driver can't allocate memory for framebuffer. But this is probably more about the hardware configuration issue, not a driver issue. [ 136.401131] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 137.444342] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 138.871166] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 140.444078] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 142.403993] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 143.970625] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 145.862013] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed So in lsdc_get_dedicated_vram() we error out if no VRAM (or VRAM is less than 1MB which is also an unusable case) detected. This is not expected, if you want this driver be there and run normally. You should guarantee that there have at least 64MiB dedicated VRAM. I'm OK if this patch is strongly requested, but this is a kind of error handling. Please give more details about the hardware in using and explain why there is no dedicated VRAM available for your hardware. Signed-off-by: Huacai Chen --- drivers/gpu/drm/loongson/lsdc_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c b/drivers/gpu/drm/loongson/lsdc_drv.c index 89ccc0c43169..d8ff60b46abe 100644 --- a/drivers/gpu/drm/loongson/lsdc_drv.c +++ b/drivers/gpu/drm/loongson/lsdc_drv.c @@ -184,7 +184,7 @@ static int lsdc_get_dedicated_vram(struct lsdc_device *ldev, drm_info(ddev, "Dedicated vram start: 0x%llx, size: %uMiB\n", (u64)base, (u32)(size >> 20)); - return 0; + return (size > SZ_1M) ? 0 : -ENODEV; } static struct lsdc_device *
Re: Implement per-key keyboard backlight as auxdisplay?
Hi, Am 19.01.24 um 11:51 schrieb Jani Nikula: On Fri, 19 Jan 2024, Hans de Goede wrote: For per key controllable rgb LEDs we need to discuss a coordinate system. I propose using a fixed size of 16 rows of 64 keys, so 64x16 in standard WxH notation. And then storing RGB in separate bytes, so userspace will then always send a buffer of 192 bytes per line (64x3) x 14 rows = 3072 bytes. With the kernel driver ignoring parts of the buffer where there are no actual keys. I would then like the map the standard 105 key layout onto this, starting at x.y (column.row) coordinates of 16.6 (with 0.0 being the top left). Leaving plenty of space on the left top and right (and some on the bottom) for extra media key rows, macro keys, etc. The idea to have the standard layout at a fixed place is to allow userspace to have a database of preset patterns which will work everywhere. Note I say standard 105 key layout, but in reality for defining the standardized part of the buffer we should use the maximum amount of keys per row of all the standard layouts, so for row 6 (the ESC row) and for extra keys on the right outside the main block we use the standard layout as shown here: Doesn't the input stack already have to have pretty much all of this already covered? I can view the keyboard layout in my desktop environment, and it's a reasonably accurate match, even if unlikely to be pixel perfect. But crucially, it has to have all the possible layouts covered already. And while I would personally hate it, you can imagine a use case where you'd like a keypress to have a visual effect around the key you pressed. A kind of force feedback, if you will. I don't actually know, and correct me if I'm wrong, but feels like implementing that outside of the input subsystem would be non-trivial. Cc: Dmitry, could we at least have some input from the input subsystem POV on this? AFAICT we have received none. BR, Jani. Don't forget: while we are currently discussing keyboards, in the future this API imho should also be usefull for other RGB devices like mice, lightbars, etc. Regards, Werner http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg For the main area of the keyboard looking at: http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png We want to max rows per key, so this means that per row we use (from the above image) : row 7: 106/109 - JIS row 8: 101/104 - ANSI row 9: 102/105 - ISO row 10: 104/107 - ABNT row 11: 106/109 - JIS (with row 7 being the main area top row) This way we can address all the possible keys in the various standard layouts in one standard wat and then the drivers can just skip keys which are not there when preparing the buffer to send to the hw / fw. One open question is if we should add padding after the main area so that the printscreen / ins / del / leftarrow of the "middle" block of http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg all start at the same x (say 32) or we just pack these directly after the main area. And the same question for the numlock block, do we align this to an x of say 36, or pack it ? As for the actual IOCTL API I think there should be the following ioctls: 1. A get-info ioctl returning a struct with the following members: { char name[64] /* Keyboard model name / identifier */ int row_begin[16]; /* The x address of the first available key per row. On a std 105key kbd this will be 16 for rows 6-11, 0 for other rows */ int row_end[16]; /* x+1 for the address of the last available key per row, end - begin gives number of keys in a row */ int rgb_zones; /* number of rgb zones for zoned keyboards. Note both zones and per key addressing may be available if effects are applied per zone. */ ? } 2. A set-leds ioctl which takes the earlier discussed 3092 bytes buffer to set all the LEDs at once, only valid if at least one row has a non 0 lenght. 3. A set-zones ioctl which takes an array of bytes sized 3 * number-of-zones containing RGB values for each zone 4. A enum_effects ioctl which takes a struct with the following members: { long size; /* Size of passed in struct including the size member itself */ long effects_mask[] } the idea being that there is an enum with effects, which gets extended as we encounter more effects and the bitmask in effects_mask has a bit set for each effects enum value which is supported. effects_mask is an array so that we don't run out of bits. If older userspace only passes 1 long (size == (2*sizeof(long)) when 2 are needed at some point in the future then the kernel will simply only fill the first long. 5. A set_effect ioctl which takes a struct with the following members: { long size; /* Size of passed in struct including the size member itself */ int effect_nr; /* enum value of the effect to enable, 0 for disable effect */ int zone; /* zone to apply the effect to */ int speed; /* cycle speed of the effect in milli-hz
Re: Implement per-key keyboard backlight as auxdisplay?
Hi, sorry have to resend, thunderbird html-ified the mail Am 19.01.24 um 09:44 schrieb Hans de Goede: Hi, On 1/18/24 18:45, Pavel Machek wrote: Hi! We have an upcoming device that has a per-key keyboard backlight, but does the control completely via a wmi/acpi interface. So no usable hidraw here for a potential userspace driver implementation ... So a quick summary for the ideas floating in this thread so far: 1. Expand leds interface allowing arbitrary modes with semi arbitrary optional attributes: - Con: - Violates the simplicity paradigm of the leds interface (e.g. with this one leds entry controls possible multiple leds) Let's not do this. 2. Implement per-key keyboards as auxdisplay - Pro: - Already has a concept for led positions - Is conceptually closer to "multiple leds forming a singular entity" - Con: - No preexisting UPower support - No concept for special hardware lightning modes - No support for arbitrary led outlines yet (e.g. ISO style enter-key) Please do this one. Ok, so based on the discussion so far and Pavel's feedback lets try to design a custom userspace API for this. I do not believe that auxdisplay is a good fit because: - auxdisplay is just a directory name, it does not seem to clearly define an API - instead the deprecated /dev/fb API is used which is deprecated - auxdisplays are very much displays (hence /dev/fb) they are typically small LCD displays with a straight widthxheight grid of square pixels - /dev/fb does gives us nothing for effects, zoned keyboard, etc. I was just checking this and wanted to write something similar. When I wrote the pro/con list I was mistaken that aux displays use either one of 2 APIs (charlcd or fb), but I was mistaken. The 8 devices implemented there are actually using 5 different apis, some of them 2 at a time. Just for reference the small list I wrote on the side just now: arm-charlcd.c - own implementation without userspace interaction (just a static text is displayed) cfag12864b.c/cfag12864bfb.c - ks0108_isinited or register_framebuffer hd44780.c - charlcd_register ht16k33.c - linedisp_register or register_framebuffer img-ascii-lcd.c - linedisp_register ks0108.c - own implementetion using parport_register_dev_model lcd2s.c - charlcd_register panel.c - charlcd_register So my proposal would be an ioctl interface (ioctl only no r/w) using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev. For per key controllable rgb LEDs we need to discuss a coordinate system. I propose using a fixed size of 16 rows of 64 keys, so 64x16 in standard WxH notation. And then storing RGB in separate bytes, so userspace will then always send a buffer of 192 bytes per line (64x3) x 14 rows = 3072 bytes. With the kernel driver ignoring parts of the buffer where there are no actual keys. The be sure the "14 rows" is a typo? And should be 16 rows? I would then like the map the standard 105 key layout onto this, starting at x.y (column.row) coordinates of 16.6 (with 0.0 being the top left). Leaving plenty of space on the left top and right (and some on the bottom) for extra media key rows, macro keys, etc. The idea to have the standard layout at a fixed place is to allow userspace to have a database of preset patterns which will work everywhere. Note I say standard 105 key layout, but in reality for defining the standardized part of the buffer we should use the maximum amount of keys per row of all the standard layouts, so for row 6 (the ESC row) and for extra keys on the right outside the main block we use the standard layout as shown here: http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg For the main area of the keyboard looking at: http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png We want to max rows per key, so this means that per row we use (from the above image) : row 7: 106/109 - JIS row 8: 101/104 - ANSI row 9: 102/105 - ISO row 10: 104/107 - ABNT row 11: 106/109 - JIS (with row 7 being the main area top row) This way we can address all the possible keys in the various standard layouts in one standard wat and then the drivers can just skip keys which are not there when preparing the buffer to send to the hw / fw. Some remarks here: - Some keyboards might have two or more leds for big keys like (iso-)enter, shift, capslock, num+, etc. that in theory are individually controllable by the firmware. In windows drivers this is usually abstracted away, but could be interesting for effects (e.g. if the top of iso-enter is separate from the bottom of iso-enter like with one of our devices). - In combination with this: The driver might not be able to tell if the actual physical keyboard is ISO or ANSI, so it might not be able the correctly assign the leds around enter correctly as being an own key or being part of ANSI- or ISO-enter. - Should the interface have different addresses for
Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t
On Fri, 19 Jan 2024, Lucas De Marchi wrote: > On Fri, Jan 19, 2024 at 10:05:57AM +0100, Thomas Hellström wrote: >>The relatively recently introduced drm/exec utility was using uint32_t >>in its interface, which was then also carried over to drm/gpuvm. >> >>Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly. >> >>Cc: Christian König >>Cc: Danilo Krummrich >>Signed-off-by: Thomas Hellström >>--- >> drivers/gpu/drm/drm_exec.c | 2 +- >> include/drm/drm_exec.h | 4 ++-- >> include/drm/drm_gpuvm.h| 2 +- >> 3 files changed, 4 insertions(+), 4 deletions(-) > > > Reviewed-by: Lucas De Marchi > > I was surprised we have quite a few places using the c99 types rather > than kernel types. > > $ git grep -ce uint[0-9][0-9]_t drivers/gpu/drm/*.c > drivers/gpu/drm/drm_atomic.c:1 > drivers/gpu/drm/drm_atomic_helper.c:7 > drivers/gpu/drm/drm_atomic_state_helper.c:1 > drivers/gpu/drm/drm_atomic_uapi.c:17 > drivers/gpu/drm/drm_color_mgmt.c:4 > drivers/gpu/drm/drm_connector.c:6 > drivers/gpu/drm/drm_crtc.c:3 > drivers/gpu/drm/drm_damage_helper.c:2 > drivers/gpu/drm/drm_debugfs_crc.c:1 > drivers/gpu/drm/drm_exec.c:1 > drivers/gpu/drm/drm_fb_helper.c:10 > drivers/gpu/drm/drm_format_helper.c:6 > drivers/gpu/drm/drm_fourcc.c:6 > drivers/gpu/drm/drm_framebuffer.c:5 > drivers/gpu/drm/drm_gem.c:1 > drivers/gpu/drm/drm_gem_dma_helper.c:1 > drivers/gpu/drm/drm_gem_shmem_helper.c:1 > drivers/gpu/drm/drm_gem_ttm_helper.c:1 > drivers/gpu/drm/drm_gem_vram_helper.c:5 > drivers/gpu/drm/drm_lease.c:6 > drivers/gpu/drm/drm_mipi_dbi.c:3 > drivers/gpu/drm/drm_mode_config.c:4 > drivers/gpu/drm/drm_mode_object.c:20 > drivers/gpu/drm/drm_modeset_helper.c:1 > drivers/gpu/drm/drm_modeset_lock.c:1 > drivers/gpu/drm/drm_of.c:3 > drivers/gpu/drm/drm_plane.c:35 > drivers/gpu/drm/drm_plane_helper.c:2 > drivers/gpu/drm/drm_prime.c:9 > drivers/gpu/drm/drm_probe_helper.c:3 > drivers/gpu/drm/drm_property.c:11 > drivers/gpu/drm/drm_simple_kms_helper.c:4 > drivers/gpu/drm/drm_syncobj.c:26 > > but maybe not worth the churn for what is already there for a long time? Personally, I think the one time churn is worth it to unify and keep the codebase in kernel types only. This is basically what we did in i915 years ago, and new c99 types don't really even creep in because there are zero examples around. It's natural to follow the style around you instead of mixing. BR, Jani. -- Jani Nikula, Intel
Re: Implement per-key keyboard backlight as auxdisplay?
Hi, Am 19.01.24 um 09:44 schrieb Hans de Goede: Hi, On 1/18/24 18:45, Pavel Machek wrote: Hi! We have an upcoming device that has a per-key keyboard backlight, but does the control completely via a wmi/acpi interface. So no usable hidraw here for a potential userspace driver implementation ... So a quick summary for the ideas floating in this thread so far: 1. Expand leds interface allowing arbitrary modes with semi arbitrary optional attributes: - Con: - Violates the simplicity paradigm of the leds interface (e.g. with this one leds entry controls possible multiple leds) Let's not do this. 2. Implement per-key keyboards as auxdisplay - Pro: - Already has a concept for led positions - Is conceptually closer to "multiple leds forming a singular entity" - Con: - No preexisting UPower support - No concept for special hardware lightning modes - No support for arbitrary led outlines yet (e.g. ISO style enter-key) Please do this one. Ok, so based on the discussion so far and Pavel's feedback lets try to design a custom userspace API for this. I do not believe that auxdisplay is a good fit because: - auxdisplay is just a directory name, it does not seem to clearly define an API - instead the deprecated /dev/fb API is used which is deprecated - auxdisplays are very much displays (hence /dev/fb) they are typically small LCD displays with a straight widthxheight grid of square pixels - /dev/fb does gives us nothing for effects, zoned keyboard, etc. I was just checking this and wanted to write something similar. When I wrote the pro/con list I was mistaken that aux displays use either one of 2 APIs (charlcd or fb), but I was mistaken. The 8 devices implemented there are actually using 5 different apis, some of them 2 at a time. Just for reference the small list I wrote on the side just now: arm-charlcd.c - own implementation without userspace interaction (just a static text is displayed) cfag12864b.c/cfag12864bfb.c - ks0108_isinited or register_framebuffer hd44780.c - charlcd_register ht16k33.c - linedisp_register or register_framebuffer img-ascii-lcd.c - linedisp_register ks0108.c - own implementetion using parport_register_dev_model lcd2s.c - charlcd_register panel.c - charlcd_register So my proposal would be an ioctl interface (ioctl only no r/w) using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev. For per key controllable rgb LEDs we need to discuss a coordinate system. I propose using a fixed size of 16 rows of 64 keys, so 64x16 in standard WxH notation. And then storing RGB in separate bytes, so userspace will then always send a buffer of 192 bytes per line (64x3) x 14 rows = 3072 bytes. With the kernel driver ignoring parts of the buffer where there are no actual keys. The be sure the "14 rows" is a typo? And should be 16 rows? I would then like the map the standard 105 key layout onto this, starting at x.y (column.row) coordinates of 16.6 (with 0.0 being the top left). Leaving plenty of space on the left top and right (and some on the bottom) for extra media key rows, macro keys, etc. The idea to have the standard layout at a fixed place is to allow userspace to have a database of preset patterns which will work everywhere. Note I say standard 105 key layout, but in reality for defining the standardized part of the buffer we should use the maximum amount of keys per row of all the standard layouts, so for row 6 (the ESC row) and for extra keys on the right outside the main block we use the standard layout as shown here: http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg For the main area of the keyboard looking at: http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png We want to max rows per key, so this means that per row we use (from the above image) : row 7: 106/109 - JIS row 8: 101/104 - ANSI row 9: 102/105 - ISO row 10: 104/107 - ABNT row 11: 106/109 - JIS (with row 7 being the main area top row) This way we can address all the possible keys in the various standard layouts in one standard wat and then the drivers can just skip keys which are not there when preparing the buffer to send to the hw / fw. Some remarks here: - Some keyboards might have two or more leds for big keys like (iso-)enter, shift, capslock, num+, etc. that in theory are individually controllable by the firmware. In windows drivers this is usually abstracted away, but could be interesting for effects (e.g. if the top of iso-enter is separate from the bottom of iso-enter like with one of our devices). - In combination with this: The driver might not be able to tell if the actual physical keyboard is ISO or ANSI, so it might not be able the correctly assign the leds around enter correctly as being an own key or being part of ANSI- or ISO-enter. - Should the interface have different addresses for the different enter and num+ styles (or even the
Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t
On Fri, Jan 19, 2024 at 10:05:57AM +0100, Thomas Hellström wrote: The relatively recently introduced drm/exec utility was using uint32_t in its interface, which was then also carried over to drm/gpuvm. Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly. Cc: Christian König Cc: Danilo Krummrich Signed-off-by: Thomas Hellström --- drivers/gpu/drm/drm_exec.c | 2 +- include/drm/drm_exec.h | 4 ++-- include/drm/drm_gpuvm.h| 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Lucas De Marchi I was surprised we have quite a few places using the c99 types rather than kernel types. $ git grep -ce uint[0-9][0-9]_t drivers/gpu/drm/*.c drivers/gpu/drm/drm_atomic.c:1 drivers/gpu/drm/drm_atomic_helper.c:7 drivers/gpu/drm/drm_atomic_state_helper.c:1 drivers/gpu/drm/drm_atomic_uapi.c:17 drivers/gpu/drm/drm_color_mgmt.c:4 drivers/gpu/drm/drm_connector.c:6 drivers/gpu/drm/drm_crtc.c:3 drivers/gpu/drm/drm_damage_helper.c:2 drivers/gpu/drm/drm_debugfs_crc.c:1 drivers/gpu/drm/drm_exec.c:1 drivers/gpu/drm/drm_fb_helper.c:10 drivers/gpu/drm/drm_format_helper.c:6 drivers/gpu/drm/drm_fourcc.c:6 drivers/gpu/drm/drm_framebuffer.c:5 drivers/gpu/drm/drm_gem.c:1 drivers/gpu/drm/drm_gem_dma_helper.c:1 drivers/gpu/drm/drm_gem_shmem_helper.c:1 drivers/gpu/drm/drm_gem_ttm_helper.c:1 drivers/gpu/drm/drm_gem_vram_helper.c:5 drivers/gpu/drm/drm_lease.c:6 drivers/gpu/drm/drm_mipi_dbi.c:3 drivers/gpu/drm/drm_mode_config.c:4 drivers/gpu/drm/drm_mode_object.c:20 drivers/gpu/drm/drm_modeset_helper.c:1 drivers/gpu/drm/drm_modeset_lock.c:1 drivers/gpu/drm/drm_of.c:3 drivers/gpu/drm/drm_plane.c:35 drivers/gpu/drm/drm_plane_helper.c:2 drivers/gpu/drm/drm_prime.c:9 drivers/gpu/drm/drm_probe_helper.c:3 drivers/gpu/drm/drm_property.c:11 drivers/gpu/drm/drm_simple_kms_helper.c:4 drivers/gpu/drm/drm_syncobj.c:26 but maybe not worth the churn for what is already there for a long time? Lucas De Marchi diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c index 5d2809de4517..20e59d88218d 100644 --- a/drivers/gpu/drm/drm_exec.c +++ b/drivers/gpu/drm/drm_exec.c @@ -72,7 +72,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec) * * Initialize the object and make sure that we can track locked objects. */ -void drm_exec_init(struct drm_exec *exec, uint32_t flags) +void drm_exec_init(struct drm_exec *exec, u32 flags) { exec->flags = flags; exec->objects = kmalloc(PAGE_SIZE, GFP_KERNEL); diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h index b5bf0b6da791..187c3ec44606 100644 --- a/include/drm/drm_exec.h +++ b/include/drm/drm_exec.h @@ -18,7 +18,7 @@ struct drm_exec { /** * @flags: Flags to control locking behavior */ - uint32_tflags; + u32 flags; /** * @ticket: WW ticket used for acquiring locks @@ -135,7 +135,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec) return !!exec->contended; } -void drm_exec_init(struct drm_exec *exec, uint32_t flags); +void drm_exec_init(struct drm_exec *exec, u32 flags); void drm_exec_fini(struct drm_exec *exec); bool drm_exec_cleanup(struct drm_exec *exec); int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj); diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h index 48311e6d664c..554046321d24 100644 --- a/include/drm/drm_gpuvm.h +++ b/include/drm/drm_gpuvm.h @@ -514,7 +514,7 @@ struct drm_gpuvm_exec { /** * @flags: the flags for the struct drm_exec */ - uint32_t flags; + u32 flags; /** * @vm: the _gpuvm to lock its DMA reservations -- 2.43.0
Re: Re: Re: Re: [Intel-xe] [PATCH 2/3] linux/bits.h: Add fixed-width GENMASK and BIT macros
On Thu, Jan 18, 2024 at 06:01:58PM -0800, Yury Norov wrote: On Thu, Jan 18, 2024 at 05:25:00PM -0600, Lucas De Marchi wrote: SA2PR11MB4874 X-OriginatorOrg: intel.com Status: RO Content-Length: 6257 Lines: 150 On Thu, Jan 18, 2024 at 01:48:43PM -0800, Yury Norov wrote: > On Thu, Jan 18, 2024 at 02:42:12PM -0600, Lucas De Marchi wrote: > > Hi, > > > > Reviving this thread as now with xe driver merged we have 2 users for > > a fixed-width BIT/GENMASK. > > Can you point where and why? See users of REG_GENMASK and REG_BIT in drivers/gpu/drm/i915 and drivers/gpu/drm/xe. I think the register definition in the xe shows it in a good way: drivers/gpu/drm/xe/regs/xe_gt_regs.h The GPU registers are mostly 32-bit wide. We don't want to accidently do something like below (s/30/33/ added for illustration purposes): #define LSC_CHICKEN_BIT_0 XE_REG_MCR(0xe7c8) #define DISABLE_D8_D16_COASLESCE REG_BIT(33) Same thing for GENMASK family of macros and for registers that are 16 or 8 bits. See e.g. drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > > On Wed, Jun 21, 2023 at 07:20:59PM -0700, Yury Norov wrote: > > > Hi Lucas, all! > > > > > > (Thanks, Andy, for pointing to this thread.) > > > > > > On Mon, May 08, 2023 at 10:14:02PM -0700, Lucas De Marchi wrote: > > > > Add GENMASK_U32(), GENMASK_U16() and GENMASK_U8() macros to create > > > > masks for fixed-width types and also the corresponding BIT_U32(), > > > > BIT_U16() and BIT_U8(). > > > > > > Can you split BIT() and GENMASK() material to separate patches? > > > > > > > All of those depend on a new "U" suffix added to the integer constant. > > > > Due to naming clashes it's better to call the macro U32. Since C doesn't > > > > have a proper suffix for short and char types, the U16 and U18 variants > > > > just use U32 with one additional check in the BIT_* macros to make > > > > sure the compiler gives an error when the those types overflow. > > > > > > I feel like I don't understand the sentence... > > > > > > > The BIT_U16() and BIT_U8() need the help of GENMASK_INPUT_CHECK(), > > > > as otherwise they would allow an invalid bit to be passed. Hence > > > > implement them in include/linux/bits.h rather than together with > > > > the other BIT* variants. > > > > > > I don't think it's a good way to go because BIT() belongs to a more basic > > > level than GENMASK(). Not mentioning possible header dependency issues. > > > If you need to test against tighter numeric region, I'd suggest to > > > do the same trick as GENMASK_INPUT_CHECK() does, but in uapi/linux/const.h > > > directly. Something like: > > >#define _U8(x)(CONST_GT(U8_MAX, x) + _AC(x, U)) > > > > but then make uapi/linux/const.h include linux/build_bug.h? > > I was thinking about leaving BIT() define where it is, and add the > > fixed-width versions in this header. I was thinking uapi/linux/const.h > > was more about allowing the U/ULL suffixes for things shared with asm. > > You can't include kernel headers in uapi code. But you can try doing > vice-versa: implement or move the pieces you need to share to the > uapi/linux/const.h, and use them in the kernel code. but in this CONST_GE() should trigger a BUG/static_assert on U8_MAX < x. AFAICS that check can't be on the uapi/ side, so there's nothing much left to change in uapi/linux/const.h. I'd expect drivers to be the primary user of these fixed-width BIT variants, hence the proposal to do in include/linux/bits.h. Ssomething like this WIP/untested diff (on top of your previous patch): diff --git a/include/linux/bits.h b/include/linux/bits.h index cb94128171b2..409cd10f7597 100644 --- a/include/linux/bits.h +++ b/include/linux/bits.h @@ -24,12 +24,16 @@ #define GENMASK_INPUT_CHECK(h, l) \ (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \ __is_constexpr((l) > (h)), (l) > (h), 0))) +#define BIT_INPUT_CHECK(type, b) \ + ((BUILD_BUG_ON_ZERO(__builtin_choose_expr( \ + __is_constexpr(b), (b) >= BITS_PER_TYPE(type), 0 #else /* * BUILD_BUG_ON_ZERO is not available in h files included from asm files, * disable the input check if that is the case. */ #define GENMASK_INPUT_CHECK(h, l) 0 +#define BIT_INPUT_CHECK(type, b) 0 #endif #define __GENMASK(t, h, l) \ @@ -44,4 +48,9 @@ #define GENMASK_U32(h, l) __GENMASK(u32, h, l) #define GENMASK_U64(h, l) __GENMASK(u64, h, l) +#define BIT_U8(b) (u8)(BIT_INPUT_CHECK(u8, b) + BIT(b)) +#define BIT_U16(b) (u16)(BIT_INPUT_CHECK(u16, b) + BIT(b)) +#define BIT_U32(b) (u32)(BIT_INPUT_CHECK(u32, b) + BIT(b)) +#define BIT_U64(b) (u64)(BIT_INPUT_CHECK(u64, b) + BIT(b)) Can you add some vertical spacing here, like between GENMASK and BIT blocks? I think gmail mangled this, because it does show up with more vertical space on the email I sent: https://lore.kernel.org/all/clamvpymzwiehjqd6jhuigymyg5ikxewxyeee2eae4tgzmaz7u@6rposizee3t6/
Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t
On 1/19/24 10:05, Thomas Hellström wrote: The relatively recently introduced drm/exec utility was using uint32_t in its interface, which was then also carried over to drm/gpuvm. Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly. Cc: Christian König Cc: Danilo Krummrich Signed-off-by: Thomas Hellström Reviewed-by: Danilo Krummrich --- drivers/gpu/drm/drm_exec.c | 2 +- include/drm/drm_exec.h | 4 ++-- include/drm/drm_gpuvm.h| 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c index 5d2809de4517..20e59d88218d 100644 --- a/drivers/gpu/drm/drm_exec.c +++ b/drivers/gpu/drm/drm_exec.c @@ -72,7 +72,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec) * * Initialize the object and make sure that we can track locked objects. */ -void drm_exec_init(struct drm_exec *exec, uint32_t flags) +void drm_exec_init(struct drm_exec *exec, u32 flags) { exec->flags = flags; exec->objects = kmalloc(PAGE_SIZE, GFP_KERNEL); diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h index b5bf0b6da791..187c3ec44606 100644 --- a/include/drm/drm_exec.h +++ b/include/drm/drm_exec.h @@ -18,7 +18,7 @@ struct drm_exec { /** * @flags: Flags to control locking behavior */ - uint32_tflags; + u32 flags; /** * @ticket: WW ticket used for acquiring locks @@ -135,7 +135,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec) return !!exec->contended; } -void drm_exec_init(struct drm_exec *exec, uint32_t flags); +void drm_exec_init(struct drm_exec *exec, u32 flags); void drm_exec_fini(struct drm_exec *exec); bool drm_exec_cleanup(struct drm_exec *exec); int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj); diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h index 48311e6d664c..554046321d24 100644 --- a/include/drm/drm_gpuvm.h +++ b/include/drm/drm_gpuvm.h @@ -514,7 +514,7 @@ struct drm_gpuvm_exec { /** * @flags: the flags for the struct drm_exec */ - uint32_t flags; + u32 flags; /** * @vm: the _gpuvm to lock its DMA reservations
Re: [PATCH v3 0/9] drm/amd/display: improve DTN color state log
On 2023-11-28 12:52, Melissa Wen wrote: > This series updates the color state section of the AMD DTN log to match > color resource differences between DCN versions. > > Currently, the DTN log considers the DCN10 color pipeline, which is > useless for newer DCN versions due to all the color capability > differences. In addition to the new color blocks and features, some > semantic differences meant that the DCN10 output was no longer suitable > for newer families. > > This version addresses comments from Siqueira and Harry [1]. It also > contains some improvements: DPP and MPC gamut remap matrix data in 31.32 > fixed point format and coverage of DCN2+ and DCN3+. > > - The first patch decouple DCN10 color state from HW log in a > preparation for color logs specific to each DCN family. > - Harry kindly provided the second patch with a way to read Gamut Remap > Matrix data in 31.32 fixed point format instead of HW values. > - With this, I replaced the DCN10 gamut remap output to display values > in the new format (third patch). > - Patches 4 and 6 fill up the color state of DPP and MPC blocks for DCN3 > from the right registers. > - As DCN3+ has a new MPC color block for post-blending Gamut Remap > matrix, in the patch 5 I reuse Harry's approach for reading DPP gamut > remap matrix and create a helper to read data of MPC gamut remap > matrix. > - Patch 7 and 9 create the new color state log specific for DCN2+ and > DCN3+. I didn't extend to DCN32 (and also DCN35) because I observed > some differences in the shaper and 3D LUT registers of this version. > - Patch 8 adds description of DPP and MPC color blocks for for better > interpretation of data. > > This new approach works well with the driver-specific color > properties[2] and steamdeck/gamescope[3] together, where we can see > color state changing from default values. I also tested with > steamdeck/KDE and DCN21/GNOME. > > Please find some `before vs after` results below: > > === > > DCN301 - Before: > --- > > DPP:IGAM format IGAM modeDGAM modeRGAM mode GAMUT mode C11 C12 > C13 C14 C21 C22 C23 C24 C31 C32 C33 C34 > [ 0]:0h BypassFixed Bypass Bypass0 > h h h h h h > [ 1]:0h BypassFixed Bypass Bypass0 > h h h h h h > [ 2]:0h BypassFixed Bypass Bypass0 > h h h h h h > [ 3]:0h BypassFixed Bypass Bypass0 > h h h h h h > > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE > [ 0]: 0h 0h 2h 3 01 0 0 > [ 1]: 0h 1h fh 2 20 0 0 > [ 2]: 0h 2h 3h 3 01 0 0 > [ 3]: 0h 3h 1h 3 20 0 0 > > > DCN301 - After (Gamescope): > --- > > DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit > depth 3DLUT size RGAM mode GAMUT adjust C11C12C13 > C14C21C22C23C24C31C32 > C33C34 > [ 0]:1 sRGBBypassRAM B RAM A > 12-bit17x17x17 RAM ABypass 00 00 00 > 00 00 00 00 00 00 00 > 00 00 > [ 1]:1 sRGBBypassRAM B RAM A > 12-bit17x17x17 RAM ABypass 00 00 00 > 00 00 00 00 00 00 00 > 00 00 > [ 2]:1 sRGBBypassRAM B RAM A > 12-bit17x17x17 RAM ABypass 00 00 00 > 00 00 00 00 00 00 00 > 00 00 > [ 3]:1 sRGBBypassRAM B RAM A > 12-bit17x17x17 RAM ABypass 00 00 00 > 00 00 00 00 00 00 00 > 00 00 > > DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: > srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 > dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0 > > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE > SHAPER mode 3DLUT mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT > GAMUT adjust C11C12C13C14C21C22 > C23C24C31C32C33C34 > [ 0]: 0h 0h 2h 3
Re: [PATCH v4 3/4] usb: gadget: functionfs: Add DMABUF import interface
Hi Paul, kernel test robot noticed the following build errors: [auto build test ERROR on usb/usb-testing] [also build test ERROR on usb/usb-next usb/usb-linus lwn/docs-next linus/master v6.7 next-20240119] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20240117-203111 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing patch link: https://lore.kernel.org/r/20240117122646.41616-4-paul%40crapouillou.net patch subject: [PATCH v4 3/4] usb: gadget: functionfs: Add DMABUF import interface config: sh-randconfig-r052-20240119 (https://download.01.org/0day-ci/archive/20240119/202401192234.0uzq25ka-...@intel.com/config) compiler: sh4-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240119/202401192234.0uzq25ka-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202401192234.0uzq25ka-...@intel.com/ All errors (new ones prefixed by >>): sh4-linux-ld: drivers/usb/gadget/function/f_fs.o: in function `ffs_dmabuf_signal_done': >> f_fs.c:(.text+0x254c): undefined reference to `dma_fence_begin_signalling' >> sh4-linux-ld: f_fs.c:(.text+0x2560): undefined reference to >> `dma_fence_signal' >> sh4-linux-ld: f_fs.c:(.text+0x2564): undefined reference to >> `dma_fence_end_signalling' sh4-linux-ld: drivers/usb/gadget/function/f_fs.o: in function `ffs_epfile_release': >> f_fs.c:(.text+0x28a0): undefined reference to `dma_buf_detach' >> sh4-linux-ld: f_fs.c:(.text+0x28a4): undefined reference to `dma_buf_put' sh4-linux-ld: drivers/usb/gadget/function/f_fs.o: in function `ffs_dmabuf_unmap_work': >> f_fs.c:(.text+0x2c6c): undefined reference to `dma_buf_unmap_attachment' >> sh4-linux-ld: f_fs.c:(.text+0x2c70): undefined reference to >> `dma_resv_reset_max_fences' >> sh4-linux-ld: f_fs.c:(.text+0x2c84): undefined reference to `dma_buf_detach' sh4-linux-ld: f_fs.c:(.text+0x2c88): undefined reference to `dma_buf_put' >> sh4-linux-ld: f_fs.c:(.text+0x2c94): undefined reference to >> `dma_fence_release' sh4-linux-ld: drivers/usb/gadget/function/f_fs.o: in function `ffs_dmabuf_transfer': >> f_fs.c:(.text+0x2e30): undefined reference to `dma_buf_get' sh4-linux-ld: f_fs.c:(.text+0x2e3c): undefined reference to `dma_buf_put' >> sh4-linux-ld: f_fs.c:(.text+0x2ef4): undefined reference to >> `dma_resv_test_signaled' >> sh4-linux-ld: f_fs.c:(.text+0x2efc): undefined reference to >> `dma_buf_map_attachment' >> sh4-linux-ld: f_fs.c:(.text+0x3098): undefined reference to >> `dma_resv_reserve_fences' >> sh4-linux-ld: f_fs.c:(.text+0x30bc): undefined reference to `dma_fence_init' >> sh4-linux-ld: f_fs.c:(.text+0x30c0): undefined reference to >> `dma_resv_add_fence' sh4-linux-ld: f_fs.c:(.text+0x30c4): undefined reference to `dma_resv_reset_max_fences' >> sh4-linux-ld: f_fs.c:(.text+0x30d4): undefined reference to >> `dma_fence_begin_signalling' sh4-linux-ld: f_fs.c:(.text+0x30e0): undefined reference to `dma_fence_end_signalling' sh4-linux-ld: f_fs.c:(.text+0x30f0): undefined reference to `dma_buf_put' sh4-linux-ld: f_fs.c:(.text+0x321c): undefined reference to `dma_fence_release' >> sh4-linux-ld: f_fs.c:(.text+0x3224): undefined reference to >> `dma_buf_unmap_attachment' sh4-linux-ld: f_fs.c:(.text+0x3228): undefined reference to `dma_resv_reset_max_fences' sh4-linux-ld: f_fs.c:(.text+0x3230): undefined reference to `dma_buf_detach' sh4-linux-ld: f_fs.c:(.text+0x3234): undefined reference to `dma_buf_put' sh4-linux-ld: drivers/usb/gadget/function/f_fs.o: in function `ffs_epfile_ioctl': f_fs.c:(.text+0x41f0): undefined reference to `dma_buf_get' >> sh4-linux-ld: f_fs.c:(.text+0x41f4): undefined reference to `dma_buf_attach' sh4-linux-ld: f_fs.c:(.text+0x4200): undefined reference to `dma_buf_detach' >> sh4-linux-ld: f_fs.c:(.text+0x4210): undefined reference to >> `dma_fence_context_alloc' sh4-linux-ld: f_fs.c:(.text+0x4220): undefined reference to `dma_buf_put' sh4-linux-ld: f_fs.c:(.text+0x43b0): undefined reference to `dma_buf_detach' sh4-linux-ld: f_fs.c:(.text+0x43b4): undefined reference to `dma_buf_put' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[PATCH v5 6/6] Documentation: usb: Document FunctionFS DMABUF API
Add documentation for the three ioctls used to attach or detach externally-created DMABUFs, and to request transfers from/to previously attached DMABUFs. Signed-off-by: Paul Cercueil --- v3: New patch --- Documentation/usb/functionfs.rst | 36 1 file changed, 36 insertions(+) diff --git a/Documentation/usb/functionfs.rst b/Documentation/usb/functionfs.rst index a3054bea38f3..d05a775bc45b 100644 --- a/Documentation/usb/functionfs.rst +++ b/Documentation/usb/functionfs.rst @@ -2,6 +2,9 @@ How FunctionFS works +Overview + + From kernel point of view it is just a composite function with some unique behaviour. It may be added to an USB configuration only after the user space driver has registered by writing descriptors and @@ -66,3 +69,36 @@ have been written to their ep0's. Conversely, the gadget is unregistered after the first USB function closes its endpoints. + +DMABUF interface + + +FunctionFS additionally supports a DMABUF based interface, where the +userspace can attach DMABUF objects (externally created) to an endpoint, +and subsequently use them for data transfers. + +A userspace application can then use this interface to share DMABUF +objects between several interfaces, allowing it to transfer data in a +zero-copy fashion, for instance between IIO and the USB stack. + +As part of this interface, three new IOCTLs have been added. These three +IOCTLs have to be performed on a data endpoint (ie. not ep0). They are: + + ``FUNCTIONFS_DMABUF_ATTACH(int)`` +Attach the DMABUF object, identified by its file descriptor, to the +data endpoint. Returns zero on success, and a negative errno value +on error. + + ``FUNCTIONFS_DMABUF_DETACH(int)`` +Detach the given DMABUF object, identified by its file descriptor, +from the data endpoint. Returns zero on success, and a negative +errno value on error. Note that closing the endpoint's file +descriptor will automatically detach all attached DMABUFs. + + ``FUNCTIONFS_DMABUF_TRANSFER(struct usb_ffs_dmabuf_transfer_req *)`` +Enqueue the previously attached DMABUF to the transfer queue. +The argument is a structure that packs the DMABUF's file descriptor, +the size in bytes to transfer (which should generally correspond to +the size of the DMABUF), and a 'flags' field which is unused +for now. Returns zero on success, and a negative errno value on +error. -- 2.43.0
[PATCH v5 5/6] usb: gadget: functionfs: Add DMABUF import interface
This patch introduces three new ioctls. They all should be called on a data endpoint (ie. not ep0). They are: - FUNCTIONFS_DMABUF_ATTACH, which takes the file descriptor of a DMABUF object to attach to the endpoint. - FUNCTIONFS_DMABUF_DETACH, which takes the file descriptor of the DMABUF to detach from the endpoint. Note that closing the endpoint's file descriptor will automatically detach all attached DMABUFs. - FUNCTIONFS_DMABUF_TRANSFER, which requests a data transfer from / to the given DMABUF. Its argument is a structure that packs the DMABUF's file descriptor, the size in bytes to transfer (which should generally be set to the size of the DMABUF), and a 'flags' field which is unused for now. Before this ioctl can be used, the related DMABUF must be attached with FUNCTIONFS_DMABUF_ATTACH. These three ioctls enable the FunctionFS code to transfer data between the USB stack and a DMABUF object, which can be provided by a driver from a completely different subsystem, in a zero-copy fashion. Signed-off-by: Paul Cercueil Acked-by: Daniel Vetter Acked-by: Christian König --- v2: - Make ffs_dma_resv_lock() static - Add MODULE_IMPORT_NS(DMA_BUF); - The attach/detach functions are now performed without locking the eps_lock spinlock. The transfer function starts with the spinlock unlocked, then locks it before allocating and queueing the USB transfer. v3: - Inline to_ffs_dma_fence() which was called only once. - Simplify ffs_dma_resv_lock() - Add comment explaining why we unref twice in ffs_dmabuf_detach() - Document uapi struct usb_ffs_dmabuf_transfer_req and IOCTLs v4: - Protect the DMABUF list with a mutex - Use incremental sequence number for the dma_fences - Unref attachments and DMABUFs in workers - Remove dead code in ffs_dma_resv_lock() - Fix non-block actually blocking - Use dma_fence_begin/end_signalling() - Add comment about cache-management and dma_buf_unmap_attachment() - Make sure dma_buf_map_attachment() is called with the dma-resv locked v5: - Cache the dma_buf_attachment while the DMABUF is attached. - Use dma_buf_begin/end_access() to ensure that the DMABUF data will be coherent to the hardware. - Remove comment about cache-management and dma_buf_unmap_attachment(), since we now use dma_buf_begin/end_access(). - Added Christian's ACK - Select DMA_SHARED_BUFFER in Kconfig entry --- drivers/usb/gadget/Kconfig | 1 + drivers/usb/gadget/function/f_fs.c | 456 include/uapi/linux/usb/functionfs.h | 41 +++ 3 files changed, 498 insertions(+) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index b3592bcb0f96..566ff0b1282a 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -190,6 +190,7 @@ config USB_F_MASS_STORAGE tristate config USB_F_FS + select DMA_SHARED_BUFFER tristate config USB_F_UAC1 diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index ed2a6d5fcef7..82cc449a4d7e 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -15,6 +15,9 @@ /* #define VERBOSE_DEBUG */ #include +#include +#include +#include #include #include #include @@ -43,6 +46,8 @@ #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */ +MODULE_IMPORT_NS(DMA_BUF); + /* Reference counter handling */ static void ffs_data_get(struct ffs_data *ffs); static void ffs_data_put(struct ffs_data *ffs); @@ -124,6 +129,23 @@ struct ffs_ep { u8 num; }; +struct ffs_dmabuf_priv { + struct list_head entry; + struct kref ref; + struct ffs_data *ffs; + struct dma_buf_attachment *attach; + struct sg_table *sgt; + enum dma_data_direction dir; + spinlock_t lock; + u64 context; +}; + +struct ffs_dma_fence { + struct dma_fence base; + struct ffs_dmabuf_priv *priv; + struct work_struct work; +}; + struct ffs_epfile { /* Protects ep->ep and ep->req. */ struct mutexmutex; @@ -197,6 +219,11 @@ struct ffs_epfile { unsigned char isoc; /* P: ffs->eps_lock */ unsigned char _pad; + + /* Protects dmabufs */ + struct mutexdmabufs_mutex; + struct list_headdmabufs; /* P: dmabufs_mutex */ + atomic_tseqno; }; struct ffs_buffer { @@ -1271,10 +1298,51 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to) return res; } +static void ffs_dmabuf_release(struct kref *ref) +{ + struct ffs_dmabuf_priv *priv = container_of(ref, struct ffs_dmabuf_priv, ref); + struct dma_buf_attachment *attach = priv->attach; + struct dma_buf *dmabuf = attach->dmabuf; + + pr_debug("FFS DMABUF release\n"); + dma_resv_lock(dmabuf->resv, NULL); +
[PATCH v5 4/6] usb: gadget: functionfs: Factorize wait-for-endpoint code
This exact same code was duplicated in two different places. Signed-off-by: Paul Cercueil --- drivers/usb/gadget/function/f_fs.c | 48 +- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 6bff6cb93789..ed2a6d5fcef7 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -934,31 +934,44 @@ static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile, return ret; } -static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) +static struct ffs_ep *ffs_epfile_wait_ep(struct file *file) { struct ffs_epfile *epfile = file->private_data; - struct usb_request *req; struct ffs_ep *ep; - char *data = NULL; - ssize_t ret, data_len = -EINVAL; - int halt; - - /* Are we still active? */ - if (WARN_ON(epfile->ffs->state != FFS_ACTIVE)) - return -ENODEV; + int ret; /* Wait for endpoint to be enabled */ ep = epfile->ep; if (!ep) { if (file->f_flags & O_NONBLOCK) - return -EAGAIN; + return ERR_PTR(-EAGAIN); ret = wait_event_interruptible( epfile->ffs->wait, (ep = epfile->ep)); if (ret) - return -EINTR; + return ERR_PTR(-EINTR); } + return ep; +} + +static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) +{ + struct ffs_epfile *epfile = file->private_data; + struct usb_request *req; + struct ffs_ep *ep; + char *data = NULL; + ssize_t ret, data_len = -EINVAL; + int halt; + + /* Are we still active? */ + if (WARN_ON(epfile->ffs->state != FFS_ACTIVE)) + return -ENODEV; + + ep = ffs_epfile_wait_ep(file); + if (IS_ERR(ep)) + return PTR_ERR(ep); + /* Do we halt? */ halt = (!io_data->read == !epfile->in); if (halt && epfile->isoc) @@ -1280,16 +1293,9 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code, return -ENODEV; /* Wait for endpoint to be enabled */ - ep = epfile->ep; - if (!ep) { - if (file->f_flags & O_NONBLOCK) - return -EAGAIN; - - ret = wait_event_interruptible( - epfile->ffs->wait, (ep = epfile->ep)); - if (ret) - return -EINTR; - } + ep = ffs_epfile_wait_ep(file); + if (IS_ERR(ep)) + return PTR_ERR(ep); spin_lock_irq(>ffs->eps_lock); -- 2.43.0
[PATCH v5 3/6] usb: gadget: Support already-mapped DMA SGs
Add a new 'sg_was_mapped' field to the struct usb_request. This field can be used to indicate that the scatterlist associated to the USB transfer has already been mapped into the DMA space, and it does not have to be done internally. Signed-off-by: Paul Cercueil --- drivers/usb/gadget/udc/core.c | 7 ++- include/linux/usb/gadget.h| 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index d59f94464b87..9d4150124fdb 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -903,6 +903,11 @@ int usb_gadget_map_request_by_dev(struct device *dev, if (req->length == 0) return 0; + if (req->sg_was_mapped) { + req->num_mapped_sgs = req->num_sgs; + return 0; + } + if (req->num_sgs) { int mapped; @@ -948,7 +953,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_map_request); void usb_gadget_unmap_request_by_dev(struct device *dev, struct usb_request *req, int is_in) { - if (req->length == 0) + if (req->length == 0 || req->sg_was_mapped) return; if (req->num_mapped_sgs) { diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index a771ccc038ac..c529e4e06997 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -52,6 +52,7 @@ struct usb_ep; * @short_not_ok: When reading data, makes short packets be * treated as errors (queue stops advancing till cleanup). * @dma_mapped: Indicates if request has been mapped to DMA (internal) + * @sg_was_mapped: Set if the scatterlist has been mapped before the request * @complete: Function called when request completes, so this request and * its buffer may be re-used. The function will always be called with * interrupts disabled, and it must not sleep. @@ -111,6 +112,7 @@ struct usb_request { unsignedzero:1; unsignedshort_not_ok:1; unsigneddma_mapped:1; + unsignedsg_was_mapped:1; void(*complete)(struct usb_ep *ep, struct usb_request *req); -- 2.43.0
[PATCH v5 2/6] dma-buf: udmabuf: Implement .{begin,end}_access
Implement .begin_access() and .end_access() callbacks. For now these functions will simply sync/flush the CPU cache when needed. Signed-off-by: Paul Cercueil --- v5: New patch --- drivers/dma-buf/udmabuf.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..a87d89b58816 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -179,6 +179,31 @@ static int end_cpu_udmabuf(struct dma_buf *buf, return 0; } +static int begin_udmabuf(struct dma_buf_attachment *attach, +struct sg_table *sgt, +enum dma_data_direction dir) +{ + struct dma_buf *buf = attach->dmabuf; + struct udmabuf *ubuf = buf->priv; + struct device *dev = ubuf->device->this_device; + + dma_sync_sg_for_device(dev, sgt->sgl, sg_nents(sgt->sgl), dir); + return 0; +} + +static int end_udmabuf(struct dma_buf_attachment *attach, + struct sg_table *sgt, + enum dma_data_direction dir) +{ + struct dma_buf *buf = attach->dmabuf; + struct udmabuf *ubuf = buf->priv; + struct device *dev = ubuf->device->this_device; + + if (dir != DMA_TO_DEVICE) + dma_sync_sg_for_cpu(dev, sgt->sgl, sg_nents(sgt->sgl), dir); + return 0; +} + static const struct dma_buf_ops udmabuf_ops = { .cache_sgt_mapping = true, .map_dma_buf = map_udmabuf, @@ -189,6 +214,8 @@ static const struct dma_buf_ops udmabuf_ops = { .vunmap= vunmap_udmabuf, .begin_cpu_access = begin_cpu_udmabuf, .end_cpu_access= end_cpu_udmabuf, + .begin_access = begin_udmabuf, + .end_access= end_udmabuf, }; #define SEALS_WANTED (F_SEAL_SHRINK) -- 2.43.0
[PATCH v5 1/6] dma-buf: Add dma_buf_{begin,end}_access()
These functions should be used by device drivers when they start and stop accessing the data of DMABUF. It allows DMABUF importers to cache the dma_buf_attachment while ensuring that the data they want to access is available for their device when the DMA transfers take place. Signed-off-by: Paul Cercueil --- v5: New patch --- drivers/dma-buf/dma-buf.c | 66 +++ include/linux/dma-buf.h | 37 ++ 2 files changed, 103 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 8fe5aa67b167..a8bab6c18fcd 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -830,6 +830,8 @@ static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, * - dma_buf_mmap() * - dma_buf_begin_cpu_access() * - dma_buf_end_cpu_access() + * - dma_buf_begin_access() + * - dma_buf_end_access() * - dma_buf_map_attachment_unlocked() * - dma_buf_unmap_attachment_unlocked() * - dma_buf_vmap_unlocked() @@ -1602,6 +1604,70 @@ void dma_buf_vunmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map) } EXPORT_SYMBOL_NS_GPL(dma_buf_vunmap_unlocked, DMA_BUF); +/** + * @dma_buf_begin_access - Call before any hardware access from/to the DMABUF + * @attach:[in]attachment used for hardware access + * @sg_table: [in]scatterlist used for the DMA transfer + * @direction: [in]direction of DMA transfer + */ +int dma_buf_begin_access(struct dma_buf_attachment *attach, +struct sg_table *sgt, enum dma_data_direction dir) +{ + struct dma_buf *dmabuf; + bool cookie; + int ret; + + if (WARN_ON(!attach)) + return -EINVAL; + + dmabuf = attach->dmabuf; + + if (!dmabuf->ops->begin_access) + return 0; + + cookie = dma_fence_begin_signalling(); + ret = dmabuf->ops->begin_access(attach, sgt, dir); + dma_fence_end_signalling(cookie); + + if (WARN_ON_ONCE(ret)) + return ret; + + return 0; +} +EXPORT_SYMBOL_NS_GPL(dma_buf_begin_access, DMA_BUF); + +/** + * @dma_buf_end_access - Call after any hardware access from/to the DMABUF + * @attach:[in]attachment used for hardware access + * @sg_table: [in]scatterlist used for the DMA transfer + * @direction: [in]direction of DMA transfer + */ +int dma_buf_end_access(struct dma_buf_attachment *attach, + struct sg_table *sgt, enum dma_data_direction dir) +{ + struct dma_buf *dmabuf; + bool cookie; + int ret; + + if (WARN_ON(!attach)) + return -EINVAL; + + dmabuf = attach->dmabuf; + + if (!dmabuf->ops->end_access) + return 0; + + cookie = dma_fence_begin_signalling(); + ret = dmabuf->ops->end_access(attach, sgt, dir); + dma_fence_end_signalling(cookie); + + if (WARN_ON_ONCE(ret)) + return ret; + + return 0; +} +EXPORT_SYMBOL_NS_GPL(dma_buf_end_access, DMA_BUF); + #ifdef CONFIG_DEBUG_FS static int dma_buf_debug_show(struct seq_file *s, void *unused) { diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 8ff4add71f88..8ba612c7cc16 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -246,6 +246,38 @@ struct dma_buf_ops { */ int (*end_cpu_access)(struct dma_buf *, enum dma_data_direction); + /** +* @begin_access: +* +* This is called from dma_buf_begin_access() when a device driver +* wants to access the data of the DMABUF. The exporter can use this +* to flush/sync the caches if needed. +* +* This callback is optional. +* +* Returns: +* +* 0 on success or a negative error code on failure. +*/ + int (*begin_access)(struct dma_buf_attachment *, struct sg_table *, + enum dma_data_direction); + + /** +* @end_access: +* +* This is called from dma_buf_end_access() when a device driver is +* done accessing the data of the DMABUF. The exporter can use this +* to flush/sync the caches if needed. +* +* This callback is optional. +* +* Returns: +* +* 0 on success or a negative error code on failure. +*/ + int (*end_access)(struct dma_buf_attachment *, struct sg_table *, + enum dma_data_direction); + /** * @mmap: * @@ -606,6 +638,11 @@ void dma_buf_detach(struct dma_buf *dmabuf, int dma_buf_pin(struct dma_buf_attachment *attach); void dma_buf_unpin(struct dma_buf_attachment *attach); +int dma_buf_begin_access(struct dma_buf_attachment *attach, +struct sg_table *sgt, enum dma_data_direction dir); +int dma_buf_end_access(struct dma_buf_attachment *attach, + struct sg_table *sgt, enum
[PATCH v5 0/6] usb: gadget: functionfs: DMABUF import interface
Hi, This is the v5 of my patchset that adds a new DMABUF import interface to FunctionFS. Daniel / Sima suggested that I should cache the dma_buf_attachment while the DMABUF is attached to the interface, instead of mapping/unmapping the DMABUF for every transfer (also because unmapping is not possible in the dma_fence's critical section). This meant having to add new dma_buf_begin_access() / dma_buf_end_access() functions that the driver can call to ensure cache coherency. These two functions are provided by the new patch [1/6], and an implementation for udmabuf was added in [2/6] - see the changelog below. This patchset was successfully tested with CONFIG_LOCKDEP, no errors were reported in dmesg while using the interface. This interface is being used at Analog Devices, to transfer data from high-speed transceivers to USB in a zero-copy fashion, using also the DMABUF import interface to the IIO subsystem which is being upstreamed in parallel [1]. The two are used by the Libiio software [2]. On a ZCU102 board with a FMComms3 daughter board, using the combination of these two new interfaces yields a drastic improvement of the throughput, from about 127 MiB/s using IIO's buffer read/write interface + read/write to the FunctionFS endpoints, to about 274 MiB/s when passing around DMABUFs, for a lower CPU usage (0.85 load avg. before, vs. 0.65 after). Right now, *technically* there are no users of this interface, as Analog Devices wants to wait until both interfaces are accepted upstream to merge the DMABUF code in Libiio into the main branch, and Jonathan wants to wait and see if this patchset is accepted to greenlight the DMABUF interface in IIO as well. I think this isn't really a problem; once everybody is happy with its part of the cake, we can merge them all at once. This is obviously for 5.9, and based on next-20240119. Changelog: - [1/6]: New patch - [2/6]: New patch - [5/6]: - Cache the dma_buf_attachment while the DMABUF is attached. - Use dma_buf_begin/end_access() to ensure that the DMABUF data will be coherent to the hardware. - Remove comment about cache-management and dma_buf_unmap_attachment(), since we now use dma_buf_begin/end_access(). - Select DMA_SHARED_BUFFER in Kconfig entry - Add Christian's ACK Cheers, -Paul [1] https://lore.kernel.org/linux-iio/219abc43b4fdd4a13b307ed2efaa0e6869e68e3f.ca...@gmail.com/T/ [2] https://github.com/analogdevicesinc/libiio/tree/pcercuei/dev-new-dmabuf-api Paul Cercueil (6): dma-buf: Add dma_buf_{begin,end}_access() dma-buf: udmabuf: Implement .{begin,end}_access usb: gadget: Support already-mapped DMA SGs usb: gadget: functionfs: Factorize wait-for-endpoint code usb: gadget: functionfs: Add DMABUF import interface Documentation: usb: Document FunctionFS DMABUF API Documentation/usb/functionfs.rst| 36 ++ drivers/dma-buf/dma-buf.c | 66 drivers/dma-buf/udmabuf.c | 27 ++ drivers/usb/gadget/Kconfig | 1 + drivers/usb/gadget/function/f_fs.c | 502 ++-- drivers/usb/gadget/udc/core.c | 7 +- include/linux/dma-buf.h | 37 ++ include/linux/usb/gadget.h | 2 + include/uapi/linux/usb/functionfs.h | 41 +++ 9 files changed, 698 insertions(+), 21 deletions(-) -- 2.43.0
Re: [RFC PATCH 1/3] dt-bindings: display: ti,am65x-dss: Add support for display sharing mode
On Thu, Jan 18, 2024 at 7:59 AM Devarsh Thakkar wrote: > > Hi Rob, > > Thanks for the quick review. > > On 18/01/24 01:43, Rob Herring wrote: > > On Tue, Jan 16, 2024 at 07:11:40PM +0530, Devarsh Thakkar wrote: > >> Add support for using TI Keystone DSS hardware present in display > >> sharing mode. > >> > >> TI Keystone DSS hardware supports partitioning of resources between > >> multiple hosts as it provides separate register space and unique > >> interrupt line to each host. > >> > >> The DSS hardware can be used in shared mode in such a way that one or > >> more of video planes can be owned by Linux wherease other planes can be > >> owned by remote cores. > >> > >> One or more of the video ports can be dedicated exclusively to a > >> processing core, wherease some of the video ports can be shared between > >> two hosts too with only one of them having write access. > >> > >> Signed-off-by: Devarsh Thakkar > >> --- > >> .../bindings/display/ti/ti,am65x-dss.yaml | 82 +++ > >> 1 file changed, 82 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml > >> b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml > >> index 55e3e490d0e6..d9bc69fbf1fb 100644 > >> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml > >> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml > >> @@ -112,6 +112,86 @@ properties: > >>Input memory (from main memory to dispc) bandwidth limit in > >>bytes per second > >> > >> + ti,dss-shared-mode: > >> +type: boolean > >> +description: > >> + TI DSS7 supports sharing of display between multiple hosts > >> + as it provides separate register space for display configuration and > >> + unique interrupt line to each host. > > > > If you care about line breaks, you need '|'. > > > > Noted. > > >> + One of the host is provided access to the global display > >> + configuration labelled as "common" region of DSS allows that host > >> + exclusive access to global registers of DSS while other host can > >> + configure the display for it's usage using a separate register > >> + space labelled as "common1". > >> + The DSS resources can be partitioned in such a way that one or more > >> + of the video planes are owned by Linux whereas other video planes > > > > Your h/w can only run Linux? > > > > What if you want to use this same binding to define the configuration to > > the 'remote processor'? You can easily s/Linux/the OS/, but it all > > should be reworded to describe things in terms of the local processor. > > > > It can run both Linux and RTOS or for that matter any other OS too. But yes I > got your point, will reword accordingly. > > >> + can be owned by a remote core. > >> + The video port controlling these planes acts as a shared video port > >> + and it can be configured with write access either by Linux or the > >> + remote core in which case Linux only has read-only access to that > >> + video port. > > > > What is the purpose of this property when all the other properties are > > required? > > > > The ti,dss-shared-mode and below group of properties are optional. But > if ti,dss-shared-mode is set then only driver should parse below set of > properties. Let me re-phrase. Drop this property. It serves no purpose since all the properties have to be present anyway. > >> + ti,dss-shared-mode-planes: > >> +description: > >> + The video layer that is owned by processing core running Linux. > >> + The display driver running from Linux has exclusive write access to > >> + this video layer. > >> +$ref: /schemas/types.yaml#/definitions/string > >> +enum: [vidl, vid] > >> + > >> + ti,dss-shared-mode-vp: > >> +description: > >> + The video port that is being used in context of processing core > >> + running Linux with display susbsytem being used in shared mode. > >> + This can be owned either by the processing core running Linux in > >> + which case Linux has the write access and the responsibility to > >> + configure this video port and the associated overlay manager or > >> + it can be shared between core running Linux and a remote core > >> + with remote core provided with write access to this video port and > >> + associated overlay managers and remote core configures and drives > >> + this video port also feeding data from one or more of the > >> + video planes owned by Linux, with Linux only having read-only access > >> + to this video port and associated overlay managers. > >> + > >> +$ref: /schemas/types.yaml#/definitions/string > >> +enum: [vp1, vp2] > >> + > >> + ti,dss-shared-mode-common: > >> +description: > >> + The DSS register region owned by processing core running Linux. > >> +$ref: /schemas/types.yaml#/definitions/string > >> +enum: [common,
Re: [PATCH v7 1/9] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
Hi Am 19.01.24 um 13:25 schrieb Pekka Paalanen: On Fri, 19 Jan 2024 11:58:38 +0100 Thomas Zimmermann wrote: Hi Am 17.01.24 um 17:40 schrieb Jocelyn Falempe: ... The last thing, is if I plan to add YUV support, with this implementation, I only need to write one function that convert one pixel. Otherwise I would need to add the drm_fb_r1_to_yuvxxx_line() and drm_fb_r1_to_yuv() boilerplate. 8) YUVs are multi-plane formats IIRC. So it's likely a bit more complicated.And I'm not aware of any current use case for YUV. If the framebuffer console doesn't support it, the panic helper probably won't either. Kernel panic during a fullscreen video playback, maybe? That use case is likely to have an YUV FB as the only visible KMS plane FB, either primary or overlay plane depending on which is capable of displaying it. Sub-titles might not exist, or might be in a fairly small RGB overlay. I don't know if such case is important enough to handle. That's at least a possible use case AFAICT. Each conversion is implemented in a helper drm_fb__to_format>(). drm_fb_blit() is just a big switch statement to call the correct helper. Something like drm_r1_to_yuv422() would be no different and would integrate with the existing drm_fb_blit() nicely. So it appears to me as if it's better to either extend the current code for multi-plane formats, or write custom helpers drm_fb_r1_to_yuv(). Best regards Thomas Thanks, pq -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 3/4] usb: gadget: functionfs: Add DMABUF import interface
Hi Paul, kernel test robot noticed the following build errors: [auto build test ERROR on usb/usb-testing] [also build test ERROR on usb/usb-next usb/usb-linus lwn/docs-next linus/master v6.7 next-20240119] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20240117-203111 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing patch link: https://lore.kernel.org/r/20240117122646.41616-4-paul%40crapouillou.net patch subject: [PATCH v4 3/4] usb: gadget: functionfs: Add DMABUF import interface config: arm-randconfig-r112-20240119 (https://download.01.org/0day-ci/archive/20240119/202401192043.6dtnllkn-...@intel.com/config) compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce: (https://download.01.org/0day-ci/archive/20240119/202401192043.6dtnllkn-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202401192043.6dtnllkn-...@intel.com/ All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: dma_buf_get >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: dma_buf_detach >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced 2 more times -- >> ld.lld: error: undefined symbol: dma_fence_init >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: dma_resv_add_fence >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: dma_fence_signal >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_dmabuf_io_complete) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_dmabuf_signal_done) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: dma_fence_release >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(dma_fence_put) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_dmabuf_unmap_work) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: dma_buf_put >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced 2 more times -- >> ld.lld: error: undefined symbol: dma_buf_attach >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: dma_fence_context_alloc >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: dma_resv_test_signaled >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: dma_buf_map_attachment >>> referenced by f_fs.c >>> drivers/usb/gadget/function/f_fs.o:(ffs_epfile_ioctl) in archive vmlinux.a .. -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH v3] drm/msm/dp: return correct Colorimetry for DP_TEST_DYNAMIC_RANGE_CEA case
On Wed, 17 Jan 2024 at 23:13, Kuogee Hsieh wrote: > > MSA MISC0 bit 1 to 7 contains Colorimetry Indicator Field. > dp_link_get_colorimetry_config() returns wrong colorimetry value > in the DP_TEST_DYNAMIC_RANGE_CEA case in the current implementation. > Hence fix this problem by having dp_link_get_colorimetry_config() > return defined CEA RGB colorimetry value in the case of > DP_TEST_DYNAMIC_RANGE_CEA. > > Changes in V2: > -- drop retrieving colorimetry from colorspace > -- drop dr = link->dp_link.test_video.test_dyn_range assignment > > Changes in V3: > -- move defined MISCr0a Colorimetry vale to dp_reg.h > -- rewording commit title > -- rewording commit text to more precise describe this patch > > Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") > Signed-off-by: Kuogee Hsieh > --- > drivers/gpu/drm/msm/dp/dp_link.c | 12 +++- > drivers/gpu/drm/msm/dp/dp_reg.h | 3 +++ > 2 files changed, 10 insertions(+), 5 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 3/4] drm: xlnx: zynqmp_dpsub: Don't generate vblank in live mode
Hi Anatoliy, On Fri, Jan 19, 2024 at 05:53:30AM +, Klymenko, Anatoliy wrote: > On Wed, 17 Jan 2024 16:20:10 +0200, Tomi Valkeinen wrote: > > On 13/01/2024 01:42, Anatoliy Klymenko wrote: > > > Filter out status register against interrupts' mask. > > > Some events are being reported via DP status register, even if > > > corresponding interrupts have been disabled. Avoid processing of such > > > events in interrupt handler context. > > > > The subject talks about vblank and live mode, the the description doesn't. > > Can > > you elaborate in the desc a bit about when this is an issue and why it > > wasn't > > before? > > Yes, I should make patch comment more consistent and elaborate. I will fix it > in the next version. Thank you. > > > > > > Signed-off-by: Anatoliy Klymenko > > > --- > > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 11 +-- > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > index d60b7431603f..571c5dbc97e5 100644 > > > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > @@ -1624,8 +1624,16 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, > > > void *data) > > > u32 status, mask; > > > > > > status = zynqmp_dp_read(dp, ZYNQMP_DP_INT_STATUS); > > > + zynqmp_dp_write(dp, ZYNQMP_DP_INT_STATUS, status); > > > mask = zynqmp_dp_read(dp, ZYNQMP_DP_INT_MASK); > > > - if (!(status & ~mask)) > > > + > > > + /* > > > + * Status register may report some events, which corresponding > > > interrupts > > > + * have been disabled. Filter out those events against interrupts' > > > mask. > > > + */ > > > + status &= ~mask; > > > + > > > + if (!status) > > > return IRQ_NONE; > > > > > > /* dbg for diagnostic, but not much that the driver can do */ > > > @@ -1634,7 +1642,6 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, > > > void *data) > > > if (status & ZYNQMP_DP_INT_CHBUF_OVERFLW_MASK) > > > dev_dbg_ratelimited(dp->dev, "overflow interrupt\n"); > > > > > > - zynqmp_dp_write(dp, ZYNQMP_DP_INT_STATUS, status); > > > > > > if (status & ZYNQMP_DP_INT_VBLANK_START) > > > zynqmp_dpsub_drm_handle_vblank(dp->dpsub); > > > > Moving the zynqmp_dp_write() is not related to this fix, is it? I think it > > should be in > > a separate patch. > > The sole purpose of zynqmp_dp_write() is to clear status register. The > sooner we do it the better (after reading status in the local variable > of course). No disagreement about that. Tomi's point is that it's a good change, but it should be done in a separate patch, by itself, not bundled with other changes. The usual rule in the kernel is "one change, one commit". > We can also reuse status variable for in-place filtering > against the interrupt mask, which makes this change dependent on > zynqmp_dp_write() reordering. I will add a comment explaining this. Is > it ok? -- Regards, Laurent Pinchart
Re: [PATCH v7 1/9] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
On Fri, 19 Jan 2024 11:58:38 +0100 Thomas Zimmermann wrote: > Hi > > Am 17.01.24 um 17:40 schrieb Jocelyn Falempe: ... > > The last thing, is if I plan to add YUV support, with this > > implementation, I only need to write one function that convert one > > pixel. Otherwise I would need to add the drm_fb_r1_to_yuvxxx_line() and > > drm_fb_r1_to_yuv() boilerplate. > > 8) YUVs are multi-plane formats IIRC. So it's likely a bit more > complicated.And I'm not aware of any current use case for YUV. If the > framebuffer console doesn't support it, the panic helper probably won't > either. Kernel panic during a fullscreen video playback, maybe? That use case is likely to have an YUV FB as the only visible KMS plane FB, either primary or overlay plane depending on which is capable of displaying it. Sub-titles might not exist, or might be in a fairly small RGB overlay. I don't know if such case is important enough to handle. Thanks, pq pgpRP7oNEz8LR.pgp Description: OpenPGP digital signature
[PULL] drm-misc-next-fixes
Hi, Here's this week (and last (maybe)) drm-misc-next-fixes PR. Thanks! Maxime drm-misc-next-fixes-2024-01-19: A null pointer dereference fix for v3d and a protection fault fix for ttm. The following changes since commit 89fe46019a62bc1d0cb49c9615cb3520096c4bc1: drm/v3d: Fix support for register debugging on the RPi 4 (2024-01-09 14:21:47 -0300) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2024-01-19 for you to fetch changes up to 1f1626ac0428820f998245478610f452650bcab5: drm/ttm: fix ttm pool initialization for no-dma-device drivers (2024-01-15 13:56:08 +0100) A null pointer dereference fix for v3d and a protection fault fix for ttm. Fedor Pchelkin (1): drm/ttm: fix ttm pool initialization for no-dma-device drivers Maíra Canal (1): drm/v3d: Free the job and assign it to NULL if initialization fails drivers/gpu/drm/ttm/ttm_device.c | 9 +++-- drivers/gpu/drm/v3d/v3d_submit.c | 35 --- 2 files changed, 35 insertions(+), 9 deletions(-) signature.asc Description: PGP signature
Re: [PATCH v3 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format
On Fri, Jan 19, 2024 at 03:32:49AM +, dharm...@microchip.com wrote: > On 18/01/24 9:10 pm, Conor Dooley wrote: > > On Thu, Jan 18, 2024 at 02:56:12PM +0530, Dharma Balasubiramani wrote: > >> Convert the atmel,hlcdc binding to DT schema format. > >> > >> Adjust the clock-names property to clarify that the LCD controller expects > >> one of these clocks (either sys_clk or lvds_pll_clk to be present but not > >> both) along with the slow_clk and periph_clk. This alignment with the > >> actual > >> hardware requirements will enable accurate device tree configuration for > >> systems using the HLCDC IP. > >> > >> Signed-off-by: Dharma Balasubiramani > >> --- > >> changelog > >> v2 -> v3 > >> - Rename hlcdc-display-controller and hlcdc-pwm to generic names. > >> - Modify the description by removing the unwanted comments and '|'. > >> - Modify clock-names simpler. > >> v1 -> v2 > >> - Remove the explicit copyrights. > >> - Modify title (not include words like binding/driver). > >> - Modify description actually describing the hardware and not the driver. > >> - Add details of lvds_pll addition in commit message. > >> - Ref endpoint and not endpoint-base. > >> - Fix coding style. > >> ... > >> .../devicetree/bindings/mfd/atmel,hlcdc.yaml | 97 +++ > >> .../devicetree/bindings/mfd/atmel-hlcdc.txt | 56 --- > >> 2 files changed, 97 insertions(+), 56 deletions(-) > >> create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml > >> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt > >> > >> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml > >> b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml > >> new file mode 100644 > >> index ..eccc998ac42c > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml > >> @@ -0,0 +1,97 @@ > >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id:http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml# > >> +$schema:http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: Atmel's HLCD Controller > >> + > >> +maintainers: > >> + - Nicolas Ferre > >> + - Alexandre Belloni > >> + - Claudiu Beznea > >> + > >> +description: > >> + The Atmel HLCDC (HLCD Controller) IP available on Atmel SoCs exposes two > >> + subdevices, a PWM chip and a Display Controller. > >> + > >> +properties: > >> + compatible: > >> +enum: > >> + - atmel,at91sam9n12-hlcdc > >> + - atmel,at91sam9x5-hlcdc > >> + - atmel,sama5d2-hlcdc > >> + - atmel,sama5d3-hlcdc > >> + - atmel,sama5d4-hlcdc > >> + - microchip,sam9x60-hlcdc > >> + - microchip,sam9x75-xlcdc > >> + > >> + reg: > >> +maxItems: 1 > >> + > >> + interrupts: > >> +maxItems: 1 > >> + > >> + clocks: > >> +maxItems: 3 > > Hmm, one thing I probably should have said on the previous version, but > > I missed somehow: It would be good to add an items list to the clocks > > property here to explain what the 3 clocks are/are used for - especially > > since there is additional complexity being added here to use either the > > sys or lvds clocks. > May I inquire if this approach is likely to be effective? > >clocks: > items: >- description: peripheral clock >- description: generic clock or lvds pll clock >Once the LVDS PLL is enabled, the pixel clock is used as the >clock for LCDC, so its GCLK is no longer needed. >- description: slow clock > maxItems: 3 Hmm that sounds very suspect to me. "Once the lvdspll is enabled the generic clock is no longer needed" sounds like both clocks can be provided to the IP on different pins and their provision is not mutually exclusive, just that the IP will only actually use one at a time. If that is the case, then this patch is nott correct and the binding should allow for 4 clocks, with both the generic clock and the lvds pll being present in the DT at the same time. I vaguely recall internal discussion about this problem some time back but the details all escape me. Thanks, Conor. signature.asc Description: PGP signature
Re: [PATCH v7 1/9] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
Hi Am 17.01.24 um 17:40 schrieb Jocelyn Falempe: On 17/01/2024 16:06, Thomas Zimmermann wrote: Hi Am 04.01.24 um 17:00 schrieb Jocelyn Falempe: This is needed for drm_panic, to draw the font, and fill the background color, in multiple color format. TBH, I don't like this patch at all. It looks like you're reimplementing existing functionality for a single use case; specifically drm_fb_blit(). What's wrong with the existing interfaces? I've spend considerable time to clean up the format-helper code and get it into shape. It's not there yet, but on its way. So I'd rather prefer to update the existing code for new use cases. Adding a new interface for a single use case is something like a leap backwards. So let's see if we can work out something. drm_fb_blit() is good to copy a framebuffer to another, but is clearly unoptimal to draw font. 1) The framebuffer data structure is only there for historical reasons. It should be removed from the internal implementation entirely. A first patch should go into this in any case. It didn't happened so far, as I've been busy with other work. 2) For the public API, I've long wanted to replace framebuffers with something more flexible, let's call it drm_pixmap struct drm_pixmap { struct drm_format_info *format unsigned int width, height unsigned int pitches[DRM_FORMAT_MAX_PLANES] iomap vaddr[DRM_FORMAT_MAX_PLANES] }; It's the essence of drm_framebuffer. Let's say there's also an init helper drm_pixmap_init_from_framebuffer(pix, fb) that sets up everything. The implementation of drm_fb_blit() would look like this: drm_fb_blit(...) { drm_pixmap pix; drm_pixmap_init_from_framebuffer(pix, fb) __drm_fb_blit_pixmap( ) } That would require some changes to drivers, but it's only simple refactoring. 3) When looking at your patch, there's src = font->data + (msg->txt[i] * font->height) * src_stride; which should be in a helper that sets up the drm_pixmap for a font character: drm_pixmap_init_from_char(pixmap, c, font_data) where 'c' equals msg->txt[i] The text drawing in the panic handler would do something like for (msg->txt[i]) { drm_pixmap_init_from_char(pixmap, ...) drm_fb_blit_pixmap(...) } It handles xrgb to any rgb format, and I need monochrome to any rgb format. 4) You're free to add any conversion to drm_fb_blit(). It's supposed to handle all available format conversion. With the pixmap-related changes outlined above and the actual conversion code, I think that would already put characters on the screen. I need to convert foreground and background color to the destination format, but using drm_fb_blit() to convert 1 pixel is tedious. 5) I've recently added drm_format_conv_state to the API. It is supposed to hold state that is required for the conversion process. I specifically had color palettes in mind. Please use the data structure. Something like that: struct drm_format_conv_state { ... const drm_color_lut *palette; } and in the conversion code: void r1_to_rgb() { for (x < pixels) { rgb = state->palette[r1] } } It also requires an additional memory buffer, and do an additional memory copy that we don't need at all. 6) That memcpy_to_io() not a big deal. You should pre-allocate that memory buffer in the panic handler and init the drm_format_conv_state with DRM_FORMAT_CONV_STATE_INIT_PREALLOCATED(). It also has no way to fill a region with the background color. 7) Please add a separate drm_fb_fill() implementation. If you have a palette in struct drm_format_conf_state, you can add a helper for each destination format that takes a drm_color_lut value as input. This point is probably worth a separate discussion. The last thing, is if I plan to add YUV support, with this implementation, I only need to write one function that convert one pixel. Otherwise I would need to add the drm_fb_r1_to_yuvxxx_line() and drm_fb_r1_to_yuv() boilerplate. 8) YUVs are multi-plane formats IIRC. So it's likely a bit more complicated.And I'm not aware of any current use case for YUV. If the framebuffer console doesn't support it, the panic helper probably won't either. Best regards Thomas Best regards, -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 1/3] dt-bindings: mailbox: Add mediatek,gce-props.yaml
Il 19/01/24 07:32, Jason-JH.Lin ha scritto: Add mediatek,gce-props.yaml for common GCE properties that is used for both mailbox providers and consumers. We place the common property "mediatek,gce-events" in this binding currently. The property "mediatek,gce-events" is used for GCE event ID corresponding to a hardware event signal sent by the hardware or a sofware driver. If the mailbox providers or consumers want to manipulate the value of the event ID, they need to know the specific event ID. Signed-off-by: Jason-JH.Lin --- .../bindings/mailbox/mediatek,gce-props.yaml | 52 +++ 1 file changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml new file mode 100644 index ..68b519ff089f --- /dev/null +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml @@ -0,0 +1,52 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mailbox/mediatek,gce-props.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek Global Command Engine Common Propertes + +maintainers: + - Houlong Wei + +description: + The Global Command Engine (GCE) is an instruction based, multi-threaded, + single-core command dispatcher for MediaTek hardware. The Command Queue + (CMDQ) mailbox driver is a driver for GCE, implemented using the Linux + mailbox framework. It is used to receive messages from mailbox consumers + and configure GCE to execute the specified instruction set in the message. + We use mediatek,gce-mailbox.yaml to define the properties for CMDQ mailbox + driver. A device driver that uses the CMDQ driver to configure its hardware + registers is a mailbox consumer. The mailbox consumer can request a mailbox + channel corresponding to a GCE hardware thread to send a message, specifying + that the GCE thread to configure its hardware. The mailbox provider can also + reserved a mailbox channel to configure GCE hardware register by the spcific + GCE thread. This binding defines the common GCE properties for both mailbox + provider and consumers. + +properties: + mediatek,gce-events: +description: + GCE has an event table in SRAM, consisting of 1024 event IDs (0~1023). + Each event ID has a boolean event value with the default value 0. + The property mediatek,gce-events is used to obtain the event IDs. + Some gce-events are hardware-bound and cannot be changed by software. + For instance, in MT8195, when VDO0_MUTEX is stream done, VDO_MUTEX will + send an event signal to GCE, setting the value of event ID 597 to 1. + Similarly, in MT8188, the value of event ID 574 will be set to 1 when + VOD0_MUTEX is stream done. + On the other hand, some gce-events are not hardware-bound and can be + changed by software. For example, in MT8188, we can set the value of + event ID 855, which is not bound to any hardware, to 1 when the driver + in the secure world completes a task. However, in MT8195, event ID 855 + is already bound to VDEC_LAT1, so we need to select another event ID to + achieve the same purpose. This event ID can be any ID that is not bound + to any hardware and is not yet used in any software driver. + To determine if the event ID is bound to the hardware or used by a + software driver, refer to the GCE header + include/dt-bindings/gce/-gce.h of each chip. +$ref: /schemas/types.yaml#/definitions/uint32-array +minItems: 1 +maxItems: 1024 maxItems: 1024 seems to be a bit too many... this means that one devicetree node may have up to 1024 gce events, which is impossible! If a driver needed all of the 1024 events, this means that it's not an user of the GCE, but the GCE itself! Imagine seeing a devicetree node with 1024 array entries for mediatek,gce-events... I'd set that to a more sensible value of 32 - eventually we can extend it later, if ever needed. Besides, nice job about all this documentation of the GCE and its events: love it! Cheers, Angelo
Re: Implement per-key keyboard backlight as auxdisplay?
On Fri, 19 Jan 2024, Hans de Goede wrote: > For per key controllable rgb LEDs we need to discuss a coordinate > system. I propose using a fixed size of 16 rows of 64 keys, > so 64x16 in standard WxH notation. > > And then storing RGB in separate bytes, so userspace will then > always send a buffer of 192 bytes per line (64x3) x 14 rows > = 3072 bytes. With the kernel driver ignoring parts of > the buffer where there are no actual keys. > > I would then like the map the standard 105 key layout onto this, > starting at x.y (column.row) coordinates of 16.6 (with 0.0 being > the top left). Leaving plenty of space on the left top and right > (and some on the bottom) for extra media key rows, macro keys, etc. > > The idea to have the standard layout at a fixed place is to allow > userspace to have a database of preset patterns which will work > everywhere. > > Note I say standard 105 key layout, but in reality for > defining the standardized part of the buffer we should > use the maximum amount of keys per row of all the standard layouts, > so for row 6 (the ESC row) and for extra keys on the right outside > the main block we use the standard layout as shown here: Doesn't the input stack already have to have pretty much all of this already covered? I can view the keyboard layout in my desktop environment, and it's a reasonably accurate match, even if unlikely to be pixel perfect. But crucially, it has to have all the possible layouts covered already. And while I would personally hate it, you can imagine a use case where you'd like a keypress to have a visual effect around the key you pressed. A kind of force feedback, if you will. I don't actually know, and correct me if I'm wrong, but feels like implementing that outside of the input subsystem would be non-trivial. Cc: Dmitry, could we at least have some input from the input subsystem POV on this? AFAICT we have received none. BR, Jani. > > http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg > > For the main area of the keyboard looking at: > > http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png > > We want to max rows per key, so this means that per row we use > (from the above image) : > > row 7: 106/109 - JIS > row 8: 101/104 - ANSI > row 9: 102/105 - ISO > row 10: 104/107 - ABNT > row 11: 106/109 - JIS > > (with row 7 being the main area top row) > > This way we can address all the possible keys in the various > standard layouts in one standard wat and then the drivers can > just skip keys which are not there when preparing the buffer > to send to the hw / fw. > > One open question is if we should add padding after the main > area so that the printscreen / ins / del / leftarrow of the > "middle" block of > > http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg > > all start at the same x (say 32) or we just pack these directly > after the main area. > > And the same question for the numlock block, do we align > this to an x of say 36, or pack it ? > > > As for the actual IOCTL API I think there should be > the following ioctls: > > 1. A get-info ioctl returning a struct with the following members: > > { > char name[64] /* Keyboard model name / identifier */ > int row_begin[16]; /* The x address of the first available key per row. On a > std 105key kbd this will be 16 for rows 6-11, 0 for other rows */ > int row_end[16]; /* x+1 for the address of the last available key per row, > end - begin gives number of keys in a row */ > int rgb_zones; /* number of rgb zones for zoned keyboards. Note both > zones and per key addressing may be available if > effects are applied per zone. */ > ? > } > > 2. A set-leds ioctl which takes the earlier discussed 3092 bytes buffer > to set all the LEDs at once, only valid if at least one row has a non 0 > lenght. > > 3. A set-zones ioctl which takes an array of bytes sized 3 * number-of-zones > containing RGB values for each zone > > 4. A enum_effects ioctl which takes a struct with the following members: > > { > long size; /* Size of passed in struct including the size member itself */ > long effects_mask[] > } > > the idea being that there is an enum with effects, which gets extended > as we encounter more effects and the bitmask in effects_mask has a bit set > for each effects enum value which is supported. effects_mask is an array > so that we don't run out of bits. If older userspace only passes 1 long > (size == (2*sizeof(long)) when 2 are needed at some point in the future > then the kernel will simply only fill the first long. > > 5. A set_effect ioctl which takes a struct with the following members: > > { > long size; /* Size of passed in struct including the size member itself */ > int effect_nr; /* enum value of the effect to enable, 0 for disable effect */ > int zone; /* zone to apply the effect to */ > int speed; /* cycle speed of the effect in milli-hz */ > char color1[3]; /* effect dependend may be
[PATCH] drm/loongson: Error out if no VRAM detected
If there is no VRAM (it is true if there is a discreted card), we get such an error and Xorg fails to start: [ 136.401131] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 137.444342] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 138.871166] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 140.444078] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 142.403993] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 143.970625] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed [ 145.862013] loongson :00:06.1: [drm] *ERROR* Requesting(0MiB) failed So in lsdc_get_dedicated_vram() we error out if no VRAM (or VRAM is less than 1MB which is also an unusable case) detected. Signed-off-by: Huacai Chen --- drivers/gpu/drm/loongson/lsdc_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c b/drivers/gpu/drm/loongson/lsdc_drv.c index 89ccc0c43169..d8ff60b46abe 100644 --- a/drivers/gpu/drm/loongson/lsdc_drv.c +++ b/drivers/gpu/drm/loongson/lsdc_drv.c @@ -184,7 +184,7 @@ static int lsdc_get_dedicated_vram(struct lsdc_device *ldev, drm_info(ddev, "Dedicated vram start: 0x%llx, size: %uMiB\n", (u64)base, (u32)(size >> 20)); - return 0; + return (size > SZ_1M) ? 0 : -ENODEV; } static struct lsdc_device * -- 2.39.3
Re: [PATCH] drm/bridge: tc358767: Limit the Pixel PLL input range
Am Donnerstag, dem 18.01.2024 um 23:02 +0100 schrieb Marek Vasut: > According to new configuration spreadsheet from Toshiba for TC9595, > the Pixel PLL input clock have to be in range 6..40 MHz. The sheet > calculates those PLL input clock as reference clock divided by both > pre-dividers. Add the extra limit. > > Signed-off-by: Marek Vasut Reviewed-by: Lucas Stach > --- > Cc: Andrzej Hajda > Cc: Daniel Vetter > Cc: David Airlie > Cc: Jernej Skrabec > Cc: Jonas Karlman > Cc: Laurent Pinchart > Cc: Lucas Stach > Cc: Neil Armstrong > Cc: Robert Foss > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/bridge/tc358767.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index 615cc8f950d7b..0c29a8f81cc9e 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -546,9 +546,14 @@ static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, > u32 pixelclock) > continue; > for (i_post = 0; i_post < ARRAY_SIZE(ext_div); i_post++) { > for (div = 1; div <= 16; div++) { > - u32 clk; > + u32 clk, iclk; > u64 tmp; > > + /* PCLK PLL input unit clock ... 6..40 MHz */ > + iclk = refclk / (div * ext_div[i_pre]); > + if (iclk < 600 || iclk > 4000) > + continue; > + > tmp = pixelclock * ext_div[i_pre] * > ext_div[i_post] * div; > do_div(tmp, refclk);
[PATCH] drm/display: fix typo
While studying the code I've bumped into a small typo within the kernel-doc for two functions, apparently, due to copy-paste. This commit fixes "sizo" word to be "size". Signed-off-by: Oleksandr Natalenko --- drivers/gpu/drm/display/drm_dp_dual_mode_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c index bd61e20770a5b..14a2a8473682b 100644 --- a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c +++ b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c @@ -52,7 +52,7 @@ * @adapter: I2C adapter for the DDC bus * @offset: register offset * @buffer: buffer for return data - * @size: sizo of the buffer + * @size: size of the buffer * * Reads @size bytes from the DP dual mode adaptor registers * starting at @offset. @@ -116,7 +116,7 @@ EXPORT_SYMBOL(drm_dp_dual_mode_read); * @adapter: I2C adapter for the DDC bus * @offset: register offset * @buffer: buffer for write data - * @size: sizo of the buffer + * @size: size of the buffer * * Writes @size bytes to the DP dual mode adaptor registers * starting at @offset. -- 2.43.0
Re: [PATCH v2 2/2] backlight: Add Kinetic KTD2801 driver
On Thu, Jan 18, 2024 at 06:32:39PM +0100, Duje Mihanović wrote: > Add driver for the Kinetic KTD2801 backlight driver. > > Signed-off-by: Duje Mihanović > > --- > Shared ExpressWire handling code and preemption watchdogs haven't been > implemented in this version as my questions regarding these two weren't > answered. > --- The last mail I saw on this topic was of the "do you have any better ideas?" variety. I (mis)read that as "unless you have any better ideas" and didn't realize you were waiting for anything. I didn't have any better ideas! Daniel.
Re: [PATCH v7 2/9] drm/panic: Add a drm panic handler
Hi Am 18.01.24 um 11:17 schrieb Jocelyn Falempe: On 17/01/2024 16:49, Thomas Zimmermann wrote: Hi Am 04.01.24 um 17:00 schrieb Jocelyn Falempe: [...] + /** + * @get_scanout_buffer: + * + * Get the current scanout buffer, to display a panic message with drm_panic. + * The driver should do the minimum changes to provide a linear buffer, that + * can be used to display the panic screen. + * It is called from a panic callback, and must follow its restrictions. + * (no locks, no memory allocation, no sleep, no thread/workqueue, ...) + * It's a best effort mode, so it's expected that in some complex cases the + * panic screen won't be displayed. + * Some hardware cannot provide a linear buffer, so there is a draw_pixel_xy() + * callback in the struct drm_scanout_buffer that can be used in this case. + * + * Returns: + * + * Zero on success, negative errno on failure. + */ + int (*get_scanout_buffer)(struct drm_device *dev, + struct drm_scanout_buffer *sb); + After reading through Sima's comments on (try-)locking, I'd like to propose a different interface: instead of having the panic handler search for the scanout buffer, let each driver explicitly set the scanout buffer after each page flip. The algorithm for mode programming then looks like this: 1) Maybe clear the panic handler's buffer at the beginning of atomic_commit_tail, if necessary 2) Do the mode setting as usual 3) In the driver's atomic_flush or atomic_update, call something like void drm_panic_set_scanout_buffer(dev, scanout_buffer) to set the panic handler's new output. This avoids all the locking and the second guessing about the pipeline status. I don't see an easy way of reliably showing a panic screen during a modeset. But during a modeset, the old scanout buffer should (theoretically) not disappear until the new scanout buffer is in place. So if the panic happens, it would blit to the old address at worst. Well, that assumption needs to be verified per driver. That's an interesting approach, and I will give it a try. I think you still need a callback in the driver, to actually send the data to the GPU. Sure, you could add this callback directly to the scanout buffer. Also one thing that I don't handle yet, is when there are multiple outputs, so we may want to set and update multiple scanout buffers ? That makes me think about adding panic handling directly to the plane, something like this drm_plane_enable_panic_output(struct drm_plane*) drm_plane_set_panic_output_buffer(struct drm_plane*, struct drm_scanout_buffer*) First time the driver calls drm_plane_enable_panic_output(), it sets up the panic handling for the given plane and maybe for DRM as a whole. Each instance of the DRM panic handler operates on a single plane. Then during the pageflips, drm_plane_set_panic_output_buffer() updates the output buffer. The necessary sync/flush callbacks can be part of the drm_plane_funcs. If/when the plane gets removed from the modesetting pipeline, the panic handling would be removed automatically. Best regards Thomas Best regards, -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t
Am 19.01.24 um 10:05 schrieb Thomas Hellström: The relatively recently introduced drm/exec utility was using uint32_t in its interface, which was then also carried over to drm/gpuvm. Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly. Cc: Christian König Cc: Danilo Krummrich Signed-off-by: Thomas Hellström Reviewed-by: Christian König --- drivers/gpu/drm/drm_exec.c | 2 +- include/drm/drm_exec.h | 4 ++-- include/drm/drm_gpuvm.h| 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c index 5d2809de4517..20e59d88218d 100644 --- a/drivers/gpu/drm/drm_exec.c +++ b/drivers/gpu/drm/drm_exec.c @@ -72,7 +72,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec) * * Initialize the object and make sure that we can track locked objects. */ -void drm_exec_init(struct drm_exec *exec, uint32_t flags) +void drm_exec_init(struct drm_exec *exec, u32 flags) { exec->flags = flags; exec->objects = kmalloc(PAGE_SIZE, GFP_KERNEL); diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h index b5bf0b6da791..187c3ec44606 100644 --- a/include/drm/drm_exec.h +++ b/include/drm/drm_exec.h @@ -18,7 +18,7 @@ struct drm_exec { /** * @flags: Flags to control locking behavior */ - uint32_tflags; + u32 flags; /** * @ticket: WW ticket used for acquiring locks @@ -135,7 +135,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec) return !!exec->contended; } -void drm_exec_init(struct drm_exec *exec, uint32_t flags); +void drm_exec_init(struct drm_exec *exec, u32 flags); void drm_exec_fini(struct drm_exec *exec); bool drm_exec_cleanup(struct drm_exec *exec); int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj); diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h index 48311e6d664c..554046321d24 100644 --- a/include/drm/drm_gpuvm.h +++ b/include/drm/drm_gpuvm.h @@ -514,7 +514,7 @@ struct drm_gpuvm_exec { /** * @flags: the flags for the struct drm_exec */ - uint32_t flags; + u32 flags; /** * @vm: the _gpuvm to lock its DMA reservations
Re: [PATCH] drm/vmwgfx: Filter modes which exceed 3/4 of graphics memory.
Hi Am 18.01.24 um 19:25 schrieb Zack Rusin: On Mon, Jan 15, 2024 at 3:21 AM Thomas Zimmermann wrote: Hi Am 12.01.24 um 21:38 schrieb Ian Forbes: SVGA requires surfaces to fit within graphics memory (max_mob_pages) which means that modes with a final buffer size that would exceed graphics memory must be pruned otherwise creation will fail. Additionally, device commands which use multiple graphics resources must have all their resources fit within graphics memory for the duration of the command. Thus we need a small carve out of 1/4 of graphics memory to ensure commands likes surface copies to the primary framebuffer for cursor composition or damage clips can fit within graphics memory. This fixes an issue where VMs with low graphics memory (< 64MiB) configured with high resolution mode boot to a black screen because surface creation fails. That is a long-standing problem, which we have observed with other drivers as well. On low-memory devices, TTM doesn't play well. The real fix would be to export all modes that possibly fit and sort out the invalid configurations in atomic_check. It's just a lot more work. Did you consider simply ignoring vmwgfx devices with less than 64 MiB of VRAM? Unfortunately we can't do that because on new esx servers without gpu's the default is 16MB. A lot of people are still running their esx boxes with 4MB, which is in general the most common problem because with 4MB people still tend to like to set 1280x800 which with 32bpp fb takes 4096000 bytes and with 4MB available that leaves only 96KB available and we need more to also allocate things like the cursor. Even if ttm did everything right technically 1280x800 @ 32bpp resolution will fit in a 4MB graphics memory, but then the system will not be able to have a hardware (well, virtualized) cursor. It's extremely unlikely people would even be aware of this tradeoff when making the decision to increase resolution. Do you allocate buffer storage directly in the provided VRAM? If so how do you do page flips then? You'd need for the example of 1280x800-32, you'd need around 8 MiB to keep front and back buffer in VRAM. I guess, you only support the framebuffer console (which doesn't do pageflips)? In mgag200 and ast, I had the luxury for replacing TTM with SHMEM helpers, which worked around the problem easily. Maybe that's an option for low-memory systems? Best regards Thomas So the driver either needs to preallocate all the memory it possibly might need for all the basic functionality before modesetting or the available modes need to be validated with some constraints. z -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
[PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t
The relatively recently introduced drm/exec utility was using uint32_t in its interface, which was then also carried over to drm/gpuvm. Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly. Cc: Christian König Cc: Danilo Krummrich Signed-off-by: Thomas Hellström --- drivers/gpu/drm/drm_exec.c | 2 +- include/drm/drm_exec.h | 4 ++-- include/drm/drm_gpuvm.h| 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c index 5d2809de4517..20e59d88218d 100644 --- a/drivers/gpu/drm/drm_exec.c +++ b/drivers/gpu/drm/drm_exec.c @@ -72,7 +72,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec) * * Initialize the object and make sure that we can track locked objects. */ -void drm_exec_init(struct drm_exec *exec, uint32_t flags) +void drm_exec_init(struct drm_exec *exec, u32 flags) { exec->flags = flags; exec->objects = kmalloc(PAGE_SIZE, GFP_KERNEL); diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h index b5bf0b6da791..187c3ec44606 100644 --- a/include/drm/drm_exec.h +++ b/include/drm/drm_exec.h @@ -18,7 +18,7 @@ struct drm_exec { /** * @flags: Flags to control locking behavior */ - uint32_tflags; + u32 flags; /** * @ticket: WW ticket used for acquiring locks @@ -135,7 +135,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec) return !!exec->contended; } -void drm_exec_init(struct drm_exec *exec, uint32_t flags); +void drm_exec_init(struct drm_exec *exec, u32 flags); void drm_exec_fini(struct drm_exec *exec); bool drm_exec_cleanup(struct drm_exec *exec); int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj); diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h index 48311e6d664c..554046321d24 100644 --- a/include/drm/drm_gpuvm.h +++ b/include/drm/drm_gpuvm.h @@ -514,7 +514,7 @@ struct drm_gpuvm_exec { /** * @flags: the flags for the struct drm_exec */ - uint32_t flags; + u32 flags; /** * @vm: the _gpuvm to lock its DMA reservations -- 2.43.0
Re: [PATCH v2 2/2] backlight: Add Kinetic KTD2801 driver
Hi Duje, thanks for your patch! On Thu, Jan 18, 2024 at 6:33 PM Duje Mihanović wrote: > Add driver for the Kinetic KTD2801 backlight driver.> > Signed-off-by: Duje Mihanović Add some commit message? > +#include > +#include > +#include > +#include I don't think you need , the compatible table works without that (is in the device driver core). > +/* These values have been extracted from Samsung's driver. */ > +#define KTD2801_EXPRESSWIRE_DETECT_DELAY_US150 > +#define KTD2801_EXPRESSWIRE_DETECT_US 270 > +#define KTD2801_LOW_BIT_HIGH_TIME_US 5 > +#define KTD2801_LOW_BIT_LOW_TIME_US(4 * > KTD2801_HIGH_BIT_LOW_TIME_US) > +#define KTD2801_HIGH_BIT_LOW_TIME_US 5 > +#define KTD2801_HIGH_BIT_HIGH_TIME_US (4 * > KTD2801_HIGH_BIT_LOW_TIME_US) > +#define KTD2801_DATA_START_US 5 > +#define KTD2801_END_OF_DATA_LOW_US 10 > +#define KTD2801_END_OF_DATA_HIGH_US350 > +#define KTD2801_PWR_DOWN_DELAY_US 2600 > + > +#define KTD2801_DEFAULT_BRIGHTNESS 100 > +#define KTD2801_MAX_BRIGHTNESS 255 > + > +struct ktd2801_backlight { > + struct backlight_device *bd; > + struct gpio_desc *gpiod; > + bool was_on; > +}; > + > +static int ktd2801_update_status(struct backlight_device *bd) > +{ > + struct ktd2801_backlight *ktd2801 = bl_get_data(bd); > + u8 brightness = (u8) backlight_get_brightness(bd); > + > + if (backlight_is_blank(bd)) { > + gpiod_set_value(ktd2801->gpiod, 0); > + udelay(KTD2801_PWR_DOWN_DELAY_US); That's 2600 us, a pretty long delay in a hard loop or delay timer! Can you use usleep_range() instead, at least for this one? > + for (int i = 0; i < 8; i++) { > + u8 next_bit = (brightness & 0x80) >> 7; I would just: #include bool next_bit = !!(brightness & BIT(7)); Yours, Linus Walleij
Re: [syzbot] [dri?] BUG: scheduling while atomic in drm_atomic_helper_wait_for_flip_done
#syz set subsystems: serial include/linux/tty_ldisc.h says "struct tty_ldisc_ops"->write is allowed to sleep. include/linux/tty_driver.h says "struct tty_operations"->write is not allowed to sleep. drivers/tty/vt/vt.c implements do_con_write() from con_write() sleeping, violating what include/linux/tty_driver.h says. But how to fix? - if (in_interrupt()) + if (in_interrupt() || in_atomic()) return count; in do_con_write() and con_flush_chars() ? But include/linux/preempt.h says in_atomic() cannot know about held spinlocks in non-preemptible kernels. Is there a way to detect spin_lock_irqsave(>tx_lock, flags) from gsmld_write() ? Something like whether irq is disabled? On 2024/01/18 18:51, syzbot wrote: > Hello, > > syzbot found the following issue on: > > HEAD commit:1b1934dbbdcf Merge tag 'docs-6.8-2' of git://git.lwn.net/l.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=1029adbde8 > kernel config: https://syzkaller.appspot.com/x/.config?x=68ea41b98043e6e8 > dashboard link: https://syzkaller.appspot.com/bug?extid=06fa1063cca8163ea541 > compiler: aarch64-linux-gnu-gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU > Binutils for Debian) 2.40 > userspace arch: arm64
[PATCH] dma-buf: Add syntax highlighting to code listings in the document
This patch tries to improve the display of the code listing on The Linux Kernel documentation website for dma-buf [1] . Originally, it appears that it was attempting to escape the '*' character, but looks like it's not necessary (now), so we are seeing something like '\*' on the webite. This patch removes these unnecessary backslashes and adds syntax highlighting to improve the readability of the code listing. [1] https://docs.kernel.org/driver-api/dma-buf.html Signed-off-by: Tommy Chiang --- drivers/dma-buf/dma-buf.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 8fe5aa67b167..e083a0ab06d7 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1282,10 +1282,12 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_move_notify, DMA_BUF); * vmap interface is introduced. Note that on very old 32-bit architectures * vmalloc space might be limited and result in vmap calls failing. * - * Interfaces:: + * Interfaces: * - * void \*dma_buf_vmap(struct dma_buf \*dmabuf, struct iosys_map \*map) - * void dma_buf_vunmap(struct dma_buf \*dmabuf, struct iosys_map \*map) + * .. code-block:: c + * + * void *dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map) + * void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map) * * The vmap call can fail if there is no vmap support in the exporter, or if * it runs out of vmalloc space. Note that the dma-buf layer keeps a reference @@ -1342,10 +1344,11 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_move_notify, DMA_BUF); * enough, since adding interfaces to intercept pagefaults and allow pte * shootdowns would increase the complexity quite a bit. * - * Interface:: + * Interface: + * + * .. code-block:: c * - * int dma_buf_mmap(struct dma_buf \*, struct vm_area_struct \*, - *unsigned long); + * int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, unsigned long); * * If the importing subsystem simply provides a special-purpose mmap call to * set up a mapping in userspace, calling do_mmap with _buf.file will -- 2.43.0.381.gb435a96ce8-goog
Re: [PATCH v2] drm: panel-orientation-quirks: Add quirk for GPD Win Mini
On Thu, 21 Dec 2023 22:01:50 -0500 Samuel Dionne-Riel wrote: Hi, Can I request a small share of your attention to review, and apply this patch? Thank you all, > Signed-off-by: Samuel Dionne-Riel > --- > > Changes since v1: > > - Add 1080p right-side up panel data > - Use the correct panel orientation > > drivers/gpu/drm/drm_panel_orientation_quirks.c | 12 > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c > b/drivers/gpu/drm/drm_panel_orientation_quirks.c > index 3d92f66e550c3..aa93129c3397e 100644 > --- a/drivers/gpu/drm/drm_panel_orientation_quirks.c > +++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c > @@ -117,6 +117,12 @@ static const struct drm_dmi_panel_orientation_data > lcd1080x1920_leftside_up = { > .orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP, > }; > > +static const struct drm_dmi_panel_orientation_data lcd1080x1920_rightside_up > = { > + .width = 1080, > + .height = 1920, > + .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP, > +}; > + > static const struct drm_dmi_panel_orientation_data lcd1200x1920_rightside_up > = { > .width = 1200, > .height = 1920, > @@ -279,6 +285,12 @@ static const struct dmi_system_id orientation_data[] = { > DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "G1618-03") > }, > .driver_data = (void *)_rightside_up, > + }, {/* GPD Win Mini */ > + .matches = { > + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "GPD"), > + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "G1617-01") > + }, > + .driver_data = (void *)_rightside_up, > }, {/* I.T.Works TW891 */ > .matches = { > DMI_EXACT_MATCH(DMI_SYS_VENDOR, "To be filled by O.E.M."),
Re: [PATCH v2 1/2] dt-bindings: backlight: add Kinetic KTD2801 binding
Hi Duje, thanks for your patch! On Thu, Jan 18, 2024 at 6:33 PM Duje Mihanović wrote: > Add the dt binding for the Kinetic KTD2801 backlight driver. Maybe add some commit message? > Reviewed-by: Krzysztof Kozlowski > Signed-off-by: Duje Mihanović (...) > + ctrl-gpios: > +maxItems: 1 First I thought this was inconsistent with ktd253, then I looked at the datasheets and they really did change "en" to "ctrl" so this needs a new name indeed. With commit message added: Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: Implement per-key keyboard backlight as auxdisplay?
Hi, On 1/18/24 18:45, Pavel Machek wrote: > Hi! > >> We have an upcoming device that has a per-key keyboard backlight, but does >> the control completely via a wmi/acpi interface. So no usable hidraw here >> for a potential userspace driver implementation ... >> >> So a quick summary for the ideas floating in this thread so far: >> >> 1. Expand leds interface allowing arbitrary modes with semi arbitrary >> optional attributes: > >> - Con: >> >> - Violates the simplicity paradigm of the leds interface (e.g. with >> this one leds entry controls possible multiple leds) > > Let's not do this. > >> 2. Implement per-key keyboards as auxdisplay >> >> - Pro: >> >> - Already has a concept for led positions >> >> - Is conceptually closer to "multiple leds forming a singular entity" >> >> - Con: >> >> - No preexisting UPower support >> >> - No concept for special hardware lightning modes >> >> - No support for arbitrary led outlines yet (e.g. ISO style >> enter-key) > > Please do this one. Ok, so based on the discussion so far and Pavel's feedback lets try to design a custom userspace API for this. I do not believe that auxdisplay is a good fit because: - auxdisplay is just a directory name, it does not seem to clearly define an API - instead the deprecated /dev/fb API is used which is deprecated - auxdisplays are very much displays (hence /dev/fb) they are typically small LCD displays with a straight widthxheight grid of square pixels - /dev/fb does gives us nothing for effects, zoned keyboard, etc. So my proposal would be an ioctl interface (ioctl only no r/w) using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev. For per key controllable rgb LEDs we need to discuss a coordinate system. I propose using a fixed size of 16 rows of 64 keys, so 64x16 in standard WxH notation. And then storing RGB in separate bytes, so userspace will then always send a buffer of 192 bytes per line (64x3) x 14 rows = 3072 bytes. With the kernel driver ignoring parts of the buffer where there are no actual keys. I would then like the map the standard 105 key layout onto this, starting at x.y (column.row) coordinates of 16.6 (with 0.0 being the top left). Leaving plenty of space on the left top and right (and some on the bottom) for extra media key rows, macro keys, etc. The idea to have the standard layout at a fixed place is to allow userspace to have a database of preset patterns which will work everywhere. Note I say standard 105 key layout, but in reality for defining the standardized part of the buffer we should use the maximum amount of keys per row of all the standard layouts, so for row 6 (the ESC row) and for extra keys on the right outside the main block we use the standard layout as shown here: http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg For the main area of the keyboard looking at: http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png We want to max rows per key, so this means that per row we use (from the above image) : row 7: 106/109 - JIS row 8: 101/104 - ANSI row 9: 102/105 - ISO row 10: 104/107 - ABNT row 11: 106/109 - JIS (with row 7 being the main area top row) This way we can address all the possible keys in the various standard layouts in one standard wat and then the drivers can just skip keys which are not there when preparing the buffer to send to the hw / fw. One open question is if we should add padding after the main area so that the printscreen / ins / del / leftarrow of the "middle" block of http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg all start at the same x (say 32) or we just pack these directly after the main area. And the same question for the numlock block, do we align this to an x of say 36, or pack it ? As for the actual IOCTL API I think there should be the following ioctls: 1. A get-info ioctl returning a struct with the following members: { char name[64] /* Keyboard model name / identifier */ int row_begin[16]; /* The x address of the first available key per row. On a std 105key kbd this will be 16 for rows 6-11, 0 for other rows */ int row_end[16]; /* x+1 for the address of the last available key per row, end - begin gives number of keys in a row */ int rgb_zones; /* number of rgb zones for zoned keyboards. Note both zones and per key addressing may be available if effects are applied per zone. */ ? } 2. A set-leds ioctl which takes the earlier discussed 3092 bytes buffer to set all the LEDs at once, only valid if at least one row has a non 0 lenght. 3. A set-zones ioctl which takes an array of bytes sized 3 * number-of-zones containing RGB values for each zone 4. A enum_effects ioctl which takes a struct with the following members: { long size; /* Size of passed in struct including the size member itself */ long effects_mask[] } the idea being that there is an enum
Re: [PATCH v2 2/4] drm/uAPI: Add "force color format" drm property as setting for userspace
On Wed, 17 Jan 2024 12:58:15 + Andri Yngvason wrote: > mið., 17. jan. 2024 kl. 09:21 skrifaði Pekka Paalanen : ... > > EDID and DisplayID standards also evolve. The kernel could be behind > > userspace in chasing them, which was the reason why the kernel does not > > validate HDR_OUTPUT_METADATA against EDID. > > > > The design of today with HDR_OUTPUT_METADATA and whatnot is > > that userspace is responsible for checking sink capabilities, and > > atomic check is responsible for driver and display controller > > capabilities. > > I'm not really sure where you're going with this. Are you for or > against userspace parsing EDID instead of getting the information from > the kernel? In that specific email, neither. I attempted to describe the current situation without any bias towards whether I think that is a good or not design. There is an existing behaviour, and if you want to deviate from that, you need more justification than for following it. Even the video modes list that I mentioned as the major example of things that userspace should not parse from EDID itself is not exhaustive nor exclusive. Userspace can still craft an arbitrary video mode and set it. If the driver and display controller can do it, it passes I believe, even if it would literally destroy the sink (in the CRT era, you could burn the flyback transistor of an unfortunate monitor). If you want me to take a stance, I think the kernel not gating settings based on EDID for these things is a good idea for these reasons: - EDID can easily be wrong, and it is easier to test sink "unsupported" configurations if you do not need to craft a modified EDID and (reboot to?) load it in the kernel first. - EDID spec gets occasionally extended. If the kernel gated settings, you would not be able to test new features without getting an updated kernel that allows them to pass. This mostly applies to blob properties, and not enums, because you cannot set arbitrary values to enum properties. Finally, as to why userspace parsing EDID at all is a good idea: - The kernel is not interested in most of the stuff contained in EDIDs, so it has no inherent reason to parse everything. - EDID is a fairly wide "API" and gets occasionally extended. Replicating all that in a kernel UAPI is a lot of work that won't benefit the kernel itself. There does not seem to be benefit in reinventing EDID information encoding in fine-grained UAPI structures, but there certainly is risk, because UAPI is written in stone once published. Userspace can get the equivalent from libraries like libdisplay-info which are much easier to develop and replace than UAPI. Thanks, pq pgpHVWtnurn6v.pgp Description: OpenPGP digital signature
Re: [PATCH v3 0/3] Convert Microchip's HLCDC Text based DT bindings to JSON schema
Hi Sam, On 19/01/24 1:00 am, Sam Ravnborg wrote: > [You don't often get email from s...@ravnborg.org. Learn why this is > important at https://aka.ms/LearnAboutSenderIdentification ] > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hi Dharma et al. > > On Thu, Jan 18, 2024 at 02:56:09PM +0530, Dharma Balasubiramani wrote: >> Converted the text bindings to YAML and validated them individually using >> following commands >> >> $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ >> $ make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ >> >> changelogs are available in respective patches. >> >> Dharma Balasubiramani (3): >>dt-bindings: display: convert Atmel's HLCDC to DT schema >>dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema >>dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format > > I know this is a bit late to ask - sorry in advance. > > The binding describes the single IP block as a multi functional device, > but it is a single IP block that includes the display controller and a > simple pwm that can be used for contrast or backlight. yes. > > If we ignore the fact that the current drivers for hlcdc uses an mfd > abstraction, is this then the optimal way to describe the HW? > > > In one of my stale git tree I converted atmel lcdc to DT, and here Are you referring the "bindings/display/atmel,lcdc.txt"? > I used: > > + "#pwm-cells": > +description: > + This PWM chip use the default 3 cells bindings > + defined in ../../pwm/pwm.yaml. > +const: 3 > + > + clocks: > +maxItems: 2 > + > + clock-names: > +maxItems: 2 > +items: > + - const: lcdc_clk > + - const: hclk > > This proved to be a simple way to describe the HW. > > To make the DT binding backward compatible you likely need to add a few > compatible that otherwise would have been left out - but that should do > the trick. again you mean the compatibles from atmel,lcdc binding? > > The current atmel hlcdc driver that is split in three is IMO an > over-engineering, and the driver could benefit merging it all in one. > And the binding should not prevent this. could you please confirm if my understanding is correct: you want a unified display binding that encompasses the properties of the two subdevices (display controller and pwm), eliminating the need to reference them in additional bindings? > > Sam -- With Best Regards, Dharma B.
Re: [PATCH] drm/panel: novatek-nt36672e: Include
On Tue, Jan 16, 2024 at 8:19 AM Ritesh Kumar wrote: > Include instead of to fix > below compilation errors: > > drivers/gpu/drm/panel/panel-novatek-nt36672e.c:564:14: error: implicit > declaration of function 'of_device_get_match_data' > ctx->desc = of_device_get_match_data(dev); > ^ > drivers/gpu/drm/panel/panel-novatek-nt36672e.c:622:34: error: array type has > incomplete element type 'struct of_device_id' > static const struct of_device_id nt36672e_of_match[] = { > ^ > > Signed-off-by: Ritesh Kumar Patch applied to drm-misc-next on top of the commit that need fixing. Yours, Linus Walleij
[PULL] drm-intel-next-fixes
Hi Dave & Sima, Here goes drm-intel-next-fixes for v6.8. Build warning fix for GCC11, fix for #10071 and DP test pattern fix, one OA fix for XeHP+. Regards, Joonas *** drm-intel-next-fixes-2024-01-19: - DSI sequence revert to fix GitLab #10071 and DP test-pattern fix - Drop -Wstringop-overflow (broken on GCC11) - OA fix on XeHP+ The following changes since commit d505a16e00c35919fd9fe5735894645e0f70a415: drm/i915/perf: reconcile Excess struct member kernel-doc warnings (2024-01-10 11:56:58 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2024-01-19 for you to fetch changes up to 84b5ece64477df4394d362d494a2496bf0878985: drm/i915: Drop -Wstringop-overflow (2024-01-18 13:04:36 +0200) - DSI sequence revert to fix GitLab #10071 and DP test-pattern fix - Drop -Wstringop-overflow (broken on GCC11) - OA fix on XeHP+ Khaled Almahallawy (1): drm/i915/dp: Fix passing the correct DPCD_REV for drm_dp_set_phy_test_pattern Lucas De Marchi (1): drm/i915: Drop -Wstringop-overflow Umesh Nerlige Ramappa (1): drm/i915/perf: Update handling of MMIO triggered reports Ville Syrjälä (1): Revert "drm/i915/dsi: Do display on sequence later on icl+" drivers/gpu/drm/i915/Makefile | 1 - drivers/gpu/drm/i915/display/icl_dsi.c | 3 +-- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/i915_perf.c| 39 - 4 files changed, 36 insertions(+), 9 deletions(-)
Re: [PATCH v2] drm: panel-orientation-quirks: Add quirk for GPD Win Mini
On Fri, Dec 22, 2023 at 4:02 AM Samuel Dionne-Riel wrote: > Signed-off-by: Samuel Dionne-Riel The patch looks OK, it was missing a commit message so I added one and applied the patch to drm-misc-next. Yours, Linus Walleij
Re: [PATCH] sh: ecovec24: Rename missed backlight field from fbdev to dev
On Mon, 2023-09-25 at 13:10 +0200, Geert Uytterhoeven wrote: > One instance of gpio_backlight_platform_data.fbdev was renamed, but the > second instance was forgotten, causing a build failure: > > arch/sh/boards/mach-ecovec24/setup.c: In function ‘arch_setup’: > arch/sh/boards/mach-ecovec24/setup.c:1223:37: error: ‘struct > gpio_backlight_platform_data’ has no member named ‘fbdev’; did you mean ‘dev’? > 1223 | gpio_backlight_data.fbdev = NULL; > | ^ > | dev > > Fix this by updating the second instance. > > Fixes: ed369def91c1579a ("backlight/gpio_backlight: Rename field 'fbdev' to > 'dev'") > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202309231601.uu6qcrnu-...@intel.com/ > Signed-off-by: Geert Uytterhoeven > --- > arch/sh/boards/mach-ecovec24/setup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/sh/boards/mach-ecovec24/setup.c > b/arch/sh/boards/mach-ecovec24/setup.c > index 3be293335de54512..7a788d44cc73496c 100644 > --- a/arch/sh/boards/mach-ecovec24/setup.c > +++ b/arch/sh/boards/mach-ecovec24/setup.c > @@ -1220,7 +1220,7 @@ static int __init arch_setup(void) > lcdc_info.ch[0].num_modes = > ARRAY_SIZE(ecovec_dvi_modes); > > /* No backlight */ > - gpio_backlight_data.fbdev = NULL; > + gpio_backlight_data.dev = NULL; > > gpio_set_value(GPIO_PTA2, 1); > gpio_set_value(GPIO_PTU1, 1); Applied to my sh-linux tree. Thanks, Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913