* [PATCH 54/59] drm/aspeed: Drop aspeed_gfx->fbdev [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch> @ 2020-04-15 7:40 ` Daniel Vetter 2020-04-24 18:00 ` Sam Ravnborg 2020-04-15 7:40 ` [PATCH 55/59] drm/aspeed: Use devm_drm_dev_alloc Daniel Vetter 2020-04-15 7:40 ` [PATCH 56/59] drm/aspeed: Use managed drmm_mode_config_cleanup Daniel Vetter 2 siblings, 1 reply; 8+ messages in thread From: Daniel Vetter @ 2020-04-15 7:40 UTC (permalink / raw) To: Intel Graphics Development Cc: linux-aspeed, Andrew Jeffery, Daniel Vetter, DRI Development, Joel Stanley, Daniel Vetter, linux-arm-kernel No longer used since the conversion to generic fbdev. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Joel Stanley <joel@jms.id.au> Cc: Andrew Jeffery <andrew@aj.id.au> Cc: linux-aspeed@lists.ozlabs.org Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/aspeed/aspeed_gfx.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h index a10358bb61ec..adc02940de6f 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h @@ -12,7 +12,6 @@ struct aspeed_gfx { struct drm_simple_display_pipe pipe; struct drm_connector connector; - struct drm_fbdev_cma *fbdev; }; int aspeed_gfx_create_pipe(struct drm_device *drm); -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 54/59] drm/aspeed: Drop aspeed_gfx->fbdev 2020-04-15 7:40 ` [PATCH 54/59] drm/aspeed: Drop aspeed_gfx->fbdev Daniel Vetter @ 2020-04-24 18:00 ` Sam Ravnborg 0 siblings, 0 replies; 8+ messages in thread From: Sam Ravnborg @ 2020-04-24 18:00 UTC (permalink / raw) To: Daniel Vetter Cc: linux-aspeed, Andrew Jeffery, Intel Graphics Development, DRI Development, Joel Stanley, Daniel Vetter, linux-arm-kernel On Wed, Apr 15, 2020 at 09:40:29AM +0200, Daniel Vetter wrote: > No longer used since the conversion to generic fbdev. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Joel Stanley <joel@jms.id.au> > Cc: Andrew Jeffery <andrew@aj.id.au> > Cc: linux-aspeed@lists.ozlabs.org > Cc: linux-arm-kernel@lists.infradead.org Acked-by: Sam Ravnborg <sam@ravnborg.org> > --- > drivers/gpu/drm/aspeed/aspeed_gfx.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h > index a10358bb61ec..adc02940de6f 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h > @@ -12,7 +12,6 @@ struct aspeed_gfx { > > struct drm_simple_display_pipe pipe; > struct drm_connector connector; > - struct drm_fbdev_cma *fbdev; > }; > > int aspeed_gfx_create_pipe(struct drm_device *drm); > -- > 2.25.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 55/59] drm/aspeed: Use devm_drm_dev_alloc [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch> 2020-04-15 7:40 ` [PATCH 54/59] drm/aspeed: Drop aspeed_gfx->fbdev Daniel Vetter @ 2020-04-15 7:40 ` Daniel Vetter 2020-04-24 18:02 ` Sam Ravnborg 2020-04-15 7:40 ` [PATCH 56/59] drm/aspeed: Use managed drmm_mode_config_cleanup Daniel Vetter 2 siblings, 1 reply; 8+ messages in thread From: Daniel Vetter @ 2020-04-15 7:40 UTC (permalink / raw) To: Intel Graphics Development Cc: linux-aspeed, Andrew Jeffery, Daniel Vetter, DRI Development, Joel Stanley, Daniel Vetter, linux-arm-kernel As usual, we can drop the drm_dev_put() and need to embed the drm_device. Since it's so few, also go right ahead and leave drm_device->dev_private set to NULL, so that we always use the container_of() upcast, which is faster anyway. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Joel Stanley <joel@jms.id.au> Cc: Andrew Jeffery <andrew@aj.id.au> Cc: linux-aspeed@lists.ozlabs.org Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 ++ drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 2 +- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 31 +++++++++--------------- drivers/gpu/drm/aspeed/aspeed_gfx_out.c | 2 +- 4 files changed, 15 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h index adc02940de6f..e7ca95827ae8 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h @@ -5,6 +5,7 @@ #include <drm/drm_simple_kms_helper.h> struct aspeed_gfx { + struct drm_device drm; void __iomem *base; struct clk *clk; struct reset_control *rst; @@ -13,6 +14,7 @@ struct aspeed_gfx { struct drm_simple_display_pipe pipe; struct drm_connector connector; }; +#define to_aspeed_gfx(x) container_of(x, struct aspeed_gfx, drm) int aspeed_gfx_create_pipe(struct drm_device *drm); int aspeed_gfx_create_output(struct drm_device *drm); diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c index 2184b8be6fd4..e54686c31a90 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c @@ -231,7 +231,7 @@ static const uint32_t aspeed_gfx_formats[] = { int aspeed_gfx_create_pipe(struct drm_device *drm) { - struct aspeed_gfx *priv = drm->dev_private; + struct aspeed_gfx *priv = to_aspeed_gfx(drm); return drm_simple_display_pipe_init(drm, &priv->pipe, &aspeed_gfx_funcs, aspeed_gfx_formats, diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index ada2f6aca906..6b27242b9ee3 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -77,7 +77,7 @@ static void aspeed_gfx_setup_mode_config(struct drm_device *drm) static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) { struct drm_device *drm = data; - struct aspeed_gfx *priv = drm->dev_private; + struct aspeed_gfx *priv = to_aspeed_gfx(drm); u32 reg; reg = readl(priv->base + CRT_CTRL1); @@ -96,15 +96,10 @@ static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) static int aspeed_gfx_load(struct drm_device *drm) { struct platform_device *pdev = to_platform_device(drm->dev); - struct aspeed_gfx *priv; + struct aspeed_gfx *priv = to_aspeed_gfx(drm); struct resource *res; int ret; - priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); - if (!priv) - return -ENOMEM; - drm->dev_private = priv; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); priv->base = devm_ioremap_resource(drm->dev, res); if (IS_ERR(priv->base)) @@ -187,8 +182,6 @@ static void aspeed_gfx_unload(struct drm_device *drm) { drm_kms_helper_poll_fini(drm); drm_mode_config_cleanup(drm); - - drm->dev_private = NULL; } DEFINE_DRM_GEM_CMA_FOPS(fops); @@ -216,27 +209,26 @@ static const struct of_device_id aspeed_gfx_match[] = { static int aspeed_gfx_probe(struct platform_device *pdev) { - struct drm_device *drm; + struct aspeed_gfx *priv; int ret; - drm = drm_dev_alloc(&aspeed_gfx_driver, &pdev->dev); - if (IS_ERR(drm)) - return PTR_ERR(drm); + priv = devm_drm_dev_alloc(&pdev->dev, &aspeed_gfx_driver, + struct aspeed_gfx, drm); + if (IS_ERR(priv)) + return PTR_ERR(priv); - ret = aspeed_gfx_load(drm); + ret = aspeed_gfx_load(&priv->drm); if (ret) - goto err_free; + return ret; - ret = drm_dev_register(drm, 0); + ret = drm_dev_register(&priv->drm, 0); if (ret) goto err_unload; return 0; err_unload: - aspeed_gfx_unload(drm); -err_free: - drm_dev_put(drm); + aspeed_gfx_unload(&priv->drm); return ret; } @@ -247,7 +239,6 @@ static int aspeed_gfx_remove(struct platform_device *pdev) drm_dev_unregister(drm); aspeed_gfx_unload(drm); - drm_dev_put(drm); return 0; } diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_out.c b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c index 67ee5fa10055..6759cb88415a 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_out.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c @@ -28,7 +28,7 @@ static const struct drm_connector_funcs aspeed_gfx_connector_funcs = { int aspeed_gfx_create_output(struct drm_device *drm) { - struct aspeed_gfx *priv = drm->dev_private; + struct aspeed_gfx *priv = to_aspeed_gfx(drm); int ret; priv->connector.dpms = DRM_MODE_DPMS_OFF; -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 55/59] drm/aspeed: Use devm_drm_dev_alloc 2020-04-15 7:40 ` [PATCH 55/59] drm/aspeed: Use devm_drm_dev_alloc Daniel Vetter @ 2020-04-24 18:02 ` Sam Ravnborg 0 siblings, 0 replies; 8+ messages in thread From: Sam Ravnborg @ 2020-04-24 18:02 UTC (permalink / raw) To: Daniel Vetter Cc: linux-aspeed, Andrew Jeffery, Intel Graphics Development, DRI Development, Joel Stanley, Daniel Vetter, linux-arm-kernel On Wed, Apr 15, 2020 at 09:40:30AM +0200, Daniel Vetter wrote: > As usual, we can drop the drm_dev_put() and need to embed the > drm_device. Since it's so few, also go right ahead and leave > drm_device->dev_private set to NULL, so that we always use the > container_of() upcast, which is faster anyway. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Joel Stanley <joel@jms.id.au> > Cc: Andrew Jeffery <andrew@aj.id.au> > Cc: linux-aspeed@lists.ozlabs.org > Cc: linux-arm-kernel@lists.infradead.org Acked-by: Sam Ravnborg <sam@ravnborg.org> > --- > drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 ++ > drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 2 +- > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 31 +++++++++--------------- > drivers/gpu/drm/aspeed/aspeed_gfx_out.c | 2 +- > 4 files changed, 15 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h > index adc02940de6f..e7ca95827ae8 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h > @@ -5,6 +5,7 @@ > #include <drm/drm_simple_kms_helper.h> > > struct aspeed_gfx { > + struct drm_device drm; > void __iomem *base; > struct clk *clk; > struct reset_control *rst; > @@ -13,6 +14,7 @@ struct aspeed_gfx { > struct drm_simple_display_pipe pipe; > struct drm_connector connector; > }; > +#define to_aspeed_gfx(x) container_of(x, struct aspeed_gfx, drm) > > int aspeed_gfx_create_pipe(struct drm_device *drm); > int aspeed_gfx_create_output(struct drm_device *drm); > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c > index 2184b8be6fd4..e54686c31a90 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c > @@ -231,7 +231,7 @@ static const uint32_t aspeed_gfx_formats[] = { > > int aspeed_gfx_create_pipe(struct drm_device *drm) > { > - struct aspeed_gfx *priv = drm->dev_private; > + struct aspeed_gfx *priv = to_aspeed_gfx(drm); > > return drm_simple_display_pipe_init(drm, &priv->pipe, &aspeed_gfx_funcs, > aspeed_gfx_formats, > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > index ada2f6aca906..6b27242b9ee3 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > @@ -77,7 +77,7 @@ static void aspeed_gfx_setup_mode_config(struct drm_device *drm) > static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) > { > struct drm_device *drm = data; > - struct aspeed_gfx *priv = drm->dev_private; > + struct aspeed_gfx *priv = to_aspeed_gfx(drm); > u32 reg; > > reg = readl(priv->base + CRT_CTRL1); > @@ -96,15 +96,10 @@ static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) > static int aspeed_gfx_load(struct drm_device *drm) > { > struct platform_device *pdev = to_platform_device(drm->dev); > - struct aspeed_gfx *priv; > + struct aspeed_gfx *priv = to_aspeed_gfx(drm); > struct resource *res; > int ret; > > - priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > - if (!priv) > - return -ENOMEM; > - drm->dev_private = priv; > - > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > priv->base = devm_ioremap_resource(drm->dev, res); > if (IS_ERR(priv->base)) > @@ -187,8 +182,6 @@ static void aspeed_gfx_unload(struct drm_device *drm) > { > drm_kms_helper_poll_fini(drm); > drm_mode_config_cleanup(drm); > - > - drm->dev_private = NULL; > } > > DEFINE_DRM_GEM_CMA_FOPS(fops); > @@ -216,27 +209,26 @@ static const struct of_device_id aspeed_gfx_match[] = { > > static int aspeed_gfx_probe(struct platform_device *pdev) > { > - struct drm_device *drm; > + struct aspeed_gfx *priv; > int ret; > > - drm = drm_dev_alloc(&aspeed_gfx_driver, &pdev->dev); > - if (IS_ERR(drm)) > - return PTR_ERR(drm); > + priv = devm_drm_dev_alloc(&pdev->dev, &aspeed_gfx_driver, > + struct aspeed_gfx, drm); > + if (IS_ERR(priv)) > + return PTR_ERR(priv); > > - ret = aspeed_gfx_load(drm); > + ret = aspeed_gfx_load(&priv->drm); > if (ret) > - goto err_free; > + return ret; > > - ret = drm_dev_register(drm, 0); > + ret = drm_dev_register(&priv->drm, 0); > if (ret) > goto err_unload; > > return 0; > > err_unload: > - aspeed_gfx_unload(drm); > -err_free: > - drm_dev_put(drm); > + aspeed_gfx_unload(&priv->drm); > > return ret; > } > @@ -247,7 +239,6 @@ static int aspeed_gfx_remove(struct platform_device *pdev) > > drm_dev_unregister(drm); > aspeed_gfx_unload(drm); > - drm_dev_put(drm); > > return 0; > } > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_out.c b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c > index 67ee5fa10055..6759cb88415a 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_out.c > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c > @@ -28,7 +28,7 @@ static const struct drm_connector_funcs aspeed_gfx_connector_funcs = { > > int aspeed_gfx_create_output(struct drm_device *drm) > { > - struct aspeed_gfx *priv = drm->dev_private; > + struct aspeed_gfx *priv = to_aspeed_gfx(drm); > int ret; > > priv->connector.dpms = DRM_MODE_DPMS_OFF; > -- > 2.25.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 56/59] drm/aspeed: Use managed drmm_mode_config_cleanup [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch> 2020-04-15 7:40 ` [PATCH 54/59] drm/aspeed: Drop aspeed_gfx->fbdev Daniel Vetter 2020-04-15 7:40 ` [PATCH 55/59] drm/aspeed: Use devm_drm_dev_alloc Daniel Vetter @ 2020-04-15 7:40 ` Daniel Vetter 2020-04-24 18:10 ` Sam Ravnborg 2 siblings, 1 reply; 8+ messages in thread From: Daniel Vetter @ 2020-04-15 7:40 UTC (permalink / raw) To: Intel Graphics Development Cc: linux-aspeed, Andrew Jeffery, Daniel Vetter, DRI Development, Joel Stanley, Daniel Vetter, linux-arm-kernel Since aspeed doesn't use devm_kzalloc anymore we can use the managed mode config cleanup. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Joel Stanley <joel@jms.id.au> Cc: Andrew Jeffery <andrew@aj.id.au> Cc: linux-aspeed@lists.ozlabs.org Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index 6b27242b9ee3..6e464b84a256 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -63,15 +63,15 @@ static const struct drm_mode_config_funcs aspeed_gfx_mode_config_funcs = { .atomic_commit = drm_atomic_helper_commit, }; -static void aspeed_gfx_setup_mode_config(struct drm_device *drm) +static int aspeed_gfx_setup_mode_config(struct drm_device *drm) { - drm_mode_config_init(drm); - drm->mode_config.min_width = 0; drm->mode_config.min_height = 0; drm->mode_config.max_width = 800; drm->mode_config.max_height = 600; drm->mode_config.funcs = &aspeed_gfx_mode_config_funcs; + + return drmm_mode_config_init(drm); } static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) @@ -144,7 +144,9 @@ static int aspeed_gfx_load(struct drm_device *drm) writel(0, priv->base + CRT_CTRL1); writel(0, priv->base + CRT_CTRL2); - aspeed_gfx_setup_mode_config(drm); + ret = aspeed_gfx_setup_mode_config(drm); + if (ret < 0) + return ret; ret = drm_vblank_init(drm, 1); if (ret < 0) { @@ -181,7 +183,6 @@ static int aspeed_gfx_load(struct drm_device *drm) static void aspeed_gfx_unload(struct drm_device *drm) { drm_kms_helper_poll_fini(drm); - drm_mode_config_cleanup(drm); } DEFINE_DRM_GEM_CMA_FOPS(fops); -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 56/59] drm/aspeed: Use managed drmm_mode_config_cleanup 2020-04-15 7:40 ` [PATCH 56/59] drm/aspeed: Use managed drmm_mode_config_cleanup Daniel Vetter @ 2020-04-24 18:10 ` Sam Ravnborg 2020-04-28 14:12 ` Daniel Vetter 0 siblings, 1 reply; 8+ messages in thread From: Sam Ravnborg @ 2020-04-24 18:10 UTC (permalink / raw) To: Daniel Vetter Cc: linux-aspeed, Andrew Jeffery, Intel Graphics Development, DRI Development, Joel Stanley, Daniel Vetter, linux-arm-kernel On Wed, Apr 15, 2020 at 09:40:31AM +0200, Daniel Vetter wrote: > Since aspeed doesn't use devm_kzalloc anymore we can use the managed > mode config cleanup. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Joel Stanley <joel@jms.id.au> > Cc: Andrew Jeffery <andrew@aj.id.au> > Cc: linux-aspeed@lists.ozlabs.org > Cc: linux-arm-kernel@lists.infradead.org Hmm, the helper function makes no sense, maybe embed it? One Q below. Whith Q addressed: Acked-by: Sam Ravnborg <sam@ravnborg.org> > --- > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > index 6b27242b9ee3..6e464b84a256 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > @@ -63,15 +63,15 @@ static const struct drm_mode_config_funcs aspeed_gfx_mode_config_funcs = { > .atomic_commit = drm_atomic_helper_commit, > }; > > -static void aspeed_gfx_setup_mode_config(struct drm_device *drm) > +static int aspeed_gfx_setup_mode_config(struct drm_device *drm) > { > - drm_mode_config_init(drm); > - > drm->mode_config.min_width = 0; > drm->mode_config.min_height = 0; > drm->mode_config.max_width = 800; > drm->mode_config.max_height = 600; > drm->mode_config.funcs = &aspeed_gfx_mode_config_funcs; > + > + return drmm_mode_config_init(drm); I do not see anything that documents that it is OK to init min/max width/heigh not funcs before drmm_mode_config_init() is called. Maybe drmm_mode_config_init() gain an memset(drm->mode_config), and we loose all the assingments from before the call to init(). Also most (all?) other users of drmm_mode_config_init() set them after the call to drmm_mode_config_init(). So re-order here and then embed while you are touching the code again. Sam > } > > static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) > @@ -144,7 +144,9 @@ static int aspeed_gfx_load(struct drm_device *drm) > writel(0, priv->base + CRT_CTRL1); > writel(0, priv->base + CRT_CTRL2); > > - aspeed_gfx_setup_mode_config(drm); > + ret = aspeed_gfx_setup_mode_config(drm); > + if (ret < 0) > + return ret; > > ret = drm_vblank_init(drm, 1); > if (ret < 0) { > @@ -181,7 +183,6 @@ static int aspeed_gfx_load(struct drm_device *drm) > static void aspeed_gfx_unload(struct drm_device *drm) > { > drm_kms_helper_poll_fini(drm); > - drm_mode_config_cleanup(drm); > } > > DEFINE_DRM_GEM_CMA_FOPS(fops); > -- > 2.25.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 56/59] drm/aspeed: Use managed drmm_mode_config_cleanup 2020-04-24 18:10 ` Sam Ravnborg @ 2020-04-28 14:12 ` Daniel Vetter 2020-04-28 17:03 ` Sam Ravnborg 0 siblings, 1 reply; 8+ messages in thread From: Daniel Vetter @ 2020-04-28 14:12 UTC (permalink / raw) To: Sam Ravnborg Cc: linux-aspeed, Andrew Jeffery, Daniel Vetter, Intel Graphics Development, DRI Development, Joel Stanley, Daniel Vetter, linux-arm-kernel On Fri, Apr 24, 2020 at 08:10:02PM +0200, Sam Ravnborg wrote: > On Wed, Apr 15, 2020 at 09:40:31AM +0200, Daniel Vetter wrote: > > Since aspeed doesn't use devm_kzalloc anymore we can use the managed > > mode config cleanup. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Joel Stanley <joel@jms.id.au> > > Cc: Andrew Jeffery <andrew@aj.id.au> > > Cc: linux-aspeed@lists.ozlabs.org > > Cc: linux-arm-kernel@lists.infradead.org > > Hmm, the helper function makes no sense, maybe embed it? > > One Q below. Whith Q addressed: > Acked-by: Sam Ravnborg <sam@ravnborg.org> > > > --- > > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > > index 6b27242b9ee3..6e464b84a256 100644 > > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > > @@ -63,15 +63,15 @@ static const struct drm_mode_config_funcs aspeed_gfx_mode_config_funcs = { > > .atomic_commit = drm_atomic_helper_commit, > > }; > > > > -static void aspeed_gfx_setup_mode_config(struct drm_device *drm) > > +static int aspeed_gfx_setup_mode_config(struct drm_device *drm) > > { > > - drm_mode_config_init(drm); > > - > > drm->mode_config.min_width = 0; > > drm->mode_config.min_height = 0; > > drm->mode_config.max_width = 800; > > drm->mode_config.max_height = 600; > > drm->mode_config.funcs = &aspeed_gfx_mode_config_funcs; > > + > > + return drmm_mode_config_init(drm); > > I do not see anything that documents that it is OK to init min/max > width/heigh not funcs before drmm_mode_config_init() is called. > Maybe drmm_mode_config_init() gain an memset(drm->mode_config), > and we loose all the assingments from before the call to init(). > > Also most (all?) other users of drmm_mode_config_init() > set them after the call to drmm_mode_config_init(). > So re-order here and then embed while you are touching the code again. Only reason I've done it like this is that it saves a few lines of diff compared to other options. Wrt calling stuff the wrong way round: We pretty much assume throughout that structures are allocated with kzalloc, none of our _init() functions in drm have a memset. We'd break the world if we start doing memset() in random _init() functions I think. Also the main aspeed_gfx_load() function is quite long already, smashing more random stuff in there won't help it's readability. Anyway I don't care, if you insist I'm happy to repaint this in whatever color choice you deem best :-) Cheers, Daniel > > Sam > > > } > > > > static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) > > @@ -144,7 +144,9 @@ static int aspeed_gfx_load(struct drm_device *drm) > > writel(0, priv->base + CRT_CTRL1); > > writel(0, priv->base + CRT_CTRL2); > > > > - aspeed_gfx_setup_mode_config(drm); > > + ret = aspeed_gfx_setup_mode_config(drm); > > + if (ret < 0) > > + return ret; > > > > ret = drm_vblank_init(drm, 1); > > if (ret < 0) { > > @@ -181,7 +183,6 @@ static int aspeed_gfx_load(struct drm_device *drm) > > static void aspeed_gfx_unload(struct drm_device *drm) > > { > > drm_kms_helper_poll_fini(drm); > > - drm_mode_config_cleanup(drm); > > } > > > > DEFINE_DRM_GEM_CMA_FOPS(fops); > > -- > > 2.25.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 56/59] drm/aspeed: Use managed drmm_mode_config_cleanup 2020-04-28 14:12 ` Daniel Vetter @ 2020-04-28 17:03 ` Sam Ravnborg 0 siblings, 0 replies; 8+ messages in thread From: Sam Ravnborg @ 2020-04-28 17:03 UTC (permalink / raw) To: Daniel Vetter Cc: linux-aspeed, Andrew Jeffery, Daniel Vetter, Intel Graphics Development, DRI Development, Joel Stanley, Daniel Vetter, linux-arm-kernel On Tue, Apr 28, 2020 at 04:12:21PM +0200, Daniel Vetter wrote: > On Fri, Apr 24, 2020 at 08:10:02PM +0200, Sam Ravnborg wrote: > > On Wed, Apr 15, 2020 at 09:40:31AM +0200, Daniel Vetter wrote: > > > Since aspeed doesn't use devm_kzalloc anymore we can use the managed > > > mode config cleanup. > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > Cc: Joel Stanley <joel@jms.id.au> > > > Cc: Andrew Jeffery <andrew@aj.id.au> > > > Cc: linux-aspeed@lists.ozlabs.org > > > Cc: linux-arm-kernel@lists.infradead.org > > > > Hmm, the helper function makes no sense, maybe embed it? > > > > One Q below. Whith Q addressed: > > Acked-by: Sam Ravnborg <sam@ravnborg.org> > > > > > --- > > > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 11 ++++++----- > > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > > > index 6b27242b9ee3..6e464b84a256 100644 > > > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > > > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > > > @@ -63,15 +63,15 @@ static const struct drm_mode_config_funcs aspeed_gfx_mode_config_funcs = { > > > .atomic_commit = drm_atomic_helper_commit, > > > }; > > > > > > -static void aspeed_gfx_setup_mode_config(struct drm_device *drm) > > > +static int aspeed_gfx_setup_mode_config(struct drm_device *drm) > > > { > > > - drm_mode_config_init(drm); > > > - > > > drm->mode_config.min_width = 0; > > > drm->mode_config.min_height = 0; > > > drm->mode_config.max_width = 800; > > > drm->mode_config.max_height = 600; > > > drm->mode_config.funcs = &aspeed_gfx_mode_config_funcs; > > > + > > > + return drmm_mode_config_init(drm); > > > > I do not see anything that documents that it is OK to init min/max > > width/heigh not funcs before drmm_mode_config_init() is called. > > Maybe drmm_mode_config_init() gain an memset(drm->mode_config), > > and we loose all the assingments from before the call to init(). > > > > Also most (all?) other users of drmm_mode_config_init() > > set them after the call to drmm_mode_config_init(). > > So re-order here and then embed while you are touching the code again. > > Only reason I've done it like this is that it saves a few lines of diff > compared to other options. > > Wrt calling stuff the wrong way round: We pretty much assume throughout > that structures are allocated with kzalloc, none of our _init() functions > in drm have a memset. We'd break the world if we start doing memset() in > random _init() functions I think. > > Also the main aspeed_gfx_load() function is quite long already, smashing > more random stuff in there won't help it's readability. > > Anyway I don't care, if you insist I'm happy to repaint this in whatever > color choice you deem best :-) From the principle of least suprises, you should at least call init and then set min_width and friends. This is easy to do in the helper, so easy to avoid the inlining I suggested. Sam _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-04-28 17:05 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch> 2020-04-15 7:40 ` [PATCH 54/59] drm/aspeed: Drop aspeed_gfx->fbdev Daniel Vetter 2020-04-24 18:00 ` Sam Ravnborg 2020-04-15 7:40 ` [PATCH 55/59] drm/aspeed: Use devm_drm_dev_alloc Daniel Vetter 2020-04-24 18:02 ` Sam Ravnborg 2020-04-15 7:40 ` [PATCH 56/59] drm/aspeed: Use managed drmm_mode_config_cleanup Daniel Vetter 2020-04-24 18:10 ` Sam Ravnborg 2020-04-28 14:12 ` Daniel Vetter 2020-04-28 17:03 ` Sam Ravnborg
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).