diff mbox series

[02/13] drm/fb-helper: don't preserve fb_ops across deferred IO use

Message ID e2b1685703878612093a37189d37685303fa15e9.1574871797.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series video, drm: constify fbops in struct fb_info | expand

Commit Message

Jani Nikula Nov. 27, 2019, 4:31 p.m. UTC 
Deferred IO now preserves the fb_ops.

Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

Comments

Daniel Vetter Nov. 27, 2019, 6:18 p.m. UTC | #1 
On Wed, Nov 27, 2019 at 06:31:58PM +0200, Jani Nikula wrote:
> Deferred IO now preserves the fb_ops.
> 
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0c088ea70ad0..a5a2a538d085 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1954,7 +1954,6 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user)
>  static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>  {
>  	struct fb_info *fbi = fb_helper->fbdev;
> -	struct fb_ops *fbops = NULL;
>  	void *shadow = NULL;
>  
>  	if (!fb_helper->dev)
> @@ -1963,15 +1962,11 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>  	if (fbi && fbi->fbdefio) {
>  		fb_deferred_io_cleanup(fbi);
>  		shadow = fbi->screen_buffer;
> -		fbops = fbi->fbops;
>  	}
>  
>  	drm_fb_helper_fini(fb_helper);
>  
> -	if (shadow) {
> -		vfree(shadow);
> -		kfree(fbops);
> -	}
> +	vfree(shadow);
>  
>  	drm_client_framebuffer_delete(fb_helper->buffer);
>  }
> @@ -2062,23 +2057,14 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>  	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
>  
>  	if (drm_fbdev_use_shadow_fb(fb_helper)) {
> -		struct fb_ops *fbops;
>  		void *shadow;
>  
> -		/*
> -		 * fb_deferred_io_cleanup() clears &fbops->fb_mmap so a per
> -		 * instance version is necessary.
> -		 */
> -		fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>  		shadow = vzalloc(fbi->screen_size);
> -		if (!fbops || !shadow) {
> -			kfree(fbops);
> +		if (!shadow) {
>  			vfree(shadow);
>  			return -ENOMEM;
>  		}
>  
> -		*fbops = *fbi->fbops;
> -		fbi->fbops = fbops;
>  		fbi->screen_buffer = shadow;
>  		fbi->fbdefio = &drm_fbdev_defio;
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Noralf Trønnes Nov. 28, 2019, 11:31 a.m. UTC | #2 
Den 27.11.2019 17.31, skrev Jani Nikula:
> Deferred IO now preserves the fb_ops.
> 
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0c088ea70ad0..a5a2a538d085 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1954,7 +1954,6 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user)
>  static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>  {
>  	struct fb_info *fbi = fb_helper->fbdev;
> -	struct fb_ops *fbops = NULL;
>  	void *shadow = NULL;
>  
>  	if (!fb_helper->dev)
> @@ -1963,15 +1962,11 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>  	if (fbi && fbi->fbdefio) {
>  		fb_deferred_io_cleanup(fbi);
>  		shadow = fbi->screen_buffer;
> -		fbops = fbi->fbops;
>  	}
>  
>  	drm_fb_helper_fini(fb_helper);
>  
> -	if (shadow) {
> -		vfree(shadow);
> -		kfree(fbops);
> -	}
> +	vfree(shadow);
>  
>  	drm_client_framebuffer_delete(fb_helper->buffer);
>  }
> @@ -2062,23 +2057,14 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>  	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
>  
>  	if (drm_fbdev_use_shadow_fb(fb_helper)) {
> -		struct fb_ops *fbops;
>  		void *shadow;
>  
> -		/*
> -		 * fb_deferred_io_cleanup() clears &fbops->fb_mmap so a per
> -		 * instance version is necessary.
> -		 */
> -		fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>  		shadow = vzalloc(fbi->screen_size);
> -		if (!fbops || !shadow) {
> -			kfree(fbops);
> +		if (!shadow) {
>  			vfree(shadow);

This vfree can is a no-op now and can be dropped. With that:

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

>  			return -ENOMEM;
>  		}
>  
> -		*fbops = *fbi->fbops;
> -		fbi->fbops = fbops;
>  		fbi->screen_buffer = shadow;
>  		fbi->fbdefio = &drm_fbdev_defio;
>  
>
Jani Nikula Nov. 28, 2019, 12:05 p.m. UTC | #3 
On Thu, 28 Nov 2019, Noralf Trønnes <noralf@tronnes.org> wrote:
> Den 27.11.2019 17.31, skrev Jani Nikula:
>> Deferred IO now preserves the fb_ops.
>> 
>> Cc: Noralf Trønnes <noralf@tronnes.org>
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/drm_fb_helper.c | 18 ++----------------
>>  1 file changed, 2 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 0c088ea70ad0..a5a2a538d085 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1954,7 +1954,6 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user)
>>  static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>>  {
>>  	struct fb_info *fbi = fb_helper->fbdev;
>> -	struct fb_ops *fbops = NULL;
>>  	void *shadow = NULL;
>>  
>>  	if (!fb_helper->dev)
>> @@ -1963,15 +1962,11 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>>  	if (fbi && fbi->fbdefio) {
>>  		fb_deferred_io_cleanup(fbi);
>>  		shadow = fbi->screen_buffer;
>> -		fbops = fbi->fbops;
>>  	}
>>  
>>  	drm_fb_helper_fini(fb_helper);
>>  
>> -	if (shadow) {
>> -		vfree(shadow);
>> -		kfree(fbops);
>> -	}
>> +	vfree(shadow);
>>  
>>  	drm_client_framebuffer_delete(fb_helper->buffer);
>>  }
>> @@ -2062,23 +2057,14 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>>  	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
>>  
>>  	if (drm_fbdev_use_shadow_fb(fb_helper)) {
>> -		struct fb_ops *fbops;
>>  		void *shadow;
>>  
>> -		/*
>> -		 * fb_deferred_io_cleanup() clears &fbops->fb_mmap so a per
>> -		 * instance version is necessary.
>> -		 */
>> -		fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>>  		shadow = vzalloc(fbi->screen_size);
>> -		if (!fbops || !shadow) {
>> -			kfree(fbops);
>> +		if (!shadow) {
>>  			vfree(shadow);
>
> This vfree can is a no-op now and can be dropped. With that:

D'oh!

With that I think I'd also drop the shadow local variable and assign to
fbi->screen_buffer directly. Fine with that?

Thanks.

BR,
Jani.

>
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
>
>>  			return -ENOMEM;
>>  		}
>>  
>> -		*fbops = *fbi->fbops;
>> -		fbi->fbops = fbops;
>>  		fbi->screen_buffer = shadow;
>>  		fbi->fbdefio = &drm_fbdev_defio;
>>  
>> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Noralf Trønnes Nov. 28, 2019, 2:03 p.m. UTC | #4 
Den 28.11.2019 13.05, skrev Jani Nikula:
> On Thu, 28 Nov 2019, Noralf Trønnes <noralf@tronnes.org> wrote:
>> Den 27.11.2019 17.31, skrev Jani Nikula:
>>> Deferred IO now preserves the fb_ops.
>>>
>>> Cc: Noralf Trønnes <noralf@tronnes.org>
>>> Cc: dri-devel@lists.freedesktop.org
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/drm_fb_helper.c | 18 ++----------------
>>>  1 file changed, 2 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>> index 0c088ea70ad0..a5a2a538d085 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -1954,7 +1954,6 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user)
>>>  static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>>>  {
>>>  	struct fb_info *fbi = fb_helper->fbdev;
>>> -	struct fb_ops *fbops = NULL;
>>>  	void *shadow = NULL;
>>>  
>>>  	if (!fb_helper->dev)
>>> @@ -1963,15 +1962,11 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>>>  	if (fbi && fbi->fbdefio) {
>>>  		fb_deferred_io_cleanup(fbi);
>>>  		shadow = fbi->screen_buffer;
>>> -		fbops = fbi->fbops;
>>>  	}
>>>  
>>>  	drm_fb_helper_fini(fb_helper);
>>>  
>>> -	if (shadow) {
>>> -		vfree(shadow);
>>> -		kfree(fbops);
>>> -	}
>>> +	vfree(shadow);
>>>  
>>>  	drm_client_framebuffer_delete(fb_helper->buffer);
>>>  }
>>> @@ -2062,23 +2057,14 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>>>  	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
>>>  
>>>  	if (drm_fbdev_use_shadow_fb(fb_helper)) {
>>> -		struct fb_ops *fbops;
>>>  		void *shadow;
>>>  
>>> -		/*
>>> -		 * fb_deferred_io_cleanup() clears &fbops->fb_mmap so a per
>>> -		 * instance version is necessary.
>>> -		 */
>>> -		fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>>>  		shadow = vzalloc(fbi->screen_size);
>>> -		if (!fbops || !shadow) {
>>> -			kfree(fbops);
>>> +		if (!shadow) {
>>>  			vfree(shadow);
>>
>> This vfree can is a no-op now and can be dropped. With that:
> 
> D'oh!
> 
> With that I think I'd also drop the shadow local variable and assign to
> fbi->screen_buffer directly. Fine with that?

Sure, that's even better.

Noralf.

> 
> Thanks.
> 
> BR,
> Jani.
> 
>>
>> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
>>
>>>  			return -ENOMEM;
>>>  		}
>>>  
>>> -		*fbops = *fbi->fbops;
>>> -		fbi->fbops = fbops;
>>>  		fbi->screen_buffer = shadow;
>>>  		fbi->fbdefio = &drm_fbdev_defio;
>>>  
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0c088ea70ad0..a5a2a538d085 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1954,7 +1954,6 @@  static int drm_fbdev_fb_release(struct fb_info *info, int user)
 static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
 {
 	struct fb_info *fbi = fb_helper->fbdev;
-	struct fb_ops *fbops = NULL;
 	void *shadow = NULL;
 
 	if (!fb_helper->dev)
@@ -1963,15 +1962,11 @@  static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
 	if (fbi && fbi->fbdefio) {
 		fb_deferred_io_cleanup(fbi);
 		shadow = fbi->screen_buffer;
-		fbops = fbi->fbops;
 	}
 
 	drm_fb_helper_fini(fb_helper);
 
-	if (shadow) {
-		vfree(shadow);
-		kfree(fbops);
-	}
+	vfree(shadow);
 
 	drm_client_framebuffer_delete(fb_helper->buffer);
 }
@@ -2062,23 +2057,14 @@  static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
 	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
 
 	if (drm_fbdev_use_shadow_fb(fb_helper)) {
-		struct fb_ops *fbops;
 		void *shadow;
 
-		/*
-		 * fb_deferred_io_cleanup() clears &fbops->fb_mmap so a per
-		 * instance version is necessary.
-		 */
-		fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
 		shadow = vzalloc(fbi->screen_size);
-		if (!fbops || !shadow) {
-			kfree(fbops);
+		if (!shadow) {
 			vfree(shadow);
 			return -ENOMEM;
 		}
 
-		*fbops = *fbi->fbops;
-		fbi->fbops = fbops;
 		fbi->screen_buffer = shadow;
 		fbi->fbdefio = &drm_fbdev_defio;