diff mbox

[03/22] drm/arc: Use drm_fb_cma_fbdev_init/fini()

Message ID 20171104130416.12423-4-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show

Commit Message

Noralf Trønnes Nov. 4, 2017, 1:03 p.m. UTC
Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
the fact that drm_device holds a pointer to the drm_fb_helper structure.
This means that the driver doesn't have to keep track of that.
Also use the drm_fb_helper functions directly.
Remove unused function prototype arcpgu_fbdev_cma_init().

Cc: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arc/arcpgu.h     |  4 ----
 drivers/gpu/drm/arc/arcpgu_drv.c | 36 +++++++-----------------------------
 2 files changed, 7 insertions(+), 33 deletions(-)

Comments

Daniel Vetter Nov. 6, 2017, 9:08 a.m. UTC | #1
On Sat, Nov 04, 2017 at 02:03:57PM +0100, Noralf Trønnes wrote:
> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.
> Remove unused function prototype arcpgu_fbdev_cma_init().
> 
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/arc/arcpgu.h     |  4 ----
>  drivers/gpu/drm/arc/arcpgu_drv.c | 36 +++++++-----------------------------
>  2 files changed, 7 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
> index e8fcf3ab1d9a..90ef76b19f8a 100644
> --- a/drivers/gpu/drm/arc/arcpgu.h
> +++ b/drivers/gpu/drm/arc/arcpgu.h
> @@ -20,7 +20,6 @@
>  struct arcpgu_drm_private {
>  	void __iomem		*regs;
>  	struct clk		*clk;
> -	struct drm_fbdev_cma	*fbdev;
>  	struct drm_framebuffer	*fb;
>  	struct drm_crtc		crtc;
>  	struct drm_plane	*plane;
> @@ -43,8 +42,5 @@ static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
>  int arc_pgu_setup_crtc(struct drm_device *dev);
>  int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
>  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
> -struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev,
> -	unsigned int preferred_bpp, unsigned int num_crtc,
> -	unsigned int max_conn_count);
>  
>  #endif
> diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
> index 074fd4ea7ece..f54481ee834c 100644
> --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> @@ -16,6 +16,7 @@
>  
>  #include <linux/clk.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
> @@ -25,16 +26,9 @@
>  #include "arcpgu.h"
>  #include "arcpgu_regs.h"
>  
> -static void arcpgu_fb_output_poll_changed(struct drm_device *dev)
> -{
> -	struct arcpgu_drm_private *arcpgu = dev->dev_private;
> -
> -	drm_fbdev_cma_hotplug_event(arcpgu->fbdev);
> -}
> -
>  static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
>  	.fb_create  = drm_gem_fb_create,
> -	.output_poll_changed = arcpgu_fb_output_poll_changed,
> +	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  	.atomic_check = drm_atomic_helper_check,
>  	.atomic_commit = drm_atomic_helper_commit,
>  };
> @@ -51,13 +45,6 @@ static void arcpgu_setup_mode_config(struct drm_device *drm)
>  
>  DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
>  
> -static void arcpgu_lastclose(struct drm_device *drm)
> -{
> -	struct arcpgu_drm_private *arcpgu = drm->dev_private;
> -
> -	drm_fbdev_cma_restore_mode(arcpgu->fbdev);
> -}
> -
>  static int arcpgu_load(struct drm_device *drm)
>  {
>  	struct platform_device *pdev = to_platform_device(drm->dev);
> @@ -113,13 +100,9 @@ static int arcpgu_load(struct drm_device *drm)
>  	drm_mode_config_reset(drm);
>  	drm_kms_helper_poll_init(drm);
>  
> -	arcpgu->fbdev = drm_fbdev_cma_init(drm, 16,
> -					   drm->mode_config.num_connector);
> -	if (IS_ERR(arcpgu->fbdev)) {
> -		ret = PTR_ERR(arcpgu->fbdev);
> -		arcpgu->fbdev = NULL;
> -		return -ENODEV;
> -	}
> +	ret = drm_fb_cma_fbdev_init(drm, 16, 0);

s/0/NULL/

Probably applies to other patches too.

Also, why did you merge this new change into the old patch with the
output_poll/lastclose change? It took me a while to figure out what you're
doing, because the patches looked familiar, but the cover letter didn't
mention how stuff evolved, nor the patches here.

Also, you've lost the bunch of acks/r-bs you've gathered already. If
there's no technical reason to merge the 2 patches I'm kinda leaning
towards merging them separately, at least the ones that have acks already.
Just to avoid the pitfall of the continually evolving patch series that
never lands.
-Daniel

> +	if (ret)
> +		return ret;
>  
>  	platform_set_drvdata(pdev, drm);
>  	return 0;
> @@ -127,12 +110,7 @@ static int arcpgu_load(struct drm_device *drm)
>  
>  static int arcpgu_unload(struct drm_device *drm)
>  {
> -	struct arcpgu_drm_private *arcpgu = drm->dev_private;
> -
> -	if (arcpgu->fbdev) {
> -		drm_fbdev_cma_fini(arcpgu->fbdev);
> -		arcpgu->fbdev = NULL;
> -	}
> +	drm_fb_cma_fbdev_fini(drm);
>  	drm_kms_helper_poll_fini(drm);
>  	drm_mode_config_cleanup(drm);
>  
> @@ -168,7 +146,7 @@ static int arcpgu_debugfs_init(struct drm_minor *minor)
>  static struct drm_driver arcpgu_drm_driver = {
>  	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
>  			   DRIVER_ATOMIC,
> -	.lastclose = arcpgu_lastclose,
> +	.lastclose = drm_fb_helper_lastclose,
>  	.name = "arcpgu",
>  	.desc = "ARC PGU Controller",
>  	.date = "20160219",
> -- 
> 2.14.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Noralf Trønnes Nov. 6, 2017, 12:48 p.m. UTC | #2
Den 06.11.2017 10.08, skrev Daniel Vetter:
> On Sat, Nov 04, 2017 at 02:03:57PM +0100, Noralf Trønnes wrote:
>> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
>> the fact that drm_device holds a pointer to the drm_fb_helper structure.
>> This means that the driver doesn't have to keep track of that.
>> Also use the drm_fb_helper functions directly.
>> Remove unused function prototype arcpgu_fbdev_cma_init().
>>
>> Cc: Alexey Brodkin <abrodkin@synopsys.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/arc/arcpgu.h     |  4 ----
>>   drivers/gpu/drm/arc/arcpgu_drv.c | 36 +++++++-----------------------------
>>   2 files changed, 7 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
>> index e8fcf3ab1d9a..90ef76b19f8a 100644
>> --- a/drivers/gpu/drm/arc/arcpgu.h
>> +++ b/drivers/gpu/drm/arc/arcpgu.h
>> @@ -20,7 +20,6 @@
>>   struct arcpgu_drm_private {
>>   	void __iomem		*regs;
>>   	struct clk		*clk;
>> -	struct drm_fbdev_cma	*fbdev;
>>   	struct drm_framebuffer	*fb;
>>   	struct drm_crtc		crtc;
>>   	struct drm_plane	*plane;
>> @@ -43,8 +42,5 @@ static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
>>   int arc_pgu_setup_crtc(struct drm_device *dev);
>>   int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
>>   int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
>> -struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev,
>> -	unsigned int preferred_bpp, unsigned int num_crtc,
>> -	unsigned int max_conn_count);
>>   
>>   #endif
>> diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
>> index 074fd4ea7ece..f54481ee834c 100644
>> --- a/drivers/gpu/drm/arc/arcpgu_drv.c
>> +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
>> @@ -16,6 +16,7 @@
>>   
>>   #include <linux/clk.h>
>>   #include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_fb_helper.h>
>>   #include <drm/drm_fb_cma_helper.h>
>>   #include <drm/drm_gem_cma_helper.h>
>>   #include <drm/drm_gem_framebuffer_helper.h>
>> @@ -25,16 +26,9 @@
>>   #include "arcpgu.h"
>>   #include "arcpgu_regs.h"
>>   
>> -static void arcpgu_fb_output_poll_changed(struct drm_device *dev)
>> -{
>> -	struct arcpgu_drm_private *arcpgu = dev->dev_private;
>> -
>> -	drm_fbdev_cma_hotplug_event(arcpgu->fbdev);
>> -}
>> -
>>   static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
>>   	.fb_create  = drm_gem_fb_create,
>> -	.output_poll_changed = arcpgu_fb_output_poll_changed,
>> +	.output_poll_changed = drm_fb_helper_output_poll_changed,
>>   	.atomic_check = drm_atomic_helper_check,
>>   	.atomic_commit = drm_atomic_helper_commit,
>>   };
>> @@ -51,13 +45,6 @@ static void arcpgu_setup_mode_config(struct drm_device *drm)
>>   
>>   DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
>>   
>> -static void arcpgu_lastclose(struct drm_device *drm)
>> -{
>> -	struct arcpgu_drm_private *arcpgu = drm->dev_private;
>> -
>> -	drm_fbdev_cma_restore_mode(arcpgu->fbdev);
>> -}
>> -
>>   static int arcpgu_load(struct drm_device *drm)
>>   {
>>   	struct platform_device *pdev = to_platform_device(drm->dev);
>> @@ -113,13 +100,9 @@ static int arcpgu_load(struct drm_device *drm)
>>   	drm_mode_config_reset(drm);
>>   	drm_kms_helper_poll_init(drm);
>>   
>> -	arcpgu->fbdev = drm_fbdev_cma_init(drm, 16,
>> -					   drm->mode_config.num_connector);
>> -	if (IS_ERR(arcpgu->fbdev)) {
>> -		ret = PTR_ERR(arcpgu->fbdev);
>> -		arcpgu->fbdev = NULL;
>> -		return -ENODEV;
>> -	}
>> +	ret = drm_fb_cma_fbdev_init(drm, 16, 0);
> s/0/NULL/
>
> Probably applies to other patches too.
>
> Also, why did you merge this new change into the old patch with the
> output_poll/lastclose change? It took me a while to figure out what you're
> doing, because the patches looked familiar, but the cover letter didn't
> mention how stuff evolved, nor the patches here.
>
> Also, you've lost the bunch of acks/r-bs you've gathered already. If
> there's no technical reason to merge the 2 patches I'm kinda leaning
> towards merging them separately, at least the ones that have acks already.
> Just to avoid the pitfall of the continually evolving patch series that
> never lands.

Ah, now I understand some of the confusion.

I have one patchset that converts the drivers that don't use the cma helper:
drm/fb-helper: Add .last_close and .output_poll_changed helpers

And in this patchset I remove the use of struct drm_fbdev_cma which means
I move away from the all the drm_fbdev_cma* functions. This means switching
to the .last_close and .output_poll_changed helpers from the previous
patchset.

Not sure how I can un-confuse this.

Noralf.

> -Daniel
>
>> +	if (ret)
>> +		return ret;
>>   
>>   	platform_set_drvdata(pdev, drm);
>>   	return 0;
>> @@ -127,12 +110,7 @@ static int arcpgu_load(struct drm_device *drm)
>>   
>>   static int arcpgu_unload(struct drm_device *drm)
>>   {
>> -	struct arcpgu_drm_private *arcpgu = drm->dev_private;
>> -
>> -	if (arcpgu->fbdev) {
>> -		drm_fbdev_cma_fini(arcpgu->fbdev);
>> -		arcpgu->fbdev = NULL;
>> -	}
>> +	drm_fb_cma_fbdev_fini(drm);
>>   	drm_kms_helper_poll_fini(drm);
>>   	drm_mode_config_cleanup(drm);
>>   
>> @@ -168,7 +146,7 @@ static int arcpgu_debugfs_init(struct drm_minor *minor)
>>   static struct drm_driver arcpgu_drm_driver = {
>>   	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
>>   			   DRIVER_ATOMIC,
>> -	.lastclose = arcpgu_lastclose,
>> +	.lastclose = drm_fb_helper_lastclose,
>>   	.name = "arcpgu",
>>   	.desc = "ARC PGU Controller",
>>   	.date = "20160219",
>> -- 
>> 2.14.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index e8fcf3ab1d9a..90ef76b19f8a 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -20,7 +20,6 @@ 
 struct arcpgu_drm_private {
 	void __iomem		*regs;
 	struct clk		*clk;
-	struct drm_fbdev_cma	*fbdev;
 	struct drm_framebuffer	*fb;
 	struct drm_crtc		crtc;
 	struct drm_plane	*plane;
@@ -43,8 +42,5 @@  static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
 int arc_pgu_setup_crtc(struct drm_device *dev);
 int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
-struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev,
-	unsigned int preferred_bpp, unsigned int num_crtc,
-	unsigned int max_conn_count);
 
 #endif
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 074fd4ea7ece..f54481ee834c 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -16,6 +16,7 @@ 
 
 #include <linux/clk.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -25,16 +26,9 @@ 
 #include "arcpgu.h"
 #include "arcpgu_regs.h"
 
-static void arcpgu_fb_output_poll_changed(struct drm_device *dev)
-{
-	struct arcpgu_drm_private *arcpgu = dev->dev_private;
-
-	drm_fbdev_cma_hotplug_event(arcpgu->fbdev);
-}
-
 static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
 	.fb_create  = drm_gem_fb_create,
-	.output_poll_changed = arcpgu_fb_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -51,13 +45,6 @@  static void arcpgu_setup_mode_config(struct drm_device *drm)
 
 DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
 
-static void arcpgu_lastclose(struct drm_device *drm)
-{
-	struct arcpgu_drm_private *arcpgu = drm->dev_private;
-
-	drm_fbdev_cma_restore_mode(arcpgu->fbdev);
-}
-
 static int arcpgu_load(struct drm_device *drm)
 {
 	struct platform_device *pdev = to_platform_device(drm->dev);
@@ -113,13 +100,9 @@  static int arcpgu_load(struct drm_device *drm)
 	drm_mode_config_reset(drm);
 	drm_kms_helper_poll_init(drm);
 
-	arcpgu->fbdev = drm_fbdev_cma_init(drm, 16,
-					   drm->mode_config.num_connector);
-	if (IS_ERR(arcpgu->fbdev)) {
-		ret = PTR_ERR(arcpgu->fbdev);
-		arcpgu->fbdev = NULL;
-		return -ENODEV;
-	}
+	ret = drm_fb_cma_fbdev_init(drm, 16, 0);
+	if (ret)
+		return ret;
 
 	platform_set_drvdata(pdev, drm);
 	return 0;
@@ -127,12 +110,7 @@  static int arcpgu_load(struct drm_device *drm)
 
 static int arcpgu_unload(struct drm_device *drm)
 {
-	struct arcpgu_drm_private *arcpgu = drm->dev_private;
-
-	if (arcpgu->fbdev) {
-		drm_fbdev_cma_fini(arcpgu->fbdev);
-		arcpgu->fbdev = NULL;
-	}
+	drm_fb_cma_fbdev_fini(drm);
 	drm_kms_helper_poll_fini(drm);
 	drm_mode_config_cleanup(drm);
 
@@ -168,7 +146,7 @@  static int arcpgu_debugfs_init(struct drm_minor *minor)
 static struct drm_driver arcpgu_drm_driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
 			   DRIVER_ATOMIC,
-	.lastclose = arcpgu_lastclose,
+	.lastclose = drm_fb_helper_lastclose,
 	.name = "arcpgu",
 	.desc = "ARC PGU Controller",
 	.date = "20160219",