[v2,12/14] drm/ast: Replace struct ast_crtc with struct drm_crtc
diff mbox series

Message ID 20200702115029.5281-13-tzimmermann@suse.de
State New
Headers show
Series
  • drm/ast: Managed modesetting
Related show

Commit Message

Thomas Zimmermann July 2, 2020, 11:50 a.m. UTC
Struct ast_crtc has been cleaned up and it's now a wrapper around the
DRM CRTC structure struct drm_crtc. This patch converts the driver to
struct drm_crtc and removes struct ast_crtc.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  |  5 -----
 drivers/gpu/drm/ast/ast_mode.c | 11 ++++++-----
 2 files changed, 6 insertions(+), 10 deletions(-)

Comments

Sam Ravnborg July 3, 2020, 6:38 a.m. UTC | #1
Hi Thomas.

Just browsing code..

On Thu, Jul 02, 2020 at 01:50:27PM +0200, Thomas Zimmermann wrote:
> Struct ast_crtc has been cleaned up and it's now a wrapper around the
> DRM CRTC structure struct drm_crtc. This patch converts the driver to
> struct drm_crtc and removes struct ast_crtc.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Why is it we cannot embed struct drm_crtc?
And I also failed to see where is is de-allocated - but surely I miss
something obvious here.

	Sam

