| Message ID | 20200702115029.5281-13-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ast: Managed modesetting | expand |
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
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 >
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
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:
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(-)