* [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer
@ 2025-07-28 10:16 Imre Deak
2025-07-28 10:16 ` [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct() Imre Deak
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Imre Deak @ 2025-07-28 10:16 UTC (permalink / raw)
To: dri-devel
Cc: Ville Syrjälä, Mark Brown, Tomi Valkeinen, Lyude Paul,
Danilo Krummrich, Alex Deucher, Christian König,
Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, amd-gfx
This patchset fixes initializing the format info in drm_framebuffer by
drm_helper_mode_fill_fb_struct() in the omap, nouveau, radeon drivers
after patch [1] in patchset [2], the latter one missing the conversion
of these 3 drivers. This patchset converts the 3 drivers similarly to
the other drivers converted by [2].
This patchset was only compile tested.
[1] https://lore.kernel.org/all/20250701090722.13645-20-ville.syrjala@linux.intel.com
[2] https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: amd-gfx@lists.freedesktop.org
Imre Deak (3):
drm/omap: Pass along the format info from .fb_create() to
drm_helper_mode_fill_fb_struct()
drm/nouveau: Pass along the format info from .fb_create() to
drm_helper_mode_fill_fb_struct()
drm/radeon: Pass along the format info from .fb_create() to
drm_helper_mode_fill_fb_struct()
drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++------
drivers/gpu/drm/nouveau/nouveau_display.h | 3 +++
drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
drivers/gpu/drm/radeon/radeon_display.c | 5 +++--
drivers/gpu/drm/radeon/radeon_fbdev.c | 11 ++++++-----
drivers/gpu/drm/radeon/radeon_mode.h | 2 ++
8 files changed, 33 insertions(+), 27 deletions(-)
--
2.49.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer Imre Deak
@ 2025-07-28 10:16 ` Imre Deak
2025-07-29 15:03 ` Mark Brown
` (2 more replies)
2025-07-28 10:16 ` [PATCH 2/3] drm/nouveau: " Imre Deak
` (2 subsequent siblings)
3 siblings, 3 replies; 23+ messages in thread
From: Imre Deak @ 2025-07-28 10:16 UTC (permalink / raw)
To: dri-devel
Cc: Ville Syrjälä, Tomi Valkeinen, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, Mark Brown
Plumb the format info from .fb_create() all the way to
drm_helper_mode_fill_fb_struct() to avoid the redundant
lookup.
For the fbdev case a manual drm_get_format_info() lookup
is needed.
The patch is based on the driver parts of the patchset at Link:
below, which missed converting the omap driver.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/all/98b3a62c-91ff-4f91-a58b-e1265f84180b@sirena.org.uk
Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 30c81e2e5d6b3..bb3105556f193 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -351,7 +351,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
}
}
- fb = omap_framebuffer_init(dev, mode_cmd, bos);
+ fb = omap_framebuffer_init(dev, info, mode_cmd, bos);
if (IS_ERR(fb))
goto error;
@@ -365,9 +365,9 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
}
struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
+ const struct drm_format_info *info,
const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
{
- const struct drm_format_info *format = NULL;
struct omap_framebuffer *omap_fb = NULL;
struct drm_framebuffer *fb = NULL;
unsigned int pitch = mode_cmd->pitches[0];
@@ -377,15 +377,12 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
dev, mode_cmd, mode_cmd->width, mode_cmd->height,
(char *)&mode_cmd->pixel_format);
- format = drm_get_format_info(dev, mode_cmd->pixel_format,
- mode_cmd->modifier[0]);
-
for (i = 0; i < ARRAY_SIZE(formats); i++) {
if (formats[i] == mode_cmd->pixel_format)
break;
}
- if (!format || i == ARRAY_SIZE(formats)) {
+ if (i == ARRAY_SIZE(formats)) {
dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
(char *)&mode_cmd->pixel_format);
ret = -EINVAL;
@@ -399,7 +396,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
}
fb = &omap_fb->base;
- omap_fb->format = format;
+ omap_fb->format = info;
mutex_init(&omap_fb->lock);
/*
@@ -407,23 +404,23 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
* that the two planes of multiplane formats need the same number of
* bytes per pixel.
*/
- if (format->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
+ if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
dev_dbg(dev->dev, "pitches differ between planes 0 and 1\n");
ret = -EINVAL;
goto fail;
}
- if (pitch % format->cpp[0]) {
+ if (pitch % info->cpp[0]) {
dev_dbg(dev->dev,
"buffer pitch (%u bytes) is not a multiple of pixel size (%u bytes)\n",
- pitch, format->cpp[0]);
+ pitch, info->cpp[0]);
ret = -EINVAL;
goto fail;
}
- for (i = 0; i < format->num_planes; i++) {
+ for (i = 0; i < info->num_planes; i++) {
struct plane *plane = &omap_fb->planes[i];
- unsigned int vsub = i == 0 ? 1 : format->vsub;
+ unsigned int vsub = i == 0 ? 1 : info->vsub;
unsigned int size;
size = pitch * mode_cmd->height / vsub;
@@ -440,7 +437,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
plane->dma_addr = 0;
}
- drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
+ drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
if (ret) {
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.h b/drivers/gpu/drm/omapdrm/omap_fb.h
index 0873f953cf1d1..e6010302a22bd 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.h
+++ b/drivers/gpu/drm/omapdrm/omap_fb.h
@@ -13,6 +13,7 @@ struct drm_connector;
struct drm_device;
struct drm_file;
struct drm_framebuffer;
+struct drm_format_info;
struct drm_gem_object;
struct drm_mode_fb_cmd2;
struct drm_plane_state;
@@ -23,6 +24,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
struct drm_file *file, const struct drm_format_info *info,
const struct drm_mode_fb_cmd2 *mode_cmd);
struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
+ const struct drm_format_info *info,
const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos);
int omap_framebuffer_pin(struct drm_framebuffer *fb);
void omap_framebuffer_unpin(struct drm_framebuffer *fb);
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 7b63968906817..948af7ec1130b 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -197,7 +197,10 @@ int omap_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
goto fail;
}
- fb = omap_framebuffer_init(dev, &mode_cmd, &bo);
+ fb = omap_framebuffer_init(dev,
+ drm_get_format_info(dev, mode_cmd.pixel_format,
+ mode_cmd.modifier[0]),
+ &mode_cmd, &bo);
if (IS_ERR(fb)) {
dev_err(dev->dev, "failed to allocate fb\n");
/* note: if fb creation failed, we can't rely on fb destroy
--
2.49.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer Imre Deak
2025-07-28 10:16 ` [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct() Imre Deak
@ 2025-07-28 10:16 ` Imre Deak
2025-07-30 11:21 ` Imre Deak
` (2 more replies)
2025-07-28 10:16 ` [PATCH 3/3] drm/radeon: " Imre Deak
2025-07-31 18:17 ` [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer Naresh Kamboju
3 siblings, 3 replies; 23+ messages in thread
From: Imre Deak @ 2025-07-28 10:16 UTC (permalink / raw)
To: dri-devel
Cc: Ville Syrjälä, Lyude Paul, Danilo Krummrich,
Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard
Plumb the format info from .fb_create() all the way to
drm_helper_mode_fill_fb_struct() to avoid the redundant
lookup.
The patch is based on the driver parts of the patchset at Link:
below, which missed converting the nouveau driver.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++------
drivers/gpu/drm/nouveau/nouveau_display.h | 3 +++
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index e1e5421263103..805d0a87aa546 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -253,6 +253,7 @@ nouveau_check_bl_size(struct nouveau_drm *drm, struct nouveau_bo *nvbo,
int
nouveau_framebuffer_new(struct drm_device *dev,
+ const struct drm_format_info *info,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *gem,
struct drm_framebuffer **pfb)
@@ -260,7 +261,6 @@ nouveau_framebuffer_new(struct drm_device *dev,
struct nouveau_drm *drm = nouveau_drm(dev);
struct nouveau_bo *nvbo = nouveau_gem_object(gem);
struct drm_framebuffer *fb;
- const struct drm_format_info *info;
unsigned int height, i;
uint32_t tile_mode;
uint8_t kind;
@@ -295,9 +295,6 @@ nouveau_framebuffer_new(struct drm_device *dev,
kind = nvbo->kind;
}
- info = drm_get_format_info(dev, mode_cmd->pixel_format,
- mode_cmd->modifier[0]);
-
for (i = 0; i < info->num_planes; i++) {
height = drm_format_info_plane_height(info,
mode_cmd->height,
@@ -321,7 +318,7 @@ nouveau_framebuffer_new(struct drm_device *dev,
if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL)))
return -ENOMEM;
- drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
+ drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
fb->obj[0] = gem;
ret = drm_framebuffer_init(dev, fb, &nouveau_framebuffer_funcs);
@@ -344,7 +341,7 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
if (!gem)
return ERR_PTR(-ENOENT);
- ret = nouveau_framebuffer_new(dev, mode_cmd, gem, &fb);
+ ret = nouveau_framebuffer_new(dev, info, mode_cmd, gem, &fb);
if (ret == 0)
return fb;
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index e45f211501f61..470e0910d4845 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -8,8 +8,11 @@
#include <drm/drm_framebuffer.h>
+struct drm_format_info;
+
int
nouveau_framebuffer_new(struct drm_device *dev,
+ const struct drm_format_info *info,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *gem,
struct drm_framebuffer **pfb);
--
2.49.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/3] drm/radeon: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer Imre Deak
2025-07-28 10:16 ` [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct() Imre Deak
2025-07-28 10:16 ` [PATCH 2/3] drm/nouveau: " Imre Deak
@ 2025-07-28 10:16 ` Imre Deak
2025-07-28 18:42 ` Deucher, Alexander
2025-07-31 18:17 ` [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer Naresh Kamboju
3 siblings, 1 reply; 23+ messages in thread
From: Imre Deak @ 2025-07-28 10:16 UTC (permalink / raw)
To: dri-devel
Cc: Ville Syrjälä, Alex Deucher, Christian König,
Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, amd-gfx
Plumb the format info from .fb_create() all the way to
drm_helper_mode_fill_fb_struct() to avoid the redundant
lookup.
For the fbdev case a manual drm_get_format_info() lookup
is needed.
The patch is based on the driver parts of the patchset at Link:
below, which missed converting the radeon driver.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: amd-gfx@lists.freedesktop.org
Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/radeon/radeon_display.c | 5 +++--
drivers/gpu/drm/radeon/radeon_fbdev.c | 11 ++++++-----
drivers/gpu/drm/radeon/radeon_mode.h | 2 ++
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index b4bf5dfeea2dc..4dc77c398617a 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1297,12 +1297,13 @@ static const struct drm_framebuffer_funcs radeon_fb_funcs = {
int
radeon_framebuffer_init(struct drm_device *dev,
struct drm_framebuffer *fb,
+ const struct drm_format_info *info,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj)
{
int ret;
fb->obj[0] = obj;
- drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
+ drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
ret = drm_framebuffer_init(dev, fb, &radeon_fb_funcs);
if (ret) {
fb->obj[0] = NULL;
@@ -1341,7 +1342,7 @@ radeon_user_framebuffer_create(struct drm_device *dev,
return ERR_PTR(-ENOMEM);
}
- ret = radeon_framebuffer_init(dev, fb, mode_cmd, obj);
+ ret = radeon_framebuffer_init(dev, fb, info, mode_cmd, obj);
if (ret) {
kfree(fb);
drm_gem_object_put(obj);
diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c
index e3a481bbee7b6..dc81b0c2dbff3 100644
--- a/drivers/gpu/drm/radeon/radeon_fbdev.c
+++ b/drivers/gpu/drm/radeon/radeon_fbdev.c
@@ -53,10 +53,10 @@ static void radeon_fbdev_destroy_pinned_object(struct drm_gem_object *gobj)
}
static int radeon_fbdev_create_pinned_object(struct drm_fb_helper *fb_helper,
+ const struct drm_format_info *info,
struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object **gobj_p)
{
- const struct drm_format_info *info;
struct radeon_device *rdev = fb_helper->dev->dev_private;
struct drm_gem_object *gobj = NULL;
struct radeon_bo *rbo = NULL;
@@ -67,8 +67,6 @@ static int radeon_fbdev_create_pinned_object(struct drm_fb_helper *fb_helper,
int height = mode_cmd->height;
u32 cpp;
- info = drm_get_format_info(rdev_to_drm(rdev), mode_cmd->pixel_format,
- mode_cmd->modifier[0]);
cpp = info->cpp[0];
/* need to align pitch with crtc limits */
@@ -206,6 +204,7 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
struct drm_fb_helper_surface_size *sizes)
{
struct radeon_device *rdev = fb_helper->dev->dev_private;
+ const struct drm_format_info *format_info;
struct drm_mode_fb_cmd2 mode_cmd = { };
struct fb_info *info;
struct drm_gem_object *gobj;
@@ -224,7 +223,9 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
sizes->surface_depth);
- ret = radeon_fbdev_create_pinned_object(fb_helper, &mode_cmd, &gobj);
+ format_info = drm_get_format_info(rdev_to_drm(rdev), mode_cmd.pixel_format,
+ mode_cmd.modifier[0]);
+ ret = radeon_fbdev_create_pinned_object(fb_helper, format_info, &mode_cmd, &gobj);
if (ret) {
DRM_ERROR("failed to create fbcon object %d\n", ret);
return ret;
@@ -236,7 +237,7 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
ret = -ENOMEM;
goto err_radeon_fbdev_destroy_pinned_object;
}
- ret = radeon_framebuffer_init(rdev_to_drm(rdev), fb, &mode_cmd, gobj);
+ ret = radeon_framebuffer_init(rdev_to_drm(rdev), fb, format_info, &mode_cmd, gobj);
if (ret) {
DRM_ERROR("failed to initialize framebuffer %d\n", ret);
goto err_kfree;
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 3102f6c2d0556..9e34da2cacef6 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -40,6 +40,7 @@
struct drm_fb_helper;
struct drm_fb_helper_surface_size;
+struct drm_format_info;
struct edid;
struct drm_edid;
@@ -890,6 +891,7 @@ extern void
radeon_combios_encoder_dpms_scratch_regs(struct drm_encoder *encoder, bool on);
int radeon_framebuffer_init(struct drm_device *dev,
struct drm_framebuffer *rfb,
+ const struct drm_format_info *info,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj);
--
2.49.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* RE: [PATCH 3/3] drm/radeon: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 ` [PATCH 3/3] drm/radeon: " Imre Deak
@ 2025-07-28 18:42 ` Deucher, Alexander
0 siblings, 0 replies; 23+ messages in thread
From: Deucher, Alexander @ 2025-07-28 18:42 UTC (permalink / raw)
To: Imre Deak, dri-devel@lists.freedesktop.org
Cc: Ville Syrjälä, Koenig, Christian, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, amd-gfx@lists.freedesktop.org
[Public]
> -----Original Message-----
> From: Imre Deak <imre.deak@intel.com>
> Sent: Monday, July 28, 2025 6:16 AM
> To: dri-devel@lists.freedesktop.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Thomas Zimmermann <tzimmermann@suse.de>;
> Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> <mripard@kernel.org>; amd-gfx@lists.freedesktop.org
> Subject: [PATCH 3/3] drm/radeon: Pass along the format info from .fb_create() to
> drm_helper_mode_fill_fb_struct()
>
> Plumb the format info from .fb_create() all the way to
> drm_helper_mode_fill_fb_struct() to avoid the redundant lookup.
>
> For the fbdev case a manual drm_get_format_info() lookup is needed.
>
> The patch is based on the driver parts of the patchset at Link:
> below, which missed converting the radeon driver.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: amd-gfx@lists.freedesktop.org
> Fixes: 41ab92d35ccd ("drm: Make passing of format info to
> drm_helper_mode_fill_fb_struct() mandatory")
> Link: https://lore.kernel.org/all/20250701090722.13645-1-
> ville.syrjala@linux.intel.com
> Signed-off-by: Imre Deak <imre.deak@intel.com>
Series is:
Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/radeon/radeon_display.c | 5 +++--
> drivers/gpu/drm/radeon/radeon_fbdev.c | 11 ++++++-----
> drivers/gpu/drm/radeon/radeon_mode.h | 2 ++
> 3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c
> b/drivers/gpu/drm/radeon/radeon_display.c
> index b4bf5dfeea2dc..4dc77c398617a 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -1297,12 +1297,13 @@ static const struct drm_framebuffer_funcs
> radeon_fb_funcs = { int radeon_framebuffer_init(struct drm_device *dev,
> struct drm_framebuffer *fb,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object *obj)
> {
> int ret;
> fb->obj[0] = obj;
> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
> ret = drm_framebuffer_init(dev, fb, &radeon_fb_funcs);
> if (ret) {
> fb->obj[0] = NULL;
> @@ -1341,7 +1342,7 @@ radeon_user_framebuffer_create(struct drm_device
> *dev,
> return ERR_PTR(-ENOMEM);
> }
>
> - ret = radeon_framebuffer_init(dev, fb, mode_cmd, obj);
> + ret = radeon_framebuffer_init(dev, fb, info, mode_cmd, obj);
> if (ret) {
> kfree(fb);
> drm_gem_object_put(obj);
> diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c
> b/drivers/gpu/drm/radeon/radeon_fbdev.c
> index e3a481bbee7b6..dc81b0c2dbff3 100644
> --- a/drivers/gpu/drm/radeon/radeon_fbdev.c
> +++ b/drivers/gpu/drm/radeon/radeon_fbdev.c
> @@ -53,10 +53,10 @@ static void radeon_fbdev_destroy_pinned_object(struct
> drm_gem_object *gobj) }
>
> static int radeon_fbdev_create_pinned_object(struct drm_fb_helper *fb_helper,
> + const struct drm_format_info *info,
> struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object **gobj_p) {
> - const struct drm_format_info *info;
> struct radeon_device *rdev = fb_helper->dev->dev_private;
> struct drm_gem_object *gobj = NULL;
> struct radeon_bo *rbo = NULL;
> @@ -67,8 +67,6 @@ static int radeon_fbdev_create_pinned_object(struct
> drm_fb_helper *fb_helper,
> int height = mode_cmd->height;
> u32 cpp;
>
> - info = drm_get_format_info(rdev_to_drm(rdev), mode_cmd->pixel_format,
> - mode_cmd->modifier[0]);
> cpp = info->cpp[0];
>
> /* need to align pitch with crtc limits */ @@ -206,6 +204,7 @@ int
> radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
> struct drm_fb_helper_surface_size *sizes) {
> struct radeon_device *rdev = fb_helper->dev->dev_private;
> + const struct drm_format_info *format_info;
> struct drm_mode_fb_cmd2 mode_cmd = { };
> struct fb_info *info;
> struct drm_gem_object *gobj;
> @@ -224,7 +223,9 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper
> *fb_helper,
> mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes-
> >surface_bpp,
> sizes->surface_depth);
>
> - ret = radeon_fbdev_create_pinned_object(fb_helper, &mode_cmd, &gobj);
> + format_info = drm_get_format_info(rdev_to_drm(rdev),
> mode_cmd.pixel_format,
> + mode_cmd.modifier[0]);
> + ret = radeon_fbdev_create_pinned_object(fb_helper, format_info,
> +&mode_cmd, &gobj);
> if (ret) {
> DRM_ERROR("failed to create fbcon object %d\n", ret);
> return ret;
> @@ -236,7 +237,7 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper
> *fb_helper,
> ret = -ENOMEM;
> goto err_radeon_fbdev_destroy_pinned_object;
> }
> - ret = radeon_framebuffer_init(rdev_to_drm(rdev), fb, &mode_cmd, gobj);
> + ret = radeon_framebuffer_init(rdev_to_drm(rdev), fb, format_info,
> +&mode_cmd, gobj);
> if (ret) {
> DRM_ERROR("failed to initialize framebuffer %d\n", ret);
> goto err_kfree;
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
> b/drivers/gpu/drm/radeon/radeon_mode.h
> index 3102f6c2d0556..9e34da2cacef6 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -40,6 +40,7 @@
>
> struct drm_fb_helper;
> struct drm_fb_helper_surface_size;
> +struct drm_format_info;
>
> struct edid;
> struct drm_edid;
> @@ -890,6 +891,7 @@ extern void
> radeon_combios_encoder_dpms_scratch_regs(struct drm_encoder *encoder, bool
> on); int radeon_framebuffer_init(struct drm_device *dev,
> struct drm_framebuffer *rfb,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object *obj);
>
> --
> 2.49.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 ` [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct() Imre Deak
@ 2025-07-29 15:03 ` Mark Brown
2025-07-30 11:16 ` Imre Deak
2025-08-05 11:53 ` Tomi Valkeinen
2 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2025-07-29 15:03 UTC (permalink / raw)
To: Imre Deak
Cc: dri-devel, Ville Syrjälä, Tomi Valkeinen,
Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard
[-- Attachment #1: Type: text/plain, Size: 228 bytes --]
On Mon, Jul 28, 2025 at 01:16:01PM +0300, Imre Deak wrote:
> Plumb the format info from .fb_create() all the way to
> drm_helper_mode_fill_fb_struct() to avoid the redundant
> lookup.
Tested-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 ` [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct() Imre Deak
2025-07-29 15:03 ` Mark Brown
@ 2025-07-30 11:16 ` Imre Deak
2025-08-05 11:53 ` Tomi Valkeinen
2 siblings, 0 replies; 23+ messages in thread
From: Imre Deak @ 2025-07-30 11:16 UTC (permalink / raw)
To: Tomi Valkeinen, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: dri-devel, Ville Syrjälä, Mark Brown, Alex Deucher
Hi Tomi, Maarten, Maxime, Thomas, Dave, Simona,
could you please ack this patch? Note that it fixes commit 41ab92d35ccd
("drm: Make passing of format info to drm_helper_mode_fill_fb_struct()
mandatory") which is already part of Dave's 6.17-rc1 pull request at:
https://lore.kernel.org/all/CAPM=9tzVm80-v6_5nt6kko3nR+aQLZ7R98i419FV8f4-ayQWUw@mail.gmail.com
Alex already acked the patchset, but patch 1 and 2 are still missing an
ack/reviewed-by from maintainers.
Thanks,
Imre
On Mon, Jul 28, 2025 at 01:16:01PM +0300, Imre Deak wrote:
> Plumb the format info from .fb_create() all the way to
> drm_helper_mode_fill_fb_struct() to avoid the redundant
> lookup.
>
> For the fbdev case a manual drm_get_format_info() lookup
> is needed.
>
> The patch is based on the driver parts of the patchset at Link:
> below, which missed converting the omap driver.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/all/98b3a62c-91ff-4f91-a58b-e1265f84180b@sirena.org.uk
> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
> drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
> drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
> 3 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> index 30c81e2e5d6b3..bb3105556f193 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -351,7 +351,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> }
> }
>
> - fb = omap_framebuffer_init(dev, mode_cmd, bos);
> + fb = omap_framebuffer_init(dev, info, mode_cmd, bos);
> if (IS_ERR(fb))
> goto error;
>
> @@ -365,9 +365,9 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> }
>
> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
> {
> - const struct drm_format_info *format = NULL;
> struct omap_framebuffer *omap_fb = NULL;
> struct drm_framebuffer *fb = NULL;
> unsigned int pitch = mode_cmd->pitches[0];
> @@ -377,15 +377,12 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> dev, mode_cmd, mode_cmd->width, mode_cmd->height,
> (char *)&mode_cmd->pixel_format);
>
> - format = drm_get_format_info(dev, mode_cmd->pixel_format,
> - mode_cmd->modifier[0]);
> -
> for (i = 0; i < ARRAY_SIZE(formats); i++) {
> if (formats[i] == mode_cmd->pixel_format)
> break;
> }
>
> - if (!format || i == ARRAY_SIZE(formats)) {
> + if (i == ARRAY_SIZE(formats)) {
> dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
> (char *)&mode_cmd->pixel_format);
> ret = -EINVAL;
> @@ -399,7 +396,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> }
>
> fb = &omap_fb->base;
> - omap_fb->format = format;
> + omap_fb->format = info;
> mutex_init(&omap_fb->lock);
>
> /*
> @@ -407,23 +404,23 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> * that the two planes of multiplane formats need the same number of
> * bytes per pixel.
> */
> - if (format->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> + if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> dev_dbg(dev->dev, "pitches differ between planes 0 and 1\n");
> ret = -EINVAL;
> goto fail;
> }
>
> - if (pitch % format->cpp[0]) {
> + if (pitch % info->cpp[0]) {
> dev_dbg(dev->dev,
> "buffer pitch (%u bytes) is not a multiple of pixel size (%u bytes)\n",
> - pitch, format->cpp[0]);
> + pitch, info->cpp[0]);
> ret = -EINVAL;
> goto fail;
> }
>
> - for (i = 0; i < format->num_planes; i++) {
> + for (i = 0; i < info->num_planes; i++) {
> struct plane *plane = &omap_fb->planes[i];
> - unsigned int vsub = i == 0 ? 1 : format->vsub;
> + unsigned int vsub = i == 0 ? 1 : info->vsub;
> unsigned int size;
>
> size = pitch * mode_cmd->height / vsub;
> @@ -440,7 +437,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> plane->dma_addr = 0;
> }
>
> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
>
> ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
> if (ret) {
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.h b/drivers/gpu/drm/omapdrm/omap_fb.h
> index 0873f953cf1d1..e6010302a22bd 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.h
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.h
> @@ -13,6 +13,7 @@ struct drm_connector;
> struct drm_device;
> struct drm_file;
> struct drm_framebuffer;
> +struct drm_format_info;
> struct drm_gem_object;
> struct drm_mode_fb_cmd2;
> struct drm_plane_state;
> @@ -23,6 +24,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> struct drm_file *file, const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd);
> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos);
> int omap_framebuffer_pin(struct drm_framebuffer *fb);
> void omap_framebuffer_unpin(struct drm_framebuffer *fb);
> diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> index 7b63968906817..948af7ec1130b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> @@ -197,7 +197,10 @@ int omap_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> goto fail;
> }
>
> - fb = omap_framebuffer_init(dev, &mode_cmd, &bo);
> + fb = omap_framebuffer_init(dev,
> + drm_get_format_info(dev, mode_cmd.pixel_format,
> + mode_cmd.modifier[0]),
> + &mode_cmd, &bo);
> if (IS_ERR(fb)) {
> dev_err(dev->dev, "failed to allocate fb\n");
> /* note: if fb creation failed, we can't rely on fb destroy
> --
> 2.49.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 ` [PATCH 2/3] drm/nouveau: " Imre Deak
@ 2025-07-30 11:21 ` Imre Deak
2025-08-01 9:08 ` Danilo Krummrich
2025-08-01 20:28 ` James Jones
2 siblings, 0 replies; 23+ messages in thread
From: Imre Deak @ 2025-07-30 11:21 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich, David Airlie, Simona Vetter
Cc: dri-devel, Ville Syrjälä, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, Alex Deucher
Hi Lyude, Danilo, Dave, Simona,
could you please ack this patch? Note that it fixes commit 41ab92d35ccd
("drm: Make passing of format info to drm_helper_mode_fill_fb_struct()
mandatory") which is already part of Dave's 6.17-rc1 pull request at:
https://lore.kernel.org/all/CAPM=9tzVm80-v6_5nt6kko3nR+aQLZ7R98i419FV8f4-ayQWUw@mail.gmail.com
Alex already acked the patchset, but patch 1 and 2 are still missing an
ack/reviewed-by from maintainers.
Thanks,
Imre
On Mon, Jul 28, 2025 at 01:16:02PM +0300, Imre Deak wrote:
> Plumb the format info from .fb_create() all the way to
> drm_helper_mode_fill_fb_struct() to avoid the redundant
> lookup.
>
> The patch is based on the driver parts of the patchset at Link:
> below, which missed converting the nouveau driver.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++------
> drivers/gpu/drm/nouveau/nouveau_display.h | 3 +++
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index e1e5421263103..805d0a87aa546 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -253,6 +253,7 @@ nouveau_check_bl_size(struct nouveau_drm *drm, struct nouveau_bo *nvbo,
>
> int
> nouveau_framebuffer_new(struct drm_device *dev,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object *gem,
> struct drm_framebuffer **pfb)
> @@ -260,7 +261,6 @@ nouveau_framebuffer_new(struct drm_device *dev,
> struct nouveau_drm *drm = nouveau_drm(dev);
> struct nouveau_bo *nvbo = nouveau_gem_object(gem);
> struct drm_framebuffer *fb;
> - const struct drm_format_info *info;
> unsigned int height, i;
> uint32_t tile_mode;
> uint8_t kind;
> @@ -295,9 +295,6 @@ nouveau_framebuffer_new(struct drm_device *dev,
> kind = nvbo->kind;
> }
>
> - info = drm_get_format_info(dev, mode_cmd->pixel_format,
> - mode_cmd->modifier[0]);
> -
> for (i = 0; i < info->num_planes; i++) {
> height = drm_format_info_plane_height(info,
> mode_cmd->height,
> @@ -321,7 +318,7 @@ nouveau_framebuffer_new(struct drm_device *dev,
> if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL)))
> return -ENOMEM;
>
> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
> fb->obj[0] = gem;
>
> ret = drm_framebuffer_init(dev, fb, &nouveau_framebuffer_funcs);
> @@ -344,7 +341,7 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
> if (!gem)
> return ERR_PTR(-ENOENT);
>
> - ret = nouveau_framebuffer_new(dev, mode_cmd, gem, &fb);
> + ret = nouveau_framebuffer_new(dev, info, mode_cmd, gem, &fb);
> if (ret == 0)
> return fb;
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
> index e45f211501f61..470e0910d4845 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -8,8 +8,11 @@
>
> #include <drm/drm_framebuffer.h>
>
> +struct drm_format_info;
> +
> int
> nouveau_framebuffer_new(struct drm_device *dev,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object *gem,
> struct drm_framebuffer **pfb);
> --
> 2.49.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer
2025-07-28 10:16 [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer Imre Deak
` (2 preceding siblings ...)
2025-07-28 10:16 ` [PATCH 3/3] drm/radeon: " Imre Deak
@ 2025-07-31 18:17 ` Naresh Kamboju
3 siblings, 0 replies; 23+ messages in thread
From: Naresh Kamboju @ 2025-07-31 18:17 UTC (permalink / raw)
To: imre.deak
Cc: alexander.deucher, amd-gfx, broonie, christian.koenig, dakr,
dri-devel, lyude, maarten.lankhorst, mripard, tomi.valkeinen,
tzimmermann, ville.syrjala, Linux Kernel Functional Testing
> This patchset fixes initializing the format info in drm_framebuffer by
> drm_helper_mode_fill_fb_struct() in the omap, nouveau, radeon drivers
> after patch [1] in patchset [2], the latter one missing the conversion
> of these 3 drivers. This patchset converts the 3 drivers similarly to
> the other drivers converted by [2].
> This patchset was only compile tested.
> [1] https://lore.kernel.org/all/20250701090722.13645-20-ville.syrjala@linux.intel.com
> [2] https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Test job,
- https://lkft.validation.linaro.org/scheduler/job/8377982
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 ` [PATCH 2/3] drm/nouveau: " Imre Deak
2025-07-30 11:21 ` Imre Deak
@ 2025-08-01 9:08 ` Danilo Krummrich
2025-08-12 17:04 ` Danilo Krummrich
2025-08-01 20:28 ` James Jones
2 siblings, 1 reply; 23+ messages in thread
From: Danilo Krummrich @ 2025-08-01 9:08 UTC (permalink / raw)
To: Imre Deak
Cc: dri-devel, Ville Syrjälä, Lyude Paul, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard
On 7/28/25 12:16 PM, Imre Deak wrote:
> Plumb the format info from .fb_create() all the way to
> drm_helper_mode_fill_fb_struct() to avoid the redundant
> lookup.
>
> The patch is based on the driver parts of the patchset at Link:
> below, which missed converting the nouveau driver.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> Signed-off-by: Imre Deak <imre.deak@intel.com>
Acked-by: Danilo Krummrich <dakr@kernel.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 ` [PATCH 2/3] drm/nouveau: " Imre Deak
2025-07-30 11:21 ` Imre Deak
2025-08-01 9:08 ` Danilo Krummrich
@ 2025-08-01 20:28 ` James Jones
2 siblings, 0 replies; 23+ messages in thread
From: James Jones @ 2025-08-01 20:28 UTC (permalink / raw)
To: Imre Deak, dri-devel
Cc: Ville Syrjälä, Lyude Paul, Danilo Krummrich,
Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard
Review-by: James Jones <jajones@nvidia.com>
Tested-by: James Jones <jajones@nvidia.com>
Tested on GB203, TU102, and NV50.
Thanks,
-James
On 7/28/25 03:16, Imre Deak wrote:
> Plumb the format info from .fb_create() all the way to
> drm_helper_mode_fill_fb_struct() to avoid the redundant
> lookup.
>
> The patch is based on the driver parts of the patchset at Link:
> below, which missed converting the nouveau driver.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++------
> drivers/gpu/drm/nouveau/nouveau_display.h | 3 +++
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index e1e5421263103..805d0a87aa546 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -253,6 +253,7 @@ nouveau_check_bl_size(struct nouveau_drm *drm, struct nouveau_bo *nvbo,
>
> int
> nouveau_framebuffer_new(struct drm_device *dev,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object *gem,
> struct drm_framebuffer **pfb)
> @@ -260,7 +261,6 @@ nouveau_framebuffer_new(struct drm_device *dev,
> struct nouveau_drm *drm = nouveau_drm(dev);
> struct nouveau_bo *nvbo = nouveau_gem_object(gem);
> struct drm_framebuffer *fb;
> - const struct drm_format_info *info;
> unsigned int height, i;
> uint32_t tile_mode;
> uint8_t kind;
> @@ -295,9 +295,6 @@ nouveau_framebuffer_new(struct drm_device *dev,
> kind = nvbo->kind;
> }
>
> - info = drm_get_format_info(dev, mode_cmd->pixel_format,
> - mode_cmd->modifier[0]);
> -
> for (i = 0; i < info->num_planes; i++) {
> height = drm_format_info_plane_height(info,
> mode_cmd->height,
> @@ -321,7 +318,7 @@ nouveau_framebuffer_new(struct drm_device *dev,
> if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL)))
> return -ENOMEM;
>
> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
> fb->obj[0] = gem;
>
> ret = drm_framebuffer_init(dev, fb, &nouveau_framebuffer_funcs);
> @@ -344,7 +341,7 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
> if (!gem)
> return ERR_PTR(-ENOENT);
>
> - ret = nouveau_framebuffer_new(dev, mode_cmd, gem, &fb);
> + ret = nouveau_framebuffer_new(dev, info, mode_cmd, gem, &fb);
> if (ret == 0)
> return fb;
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
> index e45f211501f61..470e0910d4845 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -8,8 +8,11 @@
>
> #include <drm/drm_framebuffer.h>
>
> +struct drm_format_info;
> +
> int
> nouveau_framebuffer_new(struct drm_device *dev,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object *gem,
> struct drm_framebuffer **pfb);
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-07-28 10:16 ` [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct() Imre Deak
2025-07-29 15:03 ` Mark Brown
2025-07-30 11:16 ` Imre Deak
@ 2025-08-05 11:53 ` Tomi Valkeinen
2025-08-05 14:49 ` Imre Deak
2 siblings, 1 reply; 23+ messages in thread
From: Tomi Valkeinen @ 2025-08-05 11:53 UTC (permalink / raw)
To: Imre Deak, dri-devel
Cc: Ville Syrjälä, Thomas Zimmermann, Maarten Lankhorst,
Maxime Ripard, Mark Brown
Hi Imre,
On 28/07/2025 13:16, Imre Deak wrote:
> Plumb the format info from .fb_create() all the way to
> drm_helper_mode_fill_fb_struct() to avoid the redundant
> lookup.
>
> For the fbdev case a manual drm_get_format_info() lookup
> is needed.
>
> The patch is based on the driver parts of the patchset at Link:
> below, which missed converting the omap driver.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/all/98b3a62c-91ff-4f91-a58b-e1265f84180b@sirena.org.uk
> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
> drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
> drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
> 3 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> index 30c81e2e5d6b3..bb3105556f193 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -351,7 +351,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> }
> }
>
> - fb = omap_framebuffer_init(dev, mode_cmd, bos);
> + fb = omap_framebuffer_init(dev, info, mode_cmd, bos);
> if (IS_ERR(fb))
> goto error;
>
> @@ -365,9 +365,9 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> }
>
> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
> {
> - const struct drm_format_info *format = NULL;
> struct omap_framebuffer *omap_fb = NULL;
> struct drm_framebuffer *fb = NULL;
> unsigned int pitch = mode_cmd->pitches[0];
> @@ -377,15 +377,12 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> dev, mode_cmd, mode_cmd->width, mode_cmd->height,
> (char *)&mode_cmd->pixel_format);
>
> - format = drm_get_format_info(dev, mode_cmd->pixel_format,
> - mode_cmd->modifier[0]);
> -
> for (i = 0; i < ARRAY_SIZE(formats); i++) {
> if (formats[i] == mode_cmd->pixel_format)
> break;
> }
>
> - if (!format || i == ARRAY_SIZE(formats)) {
> + if (i == ARRAY_SIZE(formats)) {
> dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
> (char *)&mode_cmd->pixel_format);
> ret = -EINVAL;
> @@ -399,7 +396,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> }
>
> fb = &omap_fb->base;
> - omap_fb->format = format;
> + omap_fb->format = info;
> mutex_init(&omap_fb->lock);
>
> /*
> @@ -407,23 +404,23 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> * that the two planes of multiplane formats need the same number of
> * bytes per pixel.
> */
> - if (format->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> + if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> dev_dbg(dev->dev, "pitches differ between planes 0 and 1\n");
> ret = -EINVAL;
> goto fail;
> }
>
> - if (pitch % format->cpp[0]) {
> + if (pitch % info->cpp[0]) {
> dev_dbg(dev->dev,
> "buffer pitch (%u bytes) is not a multiple of pixel size (%u bytes)\n",
> - pitch, format->cpp[0]);
> + pitch, info->cpp[0]);
> ret = -EINVAL;
> goto fail;
> }
>
> - for (i = 0; i < format->num_planes; i++) {
> + for (i = 0; i < info->num_planes; i++) {
> struct plane *plane = &omap_fb->planes[i];
> - unsigned int vsub = i == 0 ? 1 : format->vsub;
> + unsigned int vsub = i == 0 ? 1 : info->vsub;
> unsigned int size;
>
> size = pitch * mode_cmd->height / vsub;
> @@ -440,7 +437,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> plane->dma_addr = 0;
> }
>
> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
Isn't the fix really a one-liner, just passing "format" here instead of
NULL?
Tomi
>
> ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
> if (ret) {
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.h b/drivers/gpu/drm/omapdrm/omap_fb.h
> index 0873f953cf1d1..e6010302a22bd 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.h
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.h
> @@ -13,6 +13,7 @@ struct drm_connector;
> struct drm_device;
> struct drm_file;
> struct drm_framebuffer;
> +struct drm_format_info;
> struct drm_gem_object;
> struct drm_mode_fb_cmd2;
> struct drm_plane_state;
> @@ -23,6 +24,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> struct drm_file *file, const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd);
> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> + const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos);
> int omap_framebuffer_pin(struct drm_framebuffer *fb);
> void omap_framebuffer_unpin(struct drm_framebuffer *fb);
> diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> index 7b63968906817..948af7ec1130b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> @@ -197,7 +197,10 @@ int omap_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> goto fail;
> }
>
> - fb = omap_framebuffer_init(dev, &mode_cmd, &bo);
> + fb = omap_framebuffer_init(dev,
> + drm_get_format_info(dev, mode_cmd.pixel_format,
> + mode_cmd.modifier[0]),
> + &mode_cmd, &bo);
> if (IS_ERR(fb)) {
> dev_err(dev->dev, "failed to allocate fb\n");
> /* note: if fb creation failed, we can't rely on fb destroy
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-05 11:53 ` Tomi Valkeinen
@ 2025-08-05 14:49 ` Imre Deak
2025-08-05 15:43 ` Tomi Valkeinen
0 siblings, 1 reply; 23+ messages in thread
From: Imre Deak @ 2025-08-05 14:49 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: dri-devel, Ville Syrjälä, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, Mark Brown
Hi Tomi,
On Tue, Aug 05, 2025 at 02:53:36PM +0300, Tomi Valkeinen wrote:
> Hi Imre,
>
> On 28/07/2025 13:16, Imre Deak wrote:
> > Plumb the format info from .fb_create() all the way to
> > drm_helper_mode_fill_fb_struct() to avoid the redundant
> > lookup.
> >
> > For the fbdev case a manual drm_get_format_info() lookup
> > is needed.
> >
> > The patch is based on the driver parts of the patchset at Link:
> > below, which missed converting the omap driver.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> > Reported-by: Mark Brown <broonie@kernel.org>
> > Closes: https://lore.kernel.org/all/98b3a62c-91ff-4f91-a58b-e1265f84180b@sirena.org.uk
> > Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
> > drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
> > drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
> > 3 files changed, 16 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> > index 30c81e2e5d6b3..bb3105556f193 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> > @@ -351,7 +351,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> > }
> > }
> >
> > - fb = omap_framebuffer_init(dev, mode_cmd, bos);
> > + fb = omap_framebuffer_init(dev, info, mode_cmd, bos);
> > if (IS_ERR(fb))
> > goto error;
> >
> > @@ -365,9 +365,9 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> > }
> >
> > struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> > + const struct drm_format_info *info,
> > const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
> > {
> > - const struct drm_format_info *format = NULL;
> > struct omap_framebuffer *omap_fb = NULL;
> > struct drm_framebuffer *fb = NULL;
> > unsigned int pitch = mode_cmd->pitches[0];
> > @@ -377,15 +377,12 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> > dev, mode_cmd, mode_cmd->width, mode_cmd->height,
> > (char *)&mode_cmd->pixel_format);
> >
> > - format = drm_get_format_info(dev, mode_cmd->pixel_format,
> > - mode_cmd->modifier[0]);
> > -
> > for (i = 0; i < ARRAY_SIZE(formats); i++) {
> > if (formats[i] == mode_cmd->pixel_format)
> > break;
> > }
> >
> > - if (!format || i == ARRAY_SIZE(formats)) {
> > + if (i == ARRAY_SIZE(formats)) {
> > dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
> > (char *)&mode_cmd->pixel_format);
> > ret = -EINVAL;
> > @@ -399,7 +396,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> > }
> >
> > fb = &omap_fb->base;
> > - omap_fb->format = format;
> > + omap_fb->format = info;
> > mutex_init(&omap_fb->lock);
> >
> > /*
> > @@ -407,23 +404,23 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> > * that the two planes of multiplane formats need the same number of
> > * bytes per pixel.
> > */
> > - if (format->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> > + if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> > dev_dbg(dev->dev, "pitches differ between planes 0 and 1\n");
> > ret = -EINVAL;
> > goto fail;
> > }
> >
> > - if (pitch % format->cpp[0]) {
> > + if (pitch % info->cpp[0]) {
> > dev_dbg(dev->dev,
> > "buffer pitch (%u bytes) is not a multiple of pixel size (%u bytes)\n",
> > - pitch, format->cpp[0]);
> > + pitch, info->cpp[0]);
> > ret = -EINVAL;
> > goto fail;
> > }
> >
> > - for (i = 0; i < format->num_planes; i++) {
> > + for (i = 0; i < info->num_planes; i++) {
> > struct plane *plane = &omap_fb->planes[i];
> > - unsigned int vsub = i == 0 ? 1 : format->vsub;
> > + unsigned int vsub = i == 0 ? 1 : info->vsub;
> > unsigned int size;
> >
> > size = pitch * mode_cmd->height / vsub;
> > @@ -440,7 +437,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> > plane->dma_addr = 0;
> > }
> >
> > - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> > + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
>
> Isn't the fix really a one-liner, just passing "format" here instead of
> NULL?
That would fix the issue of fb->format being initialized to NULL yes.
However, I think the change should be in sync with the conversion of the
rest of the drivers in patchset [1]. IOW, what this patch is meant to
fix is that [1] missed converting the OMAP driver similarly to the other
drivers.
I think the change is equivalent to the above one-liner you suggest with
the following differences:
- omap_framebuffer_init() uses the drm_format_info passed down from either
drm_internal_framebuffer_create(), or omap_fbdev_driver_fbdev_probe().
In both cases the passed down format info matches what
omap_framebuffer_init() would look up again.
- Skip the format == NULL check. format can't be NULL in any case, since
that would have emitted a WARN already in drm_get_format_info() ->
drm_format_info().
[1] https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> Tomi
>
> >
> > ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
> > if (ret) {
> > diff --git a/drivers/gpu/drm/omapdrm/omap_fb.h b/drivers/gpu/drm/omapdrm/omap_fb.h
> > index 0873f953cf1d1..e6010302a22bd 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_fb.h
> > +++ b/drivers/gpu/drm/omapdrm/omap_fb.h
> > @@ -13,6 +13,7 @@ struct drm_connector;
> > struct drm_device;
> > struct drm_file;
> > struct drm_framebuffer;
> > +struct drm_format_info;
> > struct drm_gem_object;
> > struct drm_mode_fb_cmd2;
> > struct drm_plane_state;
> > @@ -23,6 +24,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> > struct drm_file *file, const struct drm_format_info *info,
> > const struct drm_mode_fb_cmd2 *mode_cmd);
> > struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> > + const struct drm_format_info *info,
> > const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos);
> > int omap_framebuffer_pin(struct drm_framebuffer *fb);
> > void omap_framebuffer_unpin(struct drm_framebuffer *fb);
> > diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> > index 7b63968906817..948af7ec1130b 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> > @@ -197,7 +197,10 @@ int omap_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> > goto fail;
> > }
> >
> > - fb = omap_framebuffer_init(dev, &mode_cmd, &bo);
> > + fb = omap_framebuffer_init(dev,
> > + drm_get_format_info(dev, mode_cmd.pixel_format,
> > + mode_cmd.modifier[0]),
> > + &mode_cmd, &bo);
> > if (IS_ERR(fb)) {
> > dev_err(dev->dev, "failed to allocate fb\n");
> > /* note: if fb creation failed, we can't rely on fb destroy
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-05 14:49 ` Imre Deak
@ 2025-08-05 15:43 ` Tomi Valkeinen
2025-08-05 16:22 ` Imre Deak
2025-08-05 16:24 ` Mark Brown
0 siblings, 2 replies; 23+ messages in thread
From: Tomi Valkeinen @ 2025-08-05 15:43 UTC (permalink / raw)
To: imre.deak
Cc: dri-devel, Ville Syrjälä, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, Mark Brown
Hi,
On 05/08/2025 17:49, Imre Deak wrote:
> Hi Tomi,
>
> On Tue, Aug 05, 2025 at 02:53:36PM +0300, Tomi Valkeinen wrote:
>> Hi Imre,
>>
>> On 28/07/2025 13:16, Imre Deak wrote:
>>> Plumb the format info from .fb_create() all the way to
>>> drm_helper_mode_fill_fb_struct() to avoid the redundant
>>> lookup.
>>>
>>> For the fbdev case a manual drm_get_format_info() lookup
>>> is needed.
>>>
>>> The patch is based on the driver parts of the patchset at Link:
>>> below, which missed converting the omap driver.
>>>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: Maxime Ripard <mripard@kernel.org>
>>> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
>>> Reported-by: Mark Brown <broonie@kernel.org>
>>> Closes: https://lore.kernel.org/all/98b3a62c-91ff-4f91-a58b-e1265f84180b@sirena.org.uk
>>> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>> ---
>>> drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
>>> drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
>>> drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
>>> 3 files changed, 16 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
>>> index 30c81e2e5d6b3..bb3105556f193 100644
>>> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
>>> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
>>> @@ -351,7 +351,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
>>> }
>>> }
>>>
>>> - fb = omap_framebuffer_init(dev, mode_cmd, bos);
>>> + fb = omap_framebuffer_init(dev, info, mode_cmd, bos);
>>> if (IS_ERR(fb))
>>> goto error;
>>>
>>> @@ -365,9 +365,9 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
>>> }
>>>
>>> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>> + const struct drm_format_info *info,
>>> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
>>> {
>>> - const struct drm_format_info *format = NULL;
>>> struct omap_framebuffer *omap_fb = NULL;
>>> struct drm_framebuffer *fb = NULL;
>>> unsigned int pitch = mode_cmd->pitches[0];
>>> @@ -377,15 +377,12 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>> dev, mode_cmd, mode_cmd->width, mode_cmd->height,
>>> (char *)&mode_cmd->pixel_format);
>>>
>>> - format = drm_get_format_info(dev, mode_cmd->pixel_format,
>>> - mode_cmd->modifier[0]);
>>> -
>>> for (i = 0; i < ARRAY_SIZE(formats); i++) {
>>> if (formats[i] == mode_cmd->pixel_format)
>>> break;
>>> }
>>>
>>> - if (!format || i == ARRAY_SIZE(formats)) {
>>> + if (i == ARRAY_SIZE(formats)) {
>>> dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
>>> (char *)&mode_cmd->pixel_format);
>>> ret = -EINVAL;
>>> @@ -399,7 +396,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>> }
>>>
>>> fb = &omap_fb->base;
>>> - omap_fb->format = format;
>>> + omap_fb->format = info;
>>> mutex_init(&omap_fb->lock);
>>>
>>> /*
>>> @@ -407,23 +404,23 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>> * that the two planes of multiplane formats need the same number of
>>> * bytes per pixel.
>>> */
>>> - if (format->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
>>> + if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
>>> dev_dbg(dev->dev, "pitches differ between planes 0 and 1\n");
>>> ret = -EINVAL;
>>> goto fail;
>>> }
>>>
>>> - if (pitch % format->cpp[0]) {
>>> + if (pitch % info->cpp[0]) {
>>> dev_dbg(dev->dev,
>>> "buffer pitch (%u bytes) is not a multiple of pixel size (%u bytes)\n",
>>> - pitch, format->cpp[0]);
>>> + pitch, info->cpp[0]);
>>> ret = -EINVAL;
>>> goto fail;
>>> }
>>>
>>> - for (i = 0; i < format->num_planes; i++) {
>>> + for (i = 0; i < info->num_planes; i++) {
>>> struct plane *plane = &omap_fb->planes[i];
>>> - unsigned int vsub = i == 0 ? 1 : format->vsub;
>>> + unsigned int vsub = i == 0 ? 1 : info->vsub;
>>> unsigned int size;
>>>
>>> size = pitch * mode_cmd->height / vsub;
>>> @@ -440,7 +437,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>> plane->dma_addr = 0;
>>> }
>>>
>>> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
>>> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
>>
>> Isn't the fix really a one-liner, just passing "format" here instead of
>> NULL?
>
> That would fix the issue of fb->format being initialized to NULL yes.
>
> However, I think the change should be in sync with the conversion of the
> rest of the drivers in patchset [1]. IOW, what this patch is meant to
> fix is that [1] missed converting the OMAP driver similarly to the other
> drivers.
>
> I think the change is equivalent to the above one-liner you suggest with
> the following differences:
>
> - omap_framebuffer_init() uses the drm_format_info passed down from either
> drm_internal_framebuffer_create(), or omap_fbdev_driver_fbdev_probe().
> In both cases the passed down format info matches what
> omap_framebuffer_init() would look up again.
>
> - Skip the format == NULL check. format can't be NULL in any case, since
> that would have emitted a WARN already in drm_get_format_info() ->
> drm_format_info().
>
> [1] https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
Yep, I have no issue with the patch as such. But if it's a fix, going
into an rc, I think it's better if it's as small as possible, and do the
cleanups/refactorings as a non-fix later.
The patch description here sounds more like it's just refactoring the
code a bit, but if I understand correctly, it's fixing an issue that
cause a WARN?
So... Either we could 1) split the patch, have the WARN fix as a fix
patch to the current rc, and the rest later in the next merge window, or
2) if you want to keep this as a single patch, I think it makes sense to
change the subject and description to highlight the fix aspect.
I think my comments apply to all patches in this series, as they're
essentially doing the same thing...
Tomi
>> Tomi
>>
>>>
>>> ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
>>> if (ret) {
>>> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.h b/drivers/gpu/drm/omapdrm/omap_fb.h
>>> index 0873f953cf1d1..e6010302a22bd 100644
>>> --- a/drivers/gpu/drm/omapdrm/omap_fb.h
>>> +++ b/drivers/gpu/drm/omapdrm/omap_fb.h
>>> @@ -13,6 +13,7 @@ struct drm_connector;
>>> struct drm_device;
>>> struct drm_file;
>>> struct drm_framebuffer;
>>> +struct drm_format_info;
>>> struct drm_gem_object;
>>> struct drm_mode_fb_cmd2;
>>> struct drm_plane_state;
>>> @@ -23,6 +24,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
>>> struct drm_file *file, const struct drm_format_info *info,
>>> const struct drm_mode_fb_cmd2 *mode_cmd);
>>> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>> + const struct drm_format_info *info,
>>> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos);
>>> int omap_framebuffer_pin(struct drm_framebuffer *fb);
>>> void omap_framebuffer_unpin(struct drm_framebuffer *fb);
>>> diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
>>> index 7b63968906817..948af7ec1130b 100644
>>> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
>>> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
>>> @@ -197,7 +197,10 @@ int omap_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
>>> goto fail;
>>> }
>>>
>>> - fb = omap_framebuffer_init(dev, &mode_cmd, &bo);
>>> + fb = omap_framebuffer_init(dev,
>>> + drm_get_format_info(dev, mode_cmd.pixel_format,
>>> + mode_cmd.modifier[0]),
>>> + &mode_cmd, &bo);
>>> if (IS_ERR(fb)) {
>>> dev_err(dev->dev, "failed to allocate fb\n");
>>> /* note: if fb creation failed, we can't rely on fb destroy
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-05 15:43 ` Tomi Valkeinen
@ 2025-08-05 16:22 ` Imre Deak
2025-08-05 16:56 ` Tomi Valkeinen
2025-08-05 16:24 ` Mark Brown
1 sibling, 1 reply; 23+ messages in thread
From: Imre Deak @ 2025-08-05 16:22 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: dri-devel, Ville Syrjälä, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, Mark Brown
On Tue, Aug 05, 2025 at 06:43:04PM +0300, Tomi Valkeinen wrote:
> Hi,
>
> On 05/08/2025 17:49, Imre Deak wrote:
> > Hi Tomi,
> >
> > On Tue, Aug 05, 2025 at 02:53:36PM +0300, Tomi Valkeinen wrote:
> >> Hi Imre,
> >>
> >> On 28/07/2025 13:16, Imre Deak wrote:
> >>> Plumb the format info from .fb_create() all the way to
> >>> drm_helper_mode_fill_fb_struct() to avoid the redundant
> >>> lookup.
> >>>
> >>> For the fbdev case a manual drm_get_format_info() lookup
> >>> is needed.
> >>>
> >>> The patch is based on the driver parts of the patchset at Link:
> >>> below, which missed converting the omap driver.
> >>>
> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Cc: Maxime Ripard <mripard@kernel.org>
> >>> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> >>> Reported-by: Mark Brown <broonie@kernel.org>
> >>> Closes: https://lore.kernel.org/all/98b3a62c-91ff-4f91-a58b-e1265f84180b@sirena.org.uk
> >>> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> >>> Signed-off-by: Imre Deak <imre.deak@intel.com>
> >>> ---
> >>> drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
> >>> drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
> >>> drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
> >>> 3 files changed, 16 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> >>> index 30c81e2e5d6b3..bb3105556f193 100644
> >>> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> >>> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> >>> @@ -351,7 +351,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> >>> }
> >>> }
> >>>
> >>> - fb = omap_framebuffer_init(dev, mode_cmd, bos);
> >>> + fb = omap_framebuffer_init(dev, info, mode_cmd, bos);
> >>> if (IS_ERR(fb))
> >>> goto error;
> >>>
> >>> @@ -365,9 +365,9 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> >>> }
> >>>
> >>> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>> + const struct drm_format_info *info,
> >>> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
> >>> {
> >>> - const struct drm_format_info *format = NULL;
> >>> struct omap_framebuffer *omap_fb = NULL;
> >>> struct drm_framebuffer *fb = NULL;
> >>> unsigned int pitch = mode_cmd->pitches[0];
> >>> @@ -377,15 +377,12 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>> dev, mode_cmd, mode_cmd->width, mode_cmd->height,
> >>> (char *)&mode_cmd->pixel_format);
> >>>
> >>> - format = drm_get_format_info(dev, mode_cmd->pixel_format,
> >>> - mode_cmd->modifier[0]);
> >>> -
> >>> for (i = 0; i < ARRAY_SIZE(formats); i++) {
> >>> if (formats[i] == mode_cmd->pixel_format)
> >>> break;
> >>> }
> >>>
> >>> - if (!format || i == ARRAY_SIZE(formats)) {
> >>> + if (i == ARRAY_SIZE(formats)) {
> >>> dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
> >>> (char *)&mode_cmd->pixel_format);
> >>> ret = -EINVAL;
> >>> @@ -399,7 +396,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>> }
> >>>
> >>> fb = &omap_fb->base;
> >>> - omap_fb->format = format;
> >>> + omap_fb->format = info;
> >>> mutex_init(&omap_fb->lock);
> >>>
> >>> /*
> >>> @@ -407,23 +404,23 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>> * that the two planes of multiplane formats need the same number of
> >>> * bytes per pixel.
> >>> */
> >>> - if (format->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> >>> + if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> >>> dev_dbg(dev->dev, "pitches differ between planes 0 and 1\n");
> >>> ret = -EINVAL;
> >>> goto fail;
> >>> }
> >>>
> >>> - if (pitch % format->cpp[0]) {
> >>> + if (pitch % info->cpp[0]) {
> >>> dev_dbg(dev->dev,
> >>> "buffer pitch (%u bytes) is not a multiple of pixel size (%u bytes)\n",
> >>> - pitch, format->cpp[0]);
> >>> + pitch, info->cpp[0]);
> >>> ret = -EINVAL;
> >>> goto fail;
> >>> }
> >>>
> >>> - for (i = 0; i < format->num_planes; i++) {
> >>> + for (i = 0; i < info->num_planes; i++) {
> >>> struct plane *plane = &omap_fb->planes[i];
> >>> - unsigned int vsub = i == 0 ? 1 : format->vsub;
> >>> + unsigned int vsub = i == 0 ? 1 : info->vsub;
> >>> unsigned int size;
> >>>
> >>> size = pitch * mode_cmd->height / vsub;
> >>> @@ -440,7 +437,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>> plane->dma_addr = 0;
> >>> }
> >>>
> >>> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> >>> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
> >>
> >> Isn't the fix really a one-liner, just passing "format" here instead of
> >> NULL?
> >
> > That would fix the issue of fb->format being initialized to NULL yes.
> >
> > However, I think the change should be in sync with the conversion of the
> > rest of the drivers in patchset [1]. IOW, what this patch is meant to
> > fix is that [1] missed converting the OMAP driver similarly to the other
> > drivers.
> >
> > I think the change is equivalent to the above one-liner you suggest with
> > the following differences:
> >
> > - omap_framebuffer_init() uses the drm_format_info passed down from either
> > drm_internal_framebuffer_create(), or omap_fbdev_driver_fbdev_probe().
> > In both cases the passed down format info matches what
> > omap_framebuffer_init() would look up again.
> >
> > - Skip the format == NULL check. format can't be NULL in any case, since
> > that would have emitted a WARN already in drm_get_format_info() ->
> > drm_format_info().
> >
> > [1] https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
>
> Yep, I have no issue with the patch as such. But if it's a fix, going
> into an rc, I think it's better if it's as small as possible, and do the
> cleanups/refactorings as a non-fix later.
>
> The patch description here sounds more like it's just refactoring the
> code a bit, but if I understand correctly, it's fixing an issue that
> cause a WARN?
Yes, the patch description should've mentioned that it fixes the
!fb->format WARN in drm_framebuffer_init() and the resulting failure to
create the framebuffer for fbdev and other FB users. I will add this.
> So... Either we could 1) split the patch, have the WARN fix as a fix
> patch to the current rc, and the rest later in the next merge window, or
> 2) if you want to keep this as a single patch, I think it makes sense to
> change the subject and description to highlight the fix aspect.
Yes, the patch should go to 6.17-rc1, but I would prefer 2) above, since
patchset [1] requiring it was also added in the same -rc1 and this patch
has been also tested at least by the bug reporter.
> I think my comments apply to all patches in this series, as they're
> essentially doing the same thing...
I can also amend the commit log for the other patches according to the
above.
> Tomi
>
> >> Tomi
> >>
> >>>
> >>> ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
> >>> if (ret) {
> >>> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.h b/drivers/gpu/drm/omapdrm/omap_fb.h
> >>> index 0873f953cf1d1..e6010302a22bd 100644
> >>> --- a/drivers/gpu/drm/omapdrm/omap_fb.h
> >>> +++ b/drivers/gpu/drm/omapdrm/omap_fb.h
> >>> @@ -13,6 +13,7 @@ struct drm_connector;
> >>> struct drm_device;
> >>> struct drm_file;
> >>> struct drm_framebuffer;
> >>> +struct drm_format_info;
> >>> struct drm_gem_object;
> >>> struct drm_mode_fb_cmd2;
> >>> struct drm_plane_state;
> >>> @@ -23,6 +24,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> >>> struct drm_file *file, const struct drm_format_info *info,
> >>> const struct drm_mode_fb_cmd2 *mode_cmd);
> >>> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>> + const struct drm_format_info *info,
> >>> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos);
> >>> int omap_framebuffer_pin(struct drm_framebuffer *fb);
> >>> void omap_framebuffer_unpin(struct drm_framebuffer *fb);
> >>> diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> >>> index 7b63968906817..948af7ec1130b 100644
> >>> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> >>> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> >>> @@ -197,7 +197,10 @@ int omap_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> >>> goto fail;
> >>> }
> >>>
> >>> - fb = omap_framebuffer_init(dev, &mode_cmd, &bo);
> >>> + fb = omap_framebuffer_init(dev,
> >>> + drm_get_format_info(dev, mode_cmd.pixel_format,
> >>> + mode_cmd.modifier[0]),
> >>> + &mode_cmd, &bo);
> >>> if (IS_ERR(fb)) {
> >>> dev_err(dev->dev, "failed to allocate fb\n");
> >>> /* note: if fb creation failed, we can't rely on fb destroy
> >>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-05 15:43 ` Tomi Valkeinen
2025-08-05 16:22 ` Imre Deak
@ 2025-08-05 16:24 ` Mark Brown
1 sibling, 0 replies; 23+ messages in thread
From: Mark Brown @ 2025-08-05 16:24 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: imre.deak, dri-devel, Ville Syrjälä, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard
[-- Attachment #1: Type: text/plain, Size: 950 bytes --]
On Tue, Aug 05, 2025 at 06:43:04PM +0300, Tomi Valkeinen wrote:
> The patch description here sounds more like it's just refactoring the
> code a bit, but if I understand correctly, it's fixing an issue that
> cause a WARN?
Yes, this has been causing a regression in my testing since before the
merge window. I originally reported the issue on the 22nd, the week
before the merge window.
> So... Either we could 1) split the patch, have the WARN fix as a fix
> patch to the current rc, and the rest later in the next merge window, or
> 2) if you want to keep this as a single patch, I think it makes sense to
> change the subject and description to highlight the fix aspect.
The offending code had only been in -next since the 17th (a week and a
half before the merge window) so I'm not sure there's huge concerns
about stability TBH. A this point I'm more concerned that we're going
to get a -rc1 with breakage.
with brea
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-05 16:22 ` Imre Deak
@ 2025-08-05 16:56 ` Tomi Valkeinen
2025-08-05 18:04 ` Imre Deak
0 siblings, 1 reply; 23+ messages in thread
From: Tomi Valkeinen @ 2025-08-05 16:56 UTC (permalink / raw)
To: imre.deak
Cc: dri-devel, Ville Syrjälä, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, Mark Brown
Hi,
On 05/08/2025 19:22, Imre Deak wrote:
> On Tue, Aug 05, 2025 at 06:43:04PM +0300, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 05/08/2025 17:49, Imre Deak wrote:
>>> Hi Tomi,
>>>
>>> On Tue, Aug 05, 2025 at 02:53:36PM +0300, Tomi Valkeinen wrote:
>>>> Hi Imre,
>>>>
>>>> On 28/07/2025 13:16, Imre Deak wrote:
>>>>> Plumb the format info from .fb_create() all the way to
>>>>> drm_helper_mode_fill_fb_struct() to avoid the redundant
>>>>> lookup.
>>>>>
>>>>> For the fbdev case a manual drm_get_format_info() lookup
>>>>> is needed.
>>>>>
>>>>> The patch is based on the driver parts of the patchset at Link:
>>>>> below, which missed converting the omap driver.
>>>>>
>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>> Cc: Maxime Ripard <mripard@kernel.org>
>>>>> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
>>>>> Reported-by: Mark Brown <broonie@kernel.org>
>>>>> Closes: https://lore.kernel.org/all/98b3a62c-91ff-4f91-a58b-e1265f84180b@sirena.org.uk
>>>>> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
>>>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
>>>>> drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
>>>>> drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
>>>>> 3 files changed, 16 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
>>>>> index 30c81e2e5d6b3..bb3105556f193 100644
>>>>> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
>>>>> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
>>>>> @@ -351,7 +351,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
>>>>> }
>>>>> }
>>>>>
>>>>> - fb = omap_framebuffer_init(dev, mode_cmd, bos);
>>>>> + fb = omap_framebuffer_init(dev, info, mode_cmd, bos);
>>>>> if (IS_ERR(fb))
>>>>> goto error;
>>>>>
>>>>> @@ -365,9 +365,9 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
>>>>> }
>>>>>
>>>>> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>>>> + const struct drm_format_info *info,
>>>>> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
>>>>> {
>>>>> - const struct drm_format_info *format = NULL;
>>>>> struct omap_framebuffer *omap_fb = NULL;
>>>>> struct drm_framebuffer *fb = NULL;
>>>>> unsigned int pitch = mode_cmd->pitches[0];
>>>>> @@ -377,15 +377,12 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>>>> dev, mode_cmd, mode_cmd->width, mode_cmd->height,
>>>>> (char *)&mode_cmd->pixel_format);
>>>>>
>>>>> - format = drm_get_format_info(dev, mode_cmd->pixel_format,
>>>>> - mode_cmd->modifier[0]);
>>>>> -
>>>>> for (i = 0; i < ARRAY_SIZE(formats); i++) {
>>>>> if (formats[i] == mode_cmd->pixel_format)
>>>>> break;
>>>>> }
>>>>>
>>>>> - if (!format || i == ARRAY_SIZE(formats)) {
>>>>> + if (i == ARRAY_SIZE(formats)) {
>>>>> dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
>>>>> (char *)&mode_cmd->pixel_format);
>>>>> ret = -EINVAL;
>>>>> @@ -399,7 +396,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>>>> }
>>>>>
>>>>> fb = &omap_fb->base;
>>>>> - omap_fb->format = format;
>>>>> + omap_fb->format = info;
>>>>> mutex_init(&omap_fb->lock);
>>>>>
>>>>> /*
>>>>> @@ -407,23 +404,23 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>>>> * that the two planes of multiplane formats need the same number of
>>>>> * bytes per pixel.
>>>>> */
>>>>> - if (format->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
>>>>> + if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
>>>>> dev_dbg(dev->dev, "pitches differ between planes 0 and 1\n");
>>>>> ret = -EINVAL;
>>>>> goto fail;
>>>>> }
>>>>>
>>>>> - if (pitch % format->cpp[0]) {
>>>>> + if (pitch % info->cpp[0]) {
>>>>> dev_dbg(dev->dev,
>>>>> "buffer pitch (%u bytes) is not a multiple of pixel size (%u bytes)\n",
>>>>> - pitch, format->cpp[0]);
>>>>> + pitch, info->cpp[0]);
>>>>> ret = -EINVAL;
>>>>> goto fail;
>>>>> }
>>>>>
>>>>> - for (i = 0; i < format->num_planes; i++) {
>>>>> + for (i = 0; i < info->num_planes; i++) {
>>>>> struct plane *plane = &omap_fb->planes[i];
>>>>> - unsigned int vsub = i == 0 ? 1 : format->vsub;
>>>>> + unsigned int vsub = i == 0 ? 1 : info->vsub;
>>>>> unsigned int size;
>>>>>
>>>>> size = pitch * mode_cmd->height / vsub;
>>>>> @@ -440,7 +437,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>>>>> plane->dma_addr = 0;
>>>>> }
>>>>>
>>>>> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
>>>>> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
>>>>
>>>> Isn't the fix really a one-liner, just passing "format" here instead of
>>>> NULL?
>>>
>>> That would fix the issue of fb->format being initialized to NULL yes.
>>>
>>> However, I think the change should be in sync with the conversion of the
>>> rest of the drivers in patchset [1]. IOW, what this patch is meant to
>>> fix is that [1] missed converting the OMAP driver similarly to the other
>>> drivers.
>>>
>>> I think the change is equivalent to the above one-liner you suggest with
>>> the following differences:
>>>
>>> - omap_framebuffer_init() uses the drm_format_info passed down from either
>>> drm_internal_framebuffer_create(), or omap_fbdev_driver_fbdev_probe().
>>> In both cases the passed down format info matches what
>>> omap_framebuffer_init() would look up again.
>>>
>>> - Skip the format == NULL check. format can't be NULL in any case, since
>>> that would have emitted a WARN already in drm_get_format_info() ->
>>> drm_format_info().
>>>
>>> [1] https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
>>
>> Yep, I have no issue with the patch as such. But if it's a fix, going
>> into an rc, I think it's better if it's as small as possible, and do the
>> cleanups/refactorings as a non-fix later.
>>
>> The patch description here sounds more like it's just refactoring the
>> code a bit, but if I understand correctly, it's fixing an issue that
>> cause a WARN?
>
> Yes, the patch description should've mentioned that it fixes the
> !fb->format WARN in drm_framebuffer_init() and the resulting failure to
> create the framebuffer for fbdev and other FB users. I will add this.
>
>> So... Either we could 1) split the patch, have the WARN fix as a fix
>> patch to the current rc, and the rest later in the next merge window, or
>> 2) if you want to keep this as a single patch, I think it makes sense to
>> change the subject and description to highlight the fix aspect.
>
> Yes, the patch should go to 6.17-rc1, but I would prefer 2) above, since
> patchset [1] requiring it was also added in the same -rc1 and this patch
> has been also tested at least by the bug reporter.
>
>> I think my comments apply to all patches in this series, as they're
>> essentially doing the same thing...
>
> I can also amend the commit log for the other patches according to the
> above.
Alright. In any case, I don't think any of my concerns are big, but I
wouldn't mind a clarification in the description. If you'll be pushing
all these together:
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
If you want me to merge the omap one separately, just say so.
Tomi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-05 16:56 ` Tomi Valkeinen
@ 2025-08-05 18:04 ` Imre Deak
0 siblings, 0 replies; 23+ messages in thread
From: Imre Deak @ 2025-08-05 18:04 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: dri-devel, Ville Syrjälä, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, Mark Brown
On Tue, Aug 05, 2025 at 07:56:50PM +0300, Tomi Valkeinen wrote:
> Hi,
>
> On 05/08/2025 19:22, Imre Deak wrote:
> > On Tue, Aug 05, 2025 at 06:43:04PM +0300, Tomi Valkeinen wrote:
> >> Hi,
> >>
> >> On 05/08/2025 17:49, Imre Deak wrote:
> >>> Hi Tomi,
> >>>
> >>> On Tue, Aug 05, 2025 at 02:53:36PM +0300, Tomi Valkeinen wrote:
> >>>> Hi Imre,
> >>>>
> >>>> On 28/07/2025 13:16, Imre Deak wrote:
> >>>>> Plumb the format info from .fb_create() all the way to
> >>>>> drm_helper_mode_fill_fb_struct() to avoid the redundant
> >>>>> lookup.
> >>>>>
> >>>>> For the fbdev case a manual drm_get_format_info() lookup
> >>>>> is needed.
> >>>>>
> >>>>> The patch is based on the driver parts of the patchset at Link:
> >>>>> below, which missed converting the omap driver.
> >>>>>
> >>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>>> Cc: Maxime Ripard <mripard@kernel.org>
> >>>>> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> >>>>> Reported-by: Mark Brown <broonie@kernel.org>
> >>>>> Closes: https://lore.kernel.org/all/98b3a62c-91ff-4f91-a58b-e1265f84180b@sirena.org.uk
> >>>>> Link: https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> >>>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
> >>>>> ---
> >>>>> drivers/gpu/drm/omapdrm/omap_fb.c | 23 ++++++++++-------------
> >>>>> drivers/gpu/drm/omapdrm/omap_fb.h | 2 ++
> >>>>> drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++++-
> >>>>> 3 files changed, 16 insertions(+), 14 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> >>>>> index 30c81e2e5d6b3..bb3105556f193 100644
> >>>>> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> >>>>> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> >>>>> @@ -351,7 +351,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> >>>>> }
> >>>>> }
> >>>>>
> >>>>> - fb = omap_framebuffer_init(dev, mode_cmd, bos);
> >>>>> + fb = omap_framebuffer_init(dev, info, mode_cmd, bos);
> >>>>> if (IS_ERR(fb))
> >>>>> goto error;
> >>>>>
> >>>>> @@ -365,9 +365,9 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
> >>>>> }
> >>>>>
> >>>>> struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>>>> + const struct drm_format_info *info,
> >>>>> const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
> >>>>> {
> >>>>> - const struct drm_format_info *format = NULL;
> >>>>> struct omap_framebuffer *omap_fb = NULL;
> >>>>> struct drm_framebuffer *fb = NULL;
> >>>>> unsigned int pitch = mode_cmd->pitches[0];
> >>>>> @@ -377,15 +377,12 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>>>> dev, mode_cmd, mode_cmd->width, mode_cmd->height,
> >>>>> (char *)&mode_cmd->pixel_format);
> >>>>>
> >>>>> - format = drm_get_format_info(dev, mode_cmd->pixel_format,
> >>>>> - mode_cmd->modifier[0]);
> >>>>> -
> >>>>> for (i = 0; i < ARRAY_SIZE(formats); i++) {
> >>>>> if (formats[i] == mode_cmd->pixel_format)
> >>>>> break;
> >>>>> }
> >>>>>
> >>>>> - if (!format || i == ARRAY_SIZE(formats)) {
> >>>>> + if (i == ARRAY_SIZE(formats)) {
> >>>>> dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
> >>>>> (char *)&mode_cmd->pixel_format);
> >>>>> ret = -EINVAL;
> >>>>> @@ -399,7 +396,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>>>> }
> >>>>>
> >>>>> fb = &omap_fb->base;
> >>>>> - omap_fb->format = format;
> >>>>> + omap_fb->format = info;
> >>>>> mutex_init(&omap_fb->lock);
> >>>>>
> >>>>> /*
> >>>>> @@ -407,23 +404,23 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>>>> * that the two planes of multiplane formats need the same number of
> >>>>> * bytes per pixel.
> >>>>> */
> >>>>> - if (format->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> >>>>> + if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
> >>>>> dev_dbg(dev->dev, "pitches differ between planes 0 and 1\n");
> >>>>> ret = -EINVAL;
> >>>>> goto fail;
> >>>>> }
> >>>>>
> >>>>> - if (pitch % format->cpp[0]) {
> >>>>> + if (pitch % info->cpp[0]) {
> >>>>> dev_dbg(dev->dev,
> >>>>> "buffer pitch (%u bytes) is not a multiple of pixel size (%u bytes)\n",
> >>>>> - pitch, format->cpp[0]);
> >>>>> + pitch, info->cpp[0]);
> >>>>> ret = -EINVAL;
> >>>>> goto fail;
> >>>>> }
> >>>>>
> >>>>> - for (i = 0; i < format->num_planes; i++) {
> >>>>> + for (i = 0; i < info->num_planes; i++) {
> >>>>> struct plane *plane = &omap_fb->planes[i];
> >>>>> - unsigned int vsub = i == 0 ? 1 : format->vsub;
> >>>>> + unsigned int vsub = i == 0 ? 1 : info->vsub;
> >>>>> unsigned int size;
> >>>>>
> >>>>> size = pitch * mode_cmd->height / vsub;
> >>>>> @@ -440,7 +437,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
> >>>>> plane->dma_addr = 0;
> >>>>> }
> >>>>>
> >>>>> - drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> >>>>> + drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
> >>>>
> >>>> Isn't the fix really a one-liner, just passing "format" here instead of
> >>>> NULL?
> >>>
> >>> That would fix the issue of fb->format being initialized to NULL yes.
> >>>
> >>> However, I think the change should be in sync with the conversion of the
> >>> rest of the drivers in patchset [1]. IOW, what this patch is meant to
> >>> fix is that [1] missed converting the OMAP driver similarly to the other
> >>> drivers.
> >>>
> >>> I think the change is equivalent to the above one-liner you suggest with
> >>> the following differences:
> >>>
> >>> - omap_framebuffer_init() uses the drm_format_info passed down from either
> >>> drm_internal_framebuffer_create(), or omap_fbdev_driver_fbdev_probe().
> >>> In both cases the passed down format info matches what
> >>> omap_framebuffer_init() would look up again.
> >>>
> >>> - Skip the format == NULL check. format can't be NULL in any case, since
> >>> that would have emitted a WARN already in drm_get_format_info() ->
> >>> drm_format_info().
> >>>
> >>> [1] https://lore.kernel.org/all/20250701090722.13645-1-ville.syrjala@linux.intel.com
> >>
> >> Yep, I have no issue with the patch as such. But if it's a fix, going
> >> into an rc, I think it's better if it's as small as possible, and do the
> >> cleanups/refactorings as a non-fix later.
> >>
> >> The patch description here sounds more like it's just refactoring the
> >> code a bit, but if I understand correctly, it's fixing an issue that
> >> cause a WARN?
> >
> > Yes, the patch description should've mentioned that it fixes the
> > !fb->format WARN in drm_framebuffer_init() and the resulting failure to
> > create the framebuffer for fbdev and other FB users. I will add this.
> >
> >> So... Either we could 1) split the patch, have the WARN fix as a fix
> >> patch to the current rc, and the rest later in the next merge window, or
> >> 2) if you want to keep this as a single patch, I think it makes sense to
> >> change the subject and description to highlight the fix aspect.
> >
> > Yes, the patch should go to 6.17-rc1, but I would prefer 2) above, since
> > patchset [1] requiring it was also added in the same -rc1 and this patch
> > has been also tested at least by the bug reporter.
> >
> >> I think my comments apply to all patches in this series, as they're
> >> essentially doing the same thing...
> >
> > I can also amend the commit log for the other patches according to the
> > above.
>
> Alright. In any case, I don't think any of my concerns are big, but I
> wouldn't mind a clarification in the description. If you'll be pushing
> all these together:
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Thanks.
> If you want me to merge the omap one separately, just say so.
I sent now the updated [1], which can be merged to drm-misc-next-fixes,
but I'm not sure if there's still a pull request to v6.17-rc1 from that.
So waiting now a suggestion on that.
[1] https://lore.kernel.org/all/20250805175752.690504-1-imre.deak@intel.com
>
> Tomi
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-01 9:08 ` Danilo Krummrich
@ 2025-08-12 17:04 ` Danilo Krummrich
2025-08-12 17:16 ` Imre Deak
0 siblings, 1 reply; 23+ messages in thread
From: Danilo Krummrich @ 2025-08-12 17:04 UTC (permalink / raw)
To: Imre Deak
Cc: dri-devel, Ville Syrjälä, Lyude Paul, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard
On 8/1/25 11:08 AM, Danilo Krummrich wrote:
> On 7/28/25 12:16 PM, Imre Deak wrote:
>> Plumb the format info from .fb_create() all the way to
>> drm_helper_mode_fill_fb_struct() to avoid the redundant
>> lookup.
>>
>> The patch is based on the driver parts of the patchset at Link:
>> below, which missed converting the nouveau driver.
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: Danilo Krummrich <dakr@kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Fixes: 41ab92d35ccd ("drm: Make passing of format info to
>> drm_helper_mode_fill_fb_struct() mandatory")
>> Link: https://lore.kernel.org/all/20250701090722.13645-1-
>> ville.syrjala@linux.intel.com
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>
> Acked-by: Danilo Krummrich <dakr@kernel.org>
Any concerns if I just pick this one? It's a bit annoying to not have this fixed
in drm-misc-fixes and -next. I'd hope for -rc2 getting backmerged.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-12 17:04 ` Danilo Krummrich
@ 2025-08-12 17:16 ` Imre Deak
2025-08-12 17:24 ` Danilo Krummrich
0 siblings, 1 reply; 23+ messages in thread
From: Imre Deak @ 2025-08-12 17:16 UTC (permalink / raw)
To: Danilo Krummrich, Thomas Zimmermann
Cc: dri-devel, Ville Syrjälä, Lyude Paul, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard
On Tue, Aug 12, 2025 at 07:04:44PM +0200, Danilo Krummrich wrote:
> On 8/1/25 11:08 AM, Danilo Krummrich wrote:
> > On 7/28/25 12:16 PM, Imre Deak wrote:
> > > Plumb the format info from .fb_create() all the way to
> > > drm_helper_mode_fill_fb_struct() to avoid the redundant
> > > lookup.
> > >
> > > The patch is based on the driver parts of the patchset at Link:
> > > below, which missed converting the nouveau driver.
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Cc: Danilo Krummrich <dakr@kernel.org>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Fixes: 41ab92d35ccd ("drm: Make passing of format info to
> > > drm_helper_mode_fill_fb_struct() mandatory")
> > > Link: https://lore.kernel.org/all/20250701090722.13645-1-
> > > ville.syrjala@linux.intel.com
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > Acked-by: Danilo Krummrich <dakr@kernel.org>
>
> Any concerns if I just pick this one? It's a bit annoying to not have this fixed
> in drm-misc-fixes and -next. I'd hope for -rc2 getting backmerged.
Yes, this got delayed and didn't get to -rc1, maybe I could've merged
the fixes already reviewed earlier.
The fix is in drm-misc-next-fixes atm and Thomas has sent already
https://lore.kernel.org/all/20250812064712.GA14554@2a02-2454-fd5e-fd00-2c49-c639-c55f-a125.dyn6.pyur.net
which should get pulled to drm-fixes and then to -rc2, AFAIU, not sure
if that's enough for you. I let the maintainers comment about
cherry-picking it if not, I suppose the concern could be the duplicated
commits that Linus would see.
--Imre
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-12 17:16 ` Imre Deak
@ 2025-08-12 17:24 ` Danilo Krummrich
2025-08-13 7:25 ` Thomas Zimmermann
0 siblings, 1 reply; 23+ messages in thread
From: Danilo Krummrich @ 2025-08-12 17:24 UTC (permalink / raw)
To: imre.deak
Cc: Thomas Zimmermann, dri-devel, Ville Syrjälä, Lyude Paul,
Maarten Lankhorst, Maxime Ripard
On 8/12/25 7:16 PM, Imre Deak wrote:
> The fix is in drm-misc-next-fixes atm and Thomas has sent already
>
> https://lore.kernel.org/all/20250812064712.GA14554@2a02-2454-fd5e-fd00-2c49-c639-c55f-a125.dyn6.pyur.net
Ok, that's great! (No further action needed AFAIC. :)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-12 17:24 ` Danilo Krummrich
@ 2025-08-13 7:25 ` Thomas Zimmermann
2025-08-13 8:47 ` Danilo Krummrich
0 siblings, 1 reply; 23+ messages in thread
From: Thomas Zimmermann @ 2025-08-13 7:25 UTC (permalink / raw)
To: Danilo Krummrich, imre.deak
Cc: dri-devel, Ville Syrjälä, Lyude Paul, Maarten Lankhorst,
Maxime Ripard
Hi
Am 12.08.25 um 19:24 schrieb Danilo Krummrich:
> On 8/12/25 7:16 PM, Imre Deak wrote:
>> The fix is in drm-misc-next-fixes atm and Thomas has sent already
>>
>> https://lore.kernel.org/all/20250812064712.GA14554@2a02-2454-fd5e-fd00-2c49-c639-c55f-a125.dyn6.pyur.net
>>
>
> Ok, that's great! (No further action needed AFAIC. :)
Please don't cherry-pick across trees unless absolutely necessary. It's
a hassle for later backporting and merging.
Best regards
Thomas
--
--
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)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] drm/nouveau: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct()
2025-08-13 7:25 ` Thomas Zimmermann
@ 2025-08-13 8:47 ` Danilo Krummrich
0 siblings, 0 replies; 23+ messages in thread
From: Danilo Krummrich @ 2025-08-13 8:47 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: imre.deak, dri-devel, Ville Syrjälä, Lyude Paul,
Maarten Lankhorst, Maxime Ripard
On Wed Aug 13, 2025 at 9:25 AM CEST, Thomas Zimmermann wrote:
> Hi
>
> Am 12.08.25 um 19:24 schrieb Danilo Krummrich:
>> On 8/12/25 7:16 PM, Imre Deak wrote:
>>> The fix is in drm-misc-next-fixes atm and Thomas has sent already
>>>
>>> https://lore.kernel.org/all/20250812064712.GA14554@2a02-2454-fd5e-fd00-2c49-c639-c55f-a125.dyn6.pyur.net
>>>
>>
>> Ok, that's great! (No further action needed AFAIC. :)
>
> Please don't cherry-pick across trees unless absolutely necessary. It's
> a hassle for later backporting and merging.
No worries, maintaining a couple trees myself I'm not willing to contribute to
such kind of mess. :)
I didn't know if this patch was picked up somewhere else already (since I was
asked for an ACK previously). Hence, I asked if this has happened and if not, I'd
have taken it thorugh -fixes myself, ideally getting the corresponding -rc
backmerged into -next.
Thanks,
Danilo
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-08-13 8:47 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-28 10:16 [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer Imre Deak
2025-07-28 10:16 ` [PATCH 1/3] drm/omap: Pass along the format info from .fb_create() to drm_helper_mode_fill_fb_struct() Imre Deak
2025-07-29 15:03 ` Mark Brown
2025-07-30 11:16 ` Imre Deak
2025-08-05 11:53 ` Tomi Valkeinen
2025-08-05 14:49 ` Imre Deak
2025-08-05 15:43 ` Tomi Valkeinen
2025-08-05 16:22 ` Imre Deak
2025-08-05 16:56 ` Tomi Valkeinen
2025-08-05 18:04 ` Imre Deak
2025-08-05 16:24 ` Mark Brown
2025-07-28 10:16 ` [PATCH 2/3] drm/nouveau: " Imre Deak
2025-07-30 11:21 ` Imre Deak
2025-08-01 9:08 ` Danilo Krummrich
2025-08-12 17:04 ` Danilo Krummrich
2025-08-12 17:16 ` Imre Deak
2025-08-12 17:24 ` Danilo Krummrich
2025-08-13 7:25 ` Thomas Zimmermann
2025-08-13 8:47 ` Danilo Krummrich
2025-08-01 20:28 ` James Jones
2025-07-28 10:16 ` [PATCH 3/3] drm/radeon: " Imre Deak
2025-07-28 18:42 ` Deucher, Alexander
2025-07-31 18:17 ` [PATCH 0/3] drm: Fix initializing format info in drm_framebuffer Naresh Kamboju
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).