> ---
>  drivers/gpu/drm/ast/ast_drv.h  |  5 -----
>  drivers/gpu/drm/ast/ast_mode.c | 11 ++++++-----
>  2 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index ea4de3dce2c4..77226e2fd7c0 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -238,11 +238,6 @@ struct ast_connector {
>  	struct ast_i2c_chan *i2c;
>  };
>  
> -struct ast_crtc {
> -	struct drm_crtc base;
> -};
> -
> -#define to_ast_crtc(x) container_of(x, struct ast_crtc, base)
>  #define to_ast_connector(x) container_of(x, struct ast_connector, base)
>  
>  struct ast_vbios_stdtable {
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 89d9ee0a9e81..43c9686ba0f7 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -881,21 +881,22 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>  static int ast_crtc_init(struct drm_device *dev)
>  {
>  	struct ast_private *ast = to_ast_private(dev);
> -	struct ast_crtc *crtc;
> +	struct drm_crtc *crtc;
>  	int ret;
>  
> -	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
> +	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
>  	if (!crtc)
>  		return -ENOMEM;
>  
> -	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
> +	ret = drm_crtc_init_with_planes(dev, crtc, &ast->primary_plane,
>  					&ast->cursor_plane, &ast_crtc_funcs,
>  					NULL);
>  	if (ret)
>  		goto err_kfree;
>  
> -	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
> -	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
> +	drm_mode_crtc_set_gamma_size(crtc, 256);
> +	drm_crtc_helper_add(crtc, &ast_crtc_helper_funcs);
> +
>  	return 0;
>  
>  err_kfree:
> -- 
> 2.27.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Thomas Zimmermann July 3, 2020, 6:51 a.m. UTC | #2
Hi Sam

Am 03.07.20 um 08:38 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> Just browsing code..
> 
> On Thu, Jul 02, 2020 at 01:50:27PM +0200, Thomas Zimmermann wrote:
>> Struct ast_crtc has been cleaned up and it's now a wrapper around the
>> DRM CRTC structure struct drm_crtc. This patch converts the driver to
>> struct drm_crtc and removes struct ast_crtc.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Why is it we cannot embed struct drm_crtc?

I want to do that in a later patchset. The conversion to managed code is
fairly large, so thought it might be better to do it in several rounds.

This patchset is only for modesetting. I have another patchset that
converts the memory management to managed interfaces. After that the
final patchset will address device structures. Embedding everything CRTC
and other structures in struct ast_private would be part of this.

If you prefer a longer patchset that does everything, let me know.

> And I also failed to see where is is de-allocated - but surely I miss
> something obvious here.

It's freed in ast_crtc_destroy().

Best regards
Thomas

> 
> 	Sam
> 
>> ---
>>  drivers/gpu/drm/ast/ast_drv.h  |  5 -----
>>  drivers/gpu/drm/ast/ast_mode.c | 11 ++++++-----
>>  2 files changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>> index ea4de3dce2c4..77226e2fd7c0 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -238,11 +238,6 @@ struct ast_connector {
>>  	struct ast_i2c_chan *i2c;
>>  };
>>  
>> -struct ast_crtc {
>> -	struct drm_crtc base;
>> -};
>> -
>> -#define to_ast_crtc(x) container_of(x, struct ast_crtc, base)
>>  #define to_ast_connector(x) container_of(x, struct ast_connector, base)
>>  
>>  struct ast_vbios_stdtable {
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 89d9ee0a9e81..43c9686ba0f7 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -881,21 +881,22 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>>  static int ast_crtc_init(struct drm_device *dev)
>>  {
>>  	struct ast_private *ast = to_ast_private(dev);
>> -	struct ast_crtc *crtc;
>> +	struct drm_crtc *crtc;
>>  	int ret;
>>  
>> -	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
>> +	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
>>  	if (!crtc)
>>  		return -ENOMEM;
>>  
>> -	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
>> +	ret = drm_crtc_init_with_planes(dev, crtc, &ast->primary_plane,
>>  					&ast->cursor_plane, &ast_crtc_funcs,
>>  					NULL);
>>  	if (ret)
>>  		goto err_kfree;
>>  
>> -	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>> -	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
>> +	drm_mode_crtc_set_gamma_size(crtc, 256);
>> +	drm_crtc_helper_add(crtc, &ast_crtc_helper_funcs);
>> +
>>  	return 0;
>>  
>>  err_kfree:
>> -- 
>> 2.27.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Sam Ravnborg July 3, 2020, 11:11 a.m. UTC | #3
Hi Thomas.

On Fri, Jul 03, 2020 at 08:51:31AM +0200, Thomas Zimmermann wrote:
> Hi Sam
> 
> Am 03.07.20 um 08:38 schrieb Sam Ravnborg:
> > Hi Thomas.
> > 
> > Just browsing code..
> > 
> > On Thu, Jul 02, 2020 at 01:50:27PM +0200, Thomas Zimmermann wrote:
> >> Struct ast_crtc has been cleaned up and it's now a wrapper around the
> >> DRM CRTC structure struct drm_crtc. This patch converts the driver to
> >> struct drm_crtc and removes struct ast_crtc.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > 
> > Why is it we cannot embed struct drm_crtc?
> 
> I want to do that in a later patchset. The conversion to managed code is
> fairly large, so thought it might be better to do it in several rounds.

Understood, and several rounds are good.

> 
> This patchset is only for modesetting. I have another patchset that
> converts the memory management to managed interfaces. After that the
> final patchset will address device structures. Embedding everything CRTC
> and other structures in struct ast_private would be part of this.
> 
> If you prefer a longer patchset that does everything, let me know.
> 
> > And I also failed to see where is is de-allocated - but surely I miss
> > something obvious here.
> 
> It's freed in ast_crtc_destroy().

Ohh, one of the places that worked/works because struct crtc was/is the
first member.

I get it now, thanks for the explanation.

	Sam

Patch
diff mbox series

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index ea4de3dce2c4..77226e2fd7c0 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -238,11 +238,6 @@  struct ast_connector {
 	struct ast_i2c_chan *i2c;
 };
 
-struct ast_crtc {
-	struct drm_crtc base;
-};
-
-#define to_ast_crtc(x) container_of(x, struct ast_crtc, base)
 #define to_ast_connector(x) container_of(x, struct ast_connector, base)
 
 struct ast_vbios_stdtable {
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 89d9ee0a9e81..43c9686ba0f7 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -881,21 +881,22 @@  static const struct drm_crtc_funcs ast_crtc_funcs = {
 static int ast_crtc_init(struct drm_device *dev)
 {
 	struct ast_private *ast = to_ast_private(dev);
-	struct ast_crtc *crtc;
+	struct drm_crtc *crtc;
 	int ret;
 
-	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
+	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
 	if (!crtc)
 		return -ENOMEM;
 
-	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
+	ret = drm_crtc_init_with_planes(dev, crtc, &ast->primary_plane,
 					&ast->cursor_plane, &ast_crtc_funcs,
 					NULL);
 	if (ret)
 		goto err_kfree;
 
-	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
-	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
+	drm_mode_crtc_set_gamma_size(crtc, 256);
+	drm_crtc_helper_add(crtc, &ast_crtc_helper_funcs);
+
 	return 0;
 
 err_kfree: