linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/nouveau: Pass along the format info from .fb_create() nouveau_framebuffer_new()
@ 2025-07-31 23:41 James Jones
  2025-08-01  2:28 ` Imre Deak
  0 siblings, 1 reply; 4+ messages in thread
From: James Jones @ 2025-07-31 23:41 UTC (permalink / raw)
  To: Danilo Krummrich, Lyude Paul
  Cc: nouveau, dri-devel, linux-kernel, David Airlie, Simona Vetter,
	James Jones, Ville Syrjälä

Plumb the format info from .fb_create() all the way to
nouveau_framebuffer_new() to avoid the redundant lookup.
Also plumb the format info from there down to
drm_helper_mode_fill_fb_struct() as required, avoiding
a WARN_ON() and failure every time this path is used,
e.g., during fbdev init.

Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
Signed-off-by: James Jones <jajones@nvidia.com>
CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++------
 drivers/gpu/drm/nouveau/nouveau_display.h | 1 +
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index e1e542126310..805d0a87aa54 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 e45f211501f6..d569240df354 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -10,6 +10,7 @@
 
 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.50.1


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

* Re: [PATCH] drm/nouveau: Pass along the format info from .fb_create() nouveau_framebuffer_new()
  2025-07-31 23:41 [PATCH] drm/nouveau: Pass along the format info from .fb_create() nouveau_framebuffer_new() James Jones
@ 2025-08-01  2:28 ` Imre Deak
  2025-08-01 20:30   ` James Jones
  0 siblings, 1 reply; 4+ messages in thread
From: Imre Deak @ 2025-08-01  2:28 UTC (permalink / raw)
  To: James Jones
  Cc: Danilo Krummrich, Lyude Paul, nouveau, dri-devel, linux-kernel,
	David Airlie, Simona Vetter, Ville Syrjälä

On Thu, Jul 31, 2025 at 04:41:04PM -0700, James Jones wrote:
> Plumb the format info from .fb_create() all the way to
> nouveau_framebuffer_new() to avoid the redundant lookup.
> Also plumb the format info from there down to
> drm_helper_mode_fill_fb_struct() as required, avoiding
> a WARN_ON() and failure every time this path is used,
> e.g., during fbdev init.
> 
> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> Signed-off-by: James Jones <jajones@nvidia.com>
> CC: Ville Syrjälä <ville.syrjala@linux.intel.com>

I posted this change already:
https://lore.kernel.org/all/20250728101603.243788-3-imre.deak@intel.com

> ---
>  drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++------
>  drivers/gpu/drm/nouveau/nouveau_display.h | 1 +
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index e1e542126310..805d0a87aa54 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 e45f211501f6..d569240df354 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -10,6 +10,7 @@
>  
>  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.50.1
> 

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

* Re: [PATCH] drm/nouveau: Pass along the format info from .fb_create() nouveau_framebuffer_new()
  2025-08-01  2:28 ` Imre Deak
@ 2025-08-01 20:30   ` James Jones
  2025-08-02 14:24     ` Imre Deak
  0 siblings, 1 reply; 4+ messages in thread
From: James Jones @ 2025-08-01 20:30 UTC (permalink / raw)
  To: imre.deak
  Cc: Danilo Krummrich, Lyude Paul, nouveau, dri-devel, linux-kernel,
	David Airlie, Simona Vetter, Ville Syrjälä

Apologies, I saw your changes for another chipset, but missed the 
nouveau part. I've responded to your thread now. Thanks for making the fix!

-James

On 7/31/25 19:28, Imre Deak wrote:
> On Thu, Jul 31, 2025 at 04:41:04PM -0700, James Jones wrote:
>> Plumb the format info from .fb_create() all the way to
>> nouveau_framebuffer_new() to avoid the redundant lookup.
>> Also plumb the format info from there down to
>> drm_helper_mode_fill_fb_struct() as required, avoiding
>> a WARN_ON() and failure every time this path is used,
>> e.g., during fbdev init.
>>
>> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
>> Signed-off-by: James Jones <jajones@nvidia.com>
>> CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I posted this change already:
> https://lore.kernel.org/all/20250728101603.243788-3-imre.deak@intel.com
> 
>> ---
>>   drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++------
>>   drivers/gpu/drm/nouveau/nouveau_display.h | 1 +
>>   2 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
>> index e1e542126310..805d0a87aa54 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 e45f211501f6..d569240df354 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
>> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
>> @@ -10,6 +10,7 @@
>>   
>>   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.50.1
>>


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

* Re: [PATCH] drm/nouveau: Pass along the format info from .fb_create() nouveau_framebuffer_new()
  2025-08-01 20:30   ` James Jones
@ 2025-08-02 14:24     ` Imre Deak
  0 siblings, 0 replies; 4+ messages in thread
From: Imre Deak @ 2025-08-02 14:24 UTC (permalink / raw)
  To: James Jones
  Cc: Danilo Krummrich, Lyude Paul, nouveau, dri-devel, linux-kernel,
	David Airlie, Simona Vetter, Ville Syrjälä

On Fri, Aug 01, 2025 at 01:30:02PM -0700, James Jones wrote:
> Apologies, I saw your changes for another chipset, but missed the nouveau
> part.

Ok, no problem, thanks for testing it and the review. The omap patch is
still missing an ack/review, the patchset can be merged after an
omap maintainer can provide that.

> I've responded to your thread now. Thanks for making the fix!
> 
> -James
> 
> On 7/31/25 19:28, Imre Deak wrote:
> > On Thu, Jul 31, 2025 at 04:41:04PM -0700, James Jones wrote:
> > > Plumb the format info from .fb_create() all the way to
> > > nouveau_framebuffer_new() to avoid the redundant lookup.
> > > Also plumb the format info from there down to
> > > drm_helper_mode_fill_fb_struct() as required, avoiding
> > > a WARN_ON() and failure every time this path is used,
> > > e.g., during fbdev init.
> > > 
> > > Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> > > Signed-off-by: James Jones <jajones@nvidia.com>
> > > CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I posted this change already:
> > https://lore.kernel.org/all/20250728101603.243788-3-imre.deak@intel.com
> > 
> > > ---
> > >   drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++------
> > >   drivers/gpu/drm/nouveau/nouveau_display.h | 1 +
> > >   2 files changed, 4 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> > > index e1e542126310..805d0a87aa54 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 e45f211501f6..d569240df354 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> > > @@ -10,6 +10,7 @@
> > >   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.50.1
> > > 
> 

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

end of thread, other threads:[~2025-08-02 14:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-31 23:41 [PATCH] drm/nouveau: Pass along the format info from .fb_create() nouveau_framebuffer_new() James Jones
2025-08-01  2:28 ` Imre Deak
2025-08-01 20:30   ` James Jones
2025-08-02 14:24     ` Imre Deak

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