public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/dumb-buffers: document that it's only for linear FB
@ 2026-02-25  6:13 Icenowy Zheng
  2026-02-25  7:26 ` Thomas Zimmermann
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Icenowy Zheng @ 2026-02-25  6:13 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: dri-devel, linux-kernel, Icenowy Zheng

The ioctl interfaces for dumb buffers currently only properly support
linear buffers.

Mention this in the documentation snippet of dumb-buffers source code,
which is referenced by drm-kms.rst and will end up in the built kernel
documentation.

Also mention the existence of current drivers abusing dumb buffers for
AFBC to reduce confusion about this.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/drm_dumb_buffers.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index e2b62e5fb891b..06f74460adf62 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -57,7 +57,12 @@
  *
  * Note that dumb objects may not be used for gpu acceleration, as has been
  * attempted on some ARM embedded platforms. Such drivers really must have
- * a hardware-specific ioctl to allocate suitable buffer objects.
+ * a hardware-specific ioctl to allocate suitable buffer objects. They are
+ * also currently meant for only linear buffers, and using them with any
+ * modifier other than DRM_FORMAT_MOD_LINEAR is undefined behavior. There
+ * exist some KMS drivers abusing dumb objects for AFBC framebuffers, but this
+ * behavior is discouraged, only exists as a hack now and shouldn't be
+ * replicated.
  */
 
 static int drm_mode_align_dumb(struct drm_mode_create_dumb *args,
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/dumb-buffers: document that it's only for linear FB
  2026-02-25  6:13 [PATCH] drm/dumb-buffers: document that it's only for linear FB Icenowy Zheng
@ 2026-02-25  7:26 ` Thomas Zimmermann
  2026-02-25  7:38   ` Icenowy Zheng
  2026-02-27  3:56 ` Claude review: " Claude Code Review Bot
  2026-02-27  3:56 ` Claude Code Review Bot
  2 siblings, 1 reply; 9+ messages in thread
From: Thomas Zimmermann @ 2026-02-25  7:26 UTC (permalink / raw)
  To: Icenowy Zheng, Maarten Lankhorst, Maxime Ripard, David Airlie,
	Simona Vetter
  Cc: dri-devel, linux-kernel

Hi,

Am 25.02.26 um 07:13 schrieb Icenowy Zheng:
> The ioctl interfaces for dumb buffers currently only properly support
> linear buffers.
>
> Mention this in the documentation snippet of dumb-buffers source code,
> which is referenced by drm-kms.rst and will end up in the built kernel
> documentation.
>
> Also mention the existence of current drivers abusing dumb buffers for
> AFBC to reduce confusion about this.
>
> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> ---
>   drivers/gpu/drm/drm_dumb_buffers.c | 7 ++++++-

We documented the meaning of the color bits and the behavior of the 
dumb-buffer interface at [1]. If anything is missing, it should be added 
there.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v6.19/source/include/uapi/drm/drm_mode.h#L1200

>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index e2b62e5fb891b..06f74460adf62 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -57,7 +57,12 @@
>    *
>    * Note that dumb objects may not be used for gpu acceleration, as has been
>    * attempted on some ARM embedded platforms. Such drivers really must have
> - * a hardware-specific ioctl to allocate suitable buffer objects.
> + * a hardware-specific ioctl to allocate suitable buffer objects. They are
> + * also currently meant for only linear buffers, and using them with any
> + * modifier other than DRM_FORMAT_MOD_LINEAR is undefined behavior. There
> + * exist some KMS drivers abusing dumb objects for AFBC framebuffers, but this
> + * behavior is discouraged, only exists as a hack now and shouldn't be
> + * replicated.
>    */
>   
>   static int drm_mode_align_dumb(struct drm_mode_create_dumb *args,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/dumb-buffers: document that it's only for linear FB
  2026-02-25  7:26 ` Thomas Zimmermann
@ 2026-02-25  7:38   ` Icenowy Zheng
  2026-02-25  7:47     ` Thomas Zimmermann
  0 siblings, 1 reply; 9+ messages in thread
From: Icenowy Zheng @ 2026-02-25  7:38 UTC (permalink / raw)
  To: Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, David Airlie,
	Simona Vetter
  Cc: dri-devel, linux-kernel

在 2026-02-25三的 08:26 +0100,Thomas Zimmermann写道:
> Hi,
> 
> Am 25.02.26 um 07:13 schrieb Icenowy Zheng:
> > The ioctl interfaces for dumb buffers currently only properly
> > support
> > linear buffers.
> > 
> > Mention this in the documentation snippet of dumb-buffers source
> > code,
> > which is referenced by drm-kms.rst and will end up in the built
> > kernel
> > documentation.
> > 
> > Also mention the existence of current drivers abusing dumb buffers
> > for
> > AFBC to reduce confusion about this.
> > 
> > Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> > ---
> >   drivers/gpu/drm/drm_dumb_buffers.c | 7 ++++++-
> 
> We documented the meaning of the color bits and the behavior of the 
> dumb-buffer interface at [1]. If anything is missing, it should be
> added 
> there.

Yes, I saw this piece of document; however it's part of the interface
document instead of a concept document, and the whole existence of the
document snippet I am changing can be considered a duplicate of the
interface document.

Thanks
Icenowy

