date:20240119


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

2024-01-19 Thread Bjorn Helgaas

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

2024-01-19 Thread Frank Li

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?

2024-01-19 Thread Pavel Machek

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

2024-01-19 Thread kernel test robot

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

2024-01-19 Thread Klymenko, Anatoliy

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

2024-01-19 Thread Sam Ravnborg

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)

2024-01-19 Thread pr-tracker-bot

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

2024-01-19 Thread Jens Axboe

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

2024-01-19 Thread Kuogee Hsieh


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

2024-01-19 Thread Jens Axboe

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?

2024-01-19 Thread Pavel Machek

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 ...

2024-01-19 Thread Stefan Dirsch

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?

2024-01-19 Thread 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?

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 ...

2024-01-19 Thread Stefan Dirsch

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 ...

2024-01-19 Thread Jordan Justen

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

2024-01-19 Thread Vinay Belgaumkar

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 ...

2024-01-19 Thread Stefan Dirsch

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()

2024-01-19 Thread Harshit Mogalapalli

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?

2024-01-19 Thread Dmitry Torokhov

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

2024-01-19 Thread Ville Syrjälä

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

2024-01-19 Thread André Almeida

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

2024-01-19 Thread André Almeida

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

2024-01-19 Thread André Almeida

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 ...

2024-01-19 Thread Matt Turner

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

2024-01-19 Thread Duje Mihanović

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

2024-01-19 Thread Christophe JAILLET


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

2024-01-19 Thread Jocelyn Falempe





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

2024-01-19 Thread Rodrigo Vivi

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

2024-01-19 Thread Duje Mihanović

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

2024-01-19 Thread Duje Mihanović

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

2024-01-19 Thread Erik Kurzinger

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

2024-01-19 Thread Erik Kurzinger

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

2024-01-19 Thread Erik Kurzinger

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

2024-01-19 Thread Matthew Auld


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

2024-01-19 Thread Rodrigo Vivi

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

2024-01-19 Thread Sui JIngfeng


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

2024-01-19 Thread Jani Nikula

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

2024-01-19 Thread Lucas De Marchi


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

2024-01-19 Thread Lucas De Marchi


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

2024-01-19 Thread Danilo Krummrich


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

2024-01-19 Thread Harry Wentland




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

2024-01-19 Thread kernel test robot

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

Am 19.01.24 um 13:25 schrieb Pekka Paalanen:

On Fri, 19 Jan 2024 11:58:38 +0100
Thomas Zimmermann wrote:

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

2024-01-19 Thread kernel test robot

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

2024-01-19 Thread Dmitry Baryshkov

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

2024-01-19 Thread Laurent Pinchart

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

2024-01-19 Thread 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.

Thanks,
pq

pgpRP7oNEz8LR.pgp
Description: OpenPGP digital signature

[PULL] drm-misc-next-fixes

2024-01-19 Thread Maxime Ripard

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

2024-01-19 Thread AngeloGioacchino Del Regno


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?

2024-01-19 Thread 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.


>
> 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

2024-01-19 Thread Huacai Chen

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

2024-01-19 Thread Lucas Stach

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

2024-01-19 Thread Oleksandr Natalenko

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

2024-01-19 Thread Daniel Thompson

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

2024-01-19 Thread Christian König


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

2024-01-19 Thread 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 
---
 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

2024-01-19 Thread Tetsuo Handa

#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

2024-01-19 Thread Tommy Chiang

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

2024-01-19 Thread Samuel Dionne-Riel

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?

2024-01-19 Thread 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.

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

2024-01-19 Thread Pekka Paalanen

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

2024-01-19 Thread Dharma.B

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

2024-01-19 Thread Joonas Lahtinen

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