> 
> Best regards
> Thomas
> 
> [1] 
> https://elixir.bootlin.com/linux/v6.19/source/include/uapi/drm/drm_mode.h#L1200
> 
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
> > b/drivers/gpu/drm/drm_dumb_buffers.c
> > index e2b62e5fb891b..06f74460adf62 100644
> > --- a/drivers/gpu/drm/drm_dumb_buffers.c
> > +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> > @@ -57,7 +57,12 @@
> >    *
> >    * Note that dumb objects may not be used for gpu acceleration,
> > as has been
> >    * attempted on some ARM embedded platforms. Such drivers really
> > must have
> > - * a hardware-specific ioctl to allocate suitable buffer objects.
> > + * a hardware-specific ioctl to allocate suitable buffer objects.
> > They are
> > + * also currently meant for only linear buffers, and using them
> > with any
> > + * modifier other than DRM_FORMAT_MOD_LINEAR is undefined
> > behavior. There
> > + * exist some KMS drivers abusing dumb objects for AFBC
> > framebuffers, but this
> > + * behavior is discouraged, only exists as a hack now and
> > shouldn't be
> > + * replicated.
> >    */
> >   
> >   static int drm_mode_align_dumb(struct drm_mode_create_dumb *args,


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/dumb-buffers: document that it's only for linear FB
  2026-02-25  7:38   ` Icenowy Zheng
@ 2026-02-25  7:47     ` Thomas Zimmermann
  2026-02-25  8:10       ` Icenowy Zheng
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Zimmermann @ 2026-02-25  7:47 UTC (permalink / raw)
  To: Icenowy Zheng, Maarten Lankhorst, Maxime Ripard, David Airlie,
	Simona Vetter
  Cc: dri-devel, linux-kernel



Am 25.02.26 um 08:38 schrieb Icenowy Zheng:
> 在 2026-02-25三的 08:26 +0100,Thomas Zimmermann写道:
>> Hi,
>>
>> Am 25.02.26 um 07:13 schrieb Icenowy Zheng:
>>> The ioctl interfaces for dumb buffers currently only properly
>>> support
>>> linear buffers.
>>>
>>> Mention this in the documentation snippet of dumb-buffers source
>>> code,
>>> which is referenced by drm-kms.rst and will end up in the built
>>> kernel
>>> documentation.
>>>
>>> Also mention the existence of current drivers abusing dumb buffers
>>> for
>>> AFBC to reduce confusion about this.
>>>
>>> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
>>> ---
>>>    drivers/gpu/drm/drm_dumb_buffers.c | 7 ++++++-
>> We documented the meaning of the color bits and the behavior of the
>> dumb-buffer interface at [1]. If anything is missing, it should be
>> added
>> there.
> Yes, I saw this piece of document; however it's part of the interface
> document instead of a concept document, and the whole existence of the

What is a concept document?

> document snippet I am changing can be considered a duplicate of the
> interface document.
>
> Thanks
> Icenowy
>
>> Best regards
>> Thomas
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.19/source/include/uapi/drm/drm_mode.h#L1200
>>
>>>    1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
>>> b/drivers/gpu/drm/drm_dumb_buffers.c
>>> index e2b62e5fb891b..06f74460adf62 100644
>>> --- a/drivers/gpu/drm/drm_dumb_buffers.c
>>> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
>>> @@ -57,7 +57,12 @@
>>>     *
>>>     * Note that dumb objects may not be used for gpu acceleration,
>>> as has been
>>>     * attempted on some ARM embedded platforms. Such drivers really
>>> must have
>>> - * a hardware-specific ioctl to allocate suitable buffer objects.
>>> + * a hardware-specific ioctl to allocate suitable buffer objects.
>>> They are
>>> + * also currently meant for only linear buffers, and using them
>>> with any
>>> + * modifier other than DRM_FORMAT_MOD_LINEAR is undefined
>>> behavior. There
>>> + * exist some KMS drivers abusing dumb objects for AFBC
>>> framebuffers, but this
>>> + * behavior is discouraged, only exists as a hack now and
>>> shouldn't be
>>> + * replicated.
>>>     */
>>>    
>>>    static int drm_mode_align_dumb(struct drm_mode_create_dumb *args,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/dumb-buffers: document that it's only for linear FB
  2026-02-25  7:47     ` Thomas Zimmermann
@ 2026-02-25  8:10       ` Icenowy Zheng
  2026-02-25  8:34         ` Thomas Zimmermann
  0 siblings, 1 reply; 9+ messages in thread
From: Icenowy Zheng @ 2026-02-25  8:10 UTC (permalink / raw)
  To: Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, David Airlie,
	Simona Vetter
  Cc: dri-devel, linux-kernel

在 2026-02-25三的 08:47 +0100,Thomas Zimmermann写道:
> 
> 
> Am 25.02.26 um 08:38 schrieb Icenowy Zheng:
> > 在 2026-02-25三的 08:26 +0100,Thomas Zimmermann写道:
> > > Hi,
> > > 
> > > Am 25.02.26 um 07:13 schrieb Icenowy Zheng:
> > > > The ioctl interfaces for dumb buffers currently only properly
> > > > support
> > > > linear buffers.
> > > > 
> > > > Mention this in the documentation snippet of dumb-buffers
> > > > source
> > > > code,
> > > > which is referenced by drm-kms.rst and will end up in the built
> > > > kernel
> > > > documentation.
> > > > 
> > > > Also mention the existence of current drivers abusing dumb
> > > > buffers
> > > > for
> > > > AFBC to reduce confusion about this.
> > > > 
> > > > Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> > > > ---
> > > >    drivers/gpu/drm/drm_dumb_buffers.c | 7 ++++++-
> > > We documented the meaning of the color bits and the behavior of
> > > the
> > > dumb-buffer interface at [1]. If anything is missing, it should
> > > be
> > > added
> > > there.
> > Yes, I saw this piece of document; however it's part of the
> > interface
> > document instead of a concept document, and the whole existence of
> > the
> 
> What is a concept document?

Well I am patching this document snippet because it becomes part of the
document at [1] (by being referenced in the .rst file).

[1] https://docs.kernel.org/gpu/drm-kms.html#dumb-buffer-objects

> 
> > document snippet I am changing can be considered a duplicate of the
> > interface document.
> > 
> > Thanks
> > Icenowy
> > 
> > > Best regards
> > > Thomas
> > > 
> > > [1]
> > > https://elixir.bootlin.com/linux/v6.19/source/include/uapi/drm/drm_mode.h#L1200
> > > 
> > > >    1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
> > > > b/drivers/gpu/drm/drm_dumb_buffers.c
> > > > index e2b62e5fb891b..06f74460adf62 100644
> > > > --- a/drivers/gpu/drm/drm_dumb_buffers.c
> > > > +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> > > > @@ -57,7 +57,12 @@
> > > >     *
> > > >     * Note that dumb objects may not be used for gpu
> > > > acceleration,
> > > > as has been
> > > >     * attempted on some ARM embedded platforms. Such drivers
> > > > really
> > > > must have
> > > > - * a hardware-specific ioctl to allocate suitable buffer
> > > > objects.
> > > > + * a hardware-specific ioctl to allocate suitable buffer
> > > > objects.
> > > > They are
> > > > + * also currently meant for only linear buffers, and using
> > > > them
> > > > with any
> > > > + * modifier other than DRM_FORMAT_MOD_LINEAR is undefined
> > > > behavior. There
> > > > + * exist some KMS drivers abusing dumb objects for AFBC
> > > > framebuffers, but this
> > > > + * behavior is discouraged, only exists as a hack now and
> > > > shouldn't be
> > > > + * replicated.
> > > >     */
> > > >    
> > > >    static int drm_mode_align_dumb(struct drm_mode_create_dumb
> > > > *args,


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/dumb-buffers: document that it's only for linear FB
  2026-02-25  8:10       ` Icenowy Zheng
@ 2026-02-25  8:34         ` Thomas Zimmermann
  2026-02-25 14:13           ` Icenowy Zheng
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Zimmermann @ 2026-02-25  8:34 UTC (permalink / raw)
  To: Icenowy Zheng, Maarten Lankhorst, Maxime Ripard, David Airlie,
	Simona Vetter
  Cc: dri-devel, linux-kernel

Hi

Am 25.02.26 um 09:10 schrieb Icenowy Zheng:
> 在 2026-02-25三的 08:47 +0100,Thomas Zimmermann写道:
>>
>> Am 25.02.26 um 08:38 schrieb Icenowy Zheng:
>>> 在 2026-02-25三的 08:26 +0100,Thomas Zimmermann写道:
>>>> Hi,
>>>>
>>>> Am 25.02.26 um 07:13 schrieb Icenowy Zheng:
>>>>> The ioctl interfaces for dumb buffers currently only properly
>>>>> support
>>>>> linear buffers.
>>>>>
>>>>> Mention this in the documentation snippet of dumb-buffers
>>>>> source
>>>>> code,
>>>>> which is referenced by drm-kms.rst and will end up in the built
>>>>> kernel
>>>>> documentation.
>>>>>
>>>>> Also mention the existence of current drivers abusing dumb
>>>>> buffers
>>>>> for
>>>>> AFBC to reduce confusion about this.
>>>>>
>>>>> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
>>>>> ---
>>>>>     drivers/gpu/drm/drm_dumb_buffers.c | 7 ++++++-
>>>> We documented the meaning of the color bits and the behavior of
>>>> the
>>>> dumb-buffer interface at [1]. If anything is missing, it should
>>>> be
>>>> added
>>>> there.
>>> Yes, I saw this piece of document; however it's part of the
>>> interface
>>> document instead of a concept document, and the whole existence of
>>> the
>> What is a concept document?
> Well I am patching this document snippet because it becomes part of the
> document at [1] (by being referenced in the .rst file).

That question was a joke, but also not entirely untrue.

These overview sections usually introduce the purpose of a module and 
give readers a sense of how to use the code; like a tutorial. We don't 
have concept documents. As the concepts keep changing, they'd bitrot 
quickly.

For example, not too long ago we discussed the possibility of a 
CREATE_DUMB2 ioctl that would allow for specifying the DRM format 
directly. It would also allow formats with multiple planes and 
non-linear layouts. My point is that whatever we write here today could 
be obsolete tomorrow. The only stable thing is the user-space interfaces.

IMHO if you think the overview should mention the supported formats, you 
should link to the UAPI documentation of the ioctl. If we ever get that 
CREATE2 ioctl, we can refer to this as well.

Best regards
Thomas

>
> [1] https://docs.kernel.org/gpu/drm-kms.html#dumb-buffer-objects
>
>>> document snippet I am changing can be considered a duplicate of the
>>> interface document.
>>>
>>> Thanks
>>> Icenowy
>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>> [1]
>>>> https://elixir.bootlin.com/linux/v6.19/source/include/uapi/drm/drm_mode.h#L1200
>>>>
>>>>>     1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
>>>>> b/drivers/gpu/drm/drm_dumb_buffers.c
>>>>> index e2b62e5fb891b..06f74460adf62 100644
>>>>> --- a/drivers/gpu/drm/drm_dumb_buffers.c
>>>>> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
>>>>> @@ -57,7 +57,12 @@
>>>>>      *
>>>>>      * Note that dumb objects may not be used for gpu
>>>>> acceleration,
>>>>> as has been
>>>>>      * attempted on some ARM embedded platforms. Such drivers
>>>>> really
>>>>> must have
>>>>> - * a hardware-specific ioctl to allocate suitable buffer
>>>>> objects.
>>>>> + * a hardware-specific ioctl to allocate suitable buffer
>>>>> objects.
>>>>> They are
>>>>> + * also currently meant for only linear buffers, and using
>>>>> them
>>>>> with any
>>>>> + * modifier other than DRM_FORMAT_MOD_LINEAR is undefined
>>>>> behavior. There
>>>>> + * exist some KMS drivers abusing dumb objects for AFBC
>>>>> framebuffers, but this
>>>>> + * behavior is discouraged, only exists as a hack now and
>>>>> shouldn't be
>>>>> + * replicated.
>>>>>      */
>>>>>     
>>>>>     static int drm_mode_align_dumb(struct drm_mode_create_dumb
>>>>> *args,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/dumb-buffers: document that it's only for linear FB
  2026-02-25  8:34         ` Thomas Zimmermann
@ 2026-02-25 14:13           ` Icenowy Zheng
  0 siblings, 0 replies; 9+ messages in thread
From: Icenowy Zheng @ 2026-02-25 14:13 UTC (permalink / raw)
  To: Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, David Airlie,
	Simona Vetter
  Cc: dri-devel, linux-kernel

在 2026-02-25三的 09:34 +0100,Thomas Zimmermann写道:
> Hi
> 
> Am 25.02.26 um 09:10 schrieb Icenowy Zheng:
> > 在 2026-02-25三的 08:47 +0100,Thomas Zimmermann写道:
> > > 
> > > Am 25.02.26 um 08:38 schrieb Icenowy Zheng:
> > > > 在 2026-02-25三的 08:26 +0100,Thomas Zimmermann写道:
> > > > > Hi,
> > > > > 
> > > > > Am 25.02.26 um 07:13 schrieb Icenowy Zheng:
> > > > > > The ioctl interfaces for dumb buffers currently only
> > > > > > properly
> > > > > > support
> > > > > > linear buffers.
> > > > > > 
> > > > > > Mention this in the documentation snippet of dumb-buffers
> > > > > > source
> > > > > > code,
> > > > > > which is referenced by drm-kms.rst and will end up in the
> > > > > > built
> > > > > > kernel
> > > > > > documentation.
> > > > > > 
> > > > > > Also mention the existence of current drivers abusing dumb
> > > > > > buffers
> > > > > > for
> > > > > > AFBC to reduce confusion about this.
> > > > > > 
> > > > > > Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> > > > > > ---
> > > > > >     drivers/gpu/drm/drm_dumb_buffers.c | 7 ++++++-
> > > > > We documented the meaning of the color bits and the behavior
> > > > > of
> > > > > the
> > > > > dumb-buffer interface at [1]. If anything is missing, it
> > > > > should
> > > > > be
> > > > > added
> > > > > there.
> > > > Yes, I saw this piece of document; however it's part of the
> > > > interface
> > > > document instead of a concept document, and the whole existence
> > > > of
> > > > the
> > > What is a concept document?
> > Well I am patching this document snippet because it becomes part of
> > the
> > document at [1] (by being referenced in the .rst file).
> 
> That question was a joke, but also not entirely untrue.
> 
> These overview sections usually introduce the purpose of a module and
> give readers a sense of how to use the code; like a tutorial. We
> don't 
> have concept documents. As the concepts keep changing, they'd bitrot 
> quickly.
> 
> For example, not too long ago we discussed the possibility of a 
> CREATE_DUMB2 ioctl that would allow for specifying the DRM format 
> directly. It would also allow formats with multiple planes and 
> non-linear layouts. My point is that whatever we write here today
> could 
> be obsolete tomorrow. The only stable thing is the user-space
> interfaces.
> 
> IMHO if you think the overview should mention the supported formats,
> you 
> should link to the UAPI documentation of the ioctl. If we ever get
> that 
> CREATE2 ioctl, we can refer to this as well.

This sounds reasonable. I will reword this to a reference to the UAPI
documentation to raise attraction (because otherwise it's not very
natrual to consult the UAPI doc, at least for me).

Thanks,
Icenowy

> 
> Best regards
> Thomas
> 
> > 
> > [1] https://docs.kernel.org/gpu/drm-kms.html#dumb-buffer-objects
> > 
> > > > document snippet I am changing can be considered a duplicate of
> > > > the
> > > > interface document.
> > > > 
> > > > Thanks
> > > > Icenowy
> > > > 
> > > > > Best regards
> > > > > Thomas
> > > > > 
> > > > > [1]
> > > > > https://elixir.bootlin.com/linux/v6.19/source/include/uapi/drm/drm_mode.h#L1200
> > > > > 
> > > > > >     1 file changed, 6 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
> > > > > > b/drivers/gpu/drm/drm_dumb_buffers.c
> > > > > > index e2b62e5fb891b..06f74460adf62 100644
> > > > > > --- a/drivers/gpu/drm/drm_dumb_buffers.c
> > > > > > +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> > > > > > @@ -57,7 +57,12 @@
> > > > > >      *
> > > > > >      * Note that dumb objects may not be used for gpu
> > > > > > acceleration,
> > > > > > as has been
> > > > > >      * attempted on some ARM embedded platforms. Such
> > > > > > drivers
> > > > > > really
> > > > > > must have
> > > > > > - * a hardware-specific ioctl to allocate suitable buffer
> > > > > > objects.
> > > > > > + * a hardware-specific ioctl to allocate suitable buffer
> > > > > > objects.
> > > > > > They are
> > > > > > + * also currently meant for only linear buffers, and using
> > > > > > them
> > > > > > with any
> > > > > > + * modifier other than DRM_FORMAT_MOD_LINEAR is undefined
> > > > > > behavior. There
> > > > > > + * exist some KMS drivers abusing dumb objects for AFBC
> > > > > > framebuffers, but this
> > > > > > + * behavior is discouraged, only exists as a hack now and
> > > > > > shouldn't be
> > > > > > + * replicated.
> > > > > >      */
> > > > > >     
> > > > > >     static int drm_mode_align_dumb(struct
> > > > > > drm_mode_create_dumb
> > > > > > *args,


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Claude review: drm/dumb-buffers: document that it's only for linear FB
  2026-02-25  6:13 [PATCH] drm/dumb-buffers: document that it's only for linear FB Icenowy Zheng
  2026-02-25  7:26 ` Thomas Zimmermann
@ 2026-02-27  3:56 ` Claude Code Review Bot
  2026-02-27  3:56 ` Claude Code Review Bot
  2 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-02-27  3:56 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/dumb-buffers: document that it's only for linear FB
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Patches: 7
Reviewed: 2026-02-27T13:56:36.617491

---

This is a single documentation-only patch that adds clarifying text to the dumb buffers kernel-doc comment in `drm_dumb_buffers.c`. The intent is good — making it explicit that dumb buffers are meant for linear layouts only, and noting existing (but discouraged) AFBC abuse. The patch is straightforward and low-risk.

However, there are some wording and tone issues worth addressing.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Claude review: drm/dumb-buffers: document that it's only for linear FB
  2026-02-25  6:13 [PATCH] drm/dumb-buffers: document that it's only for linear FB Icenowy Zheng
  2026-02-25  7:26 ` Thomas Zimmermann
  2026-02-27  3:56 ` Claude review: " Claude Code Review Bot
@ 2026-02-27  3:56 ` Claude Code Review Bot
  2 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-02-27  3:56 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Content review:**

The added text:
```c
 * a hardware-specific ioctl to allocate suitable buffer objects. They are
 * also currently meant for only linear buffers, and using them with any
 * modifier other than DRM_FORMAT_MOD_LINEAR is undefined behavior. There
 * exist some KMS drivers abusing dumb objects for AFBC framebuffers, but this
 * behavior is discouraged, only exists as a hack now and shouldn't be
 * replicated.
```

A few observations:

1. **"undefined behavior" is misleading.** In the kernel context, "undefined behavior" has a very specific C-language connotation. Dumb buffers don't take a modifier argument at all — the `drm_mode_create_dumb` ioctl has `height`, `width`, `bpp`, `flags`, `handle`, `pitch`, and `size`. There's no modifier field. The issue isn't that passing a non-linear modifier is "undefined behavior"; it's that the interface has no mechanism to request or communicate a non-linear layout. Consider wording like: "The dumb buffer interface has no way to specify or negotiate buffer modifiers, so buffers are implicitly linear (DRM_FORMAT_MOD_LINEAR)."

2. **"They" is ambiguous.** The sentence "They are also currently meant for only linear buffers" follows a sentence about "Such drivers", so "They" could be read as referring to those specific ARM drivers rather than dumb buffers in general. Starting a new sentence more explicitly would help, e.g., "Dumb buffers are also currently meant for..."

3. **"abusing" and "hack" are strong language for kernel documentation.** While technically accurate, kernel documentation typically uses more neutral phrasing. Something like "Some KMS drivers use dumb objects for AFBC framebuffers, but this usage is non-standard and should not be replicated in new drivers" would be more appropriate for documentation that ends up in the built kernel docs.

4. **"only exists as a hack now"** — the word "now" is time-relative and will become stale. Consider "only exists for legacy reasons" or "only exists for backward compatibility" instead.

5. **Minor style nit:** "meant for only linear buffers" reads more naturally as "only meant for linear buffers."

**Suggested rewrite** of the added text:

```
 * a hardware-specific ioctl to allocate suitable buffer objects.
 *
 * Dumb buffers are only meant for linear framebuffers. The dumb buffer
 * interface has no way to specify or negotiate buffer modifiers, so
 * buffers are implicitly DRM_FORMAT_MOD_LINEAR. Some existing KMS
 * drivers use dumb objects for AFBC framebuffers, but this usage is
 * non-standard and should not be replicated in new drivers.
```

Overall, the patch addresses a real documentation gap and is welcome. With the wording cleaned up, it would be a useful addition.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-02-27  3:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-25  6:13 [PATCH] drm/dumb-buffers: document that it's only for linear FB Icenowy Zheng
2026-02-25  7:26 ` Thomas Zimmermann
2026-02-25  7:38   ` Icenowy Zheng
2026-02-25  7:47     ` Thomas Zimmermann
2026-02-25  8:10       ` Icenowy Zheng
2026-02-25  8:34         ` Thomas Zimmermann
2026-02-25 14:13           ` Icenowy Zheng
2026-02-27  3:56 ` Claude review: " Claude Code Review Bot
2026-02-27  3:56 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox