* [PATCH] drm/imx: Remove unused field imx_drm_device.pipes
@ 2018-07-17 12:11 Leonard Crestez
2018-07-17 13:21 ` Philipp Zabel
0 siblings, 1 reply; 3+ messages in thread
From: Leonard Crestez @ 2018-07-17 12:11 UTC (permalink / raw)
To: Shawn Guo, Philipp Zabel, Lucas Stach
Cc: Liu Ying, Fabio Estevam, dri-devel, linux-imx, kernel, linux-kernel
This has been unused since commit 44b460cfe554 ("drm: imx: remove struct
imx_drm_crtc and imx_drm_crtc_helper_funcs")
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/gpu/drm/imx/imx-drm-core.c | 1 -
1 file changed, 1 deletion(-)
Looking at the imx_drm_device struct it is only used to save the
drm_atomic_helper_suspend state. It seems like this could be replaced
with drm_mode_config_helper_suspend/resume and the entire struct
removed.
The only difference between imx_drm_suspend/resume and
drm_mode_config_helper_suspend/resume is that the latter also suspends
the fb_helper. This would be an improvement, right?
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 1d053bbefc02..7744e10f15b0 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -35,11 +35,10 @@
#define MAX_CRTC 4
struct imx_drm_device {
struct drm_device *drm;
- unsigned int pipes;
struct drm_atomic_state *state;
};
#if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
static int legacyfb_depth = 16;
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/imx: Remove unused field imx_drm_device.pipes
2018-07-17 12:11 [PATCH] drm/imx: Remove unused field imx_drm_device.pipes Leonard Crestez
@ 2018-07-17 13:21 ` Philipp Zabel
2018-07-17 14:34 ` Leonard Crestez
0 siblings, 1 reply; 3+ messages in thread
From: Philipp Zabel @ 2018-07-17 13:21 UTC (permalink / raw)
To: Leonard Crestez, Shawn Guo, Lucas Stach
Cc: Liu Ying, Fabio Estevam, dri-devel, linux-imx, kernel, linux-kernel
On Tue, 2018-07-17 at 15:11 +0300, Leonard Crestez wrote:
> This has been unused since commit 44b460cfe554 ("drm: imx: remove struct
> imx_drm_crtc and imx_drm_crtc_helper_funcs")
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Applied to imx-drm/next, thank you.
> ---
> drivers/gpu/drm/imx/imx-drm-core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> Looking at the imx_drm_device struct it is only used to save the
> drm_atomic_helper_suspend state. It seems like this could be replaced
> with drm_mode_config_helper_suspend/resume and the entire struct
> removed.
>
> The only difference between imx_drm_suspend/resume and
> drm_mode_config_helper_suspend/resume is that the latter also suspends
> the fb_helper. This would be an improvement, right?
This looks about right to me. We currently don't call the
fbdev suspend/resume notifiers at all. The correct call
would be drm_fbdev_cma_set_suspend_unlocked instead of
drm_fb_helper_set_suspend_unlocked, though.
Since drm_fb_cma_fbdev_init registers its fb_helper with the
drm_device via drm_fb_helper_init, both (currently) do exactly
the same thing.
regards
Philipp
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/imx: Remove unused field imx_drm_device.pipes
2018-07-17 13:21 ` Philipp Zabel
@ 2018-07-17 14:34 ` Leonard Crestez
0 siblings, 0 replies; 3+ messages in thread
From: Leonard Crestez @ 2018-07-17 14:34 UTC (permalink / raw)
To: p.zabel, stefan, thierry.reding
Cc: dl-linux-imx, linux-kernel, laurent.pinchart+renesas,
Liviu.Dudau, Fabio Estevam, dri-devel, shawnguo, gnuiyl, l.stach,
kernel
On Tue, 2018-07-17 at 15:21 +0200, Philipp Zabel wrote:
> On Tue, 2018-07-17 at 15:11 +0300, Leonard Crestez wrote:
> > This has been unused since commit 44b460cfe554 ("drm: imx: remove struct
> > imx_drm_crtc and imx_drm_crtc_helper_funcs")
> >
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>
> Applied to imx-drm/next, thank you.
>
> > ---
> > drivers/gpu/drm/imx/imx-drm-core.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > Looking at the imx_drm_device struct it is only used to save the
> > drm_atomic_helper_suspend state. It seems like this could be replaced
> > with drm_mode_config_helper_suspend/resume and the entire struct
> > removed.
> >
> > The only difference between imx_drm_suspend/resume and
> > drm_mode_config_helper_suspend/resume is that the latter also suspends
> > the fb_helper. This would be an improvement, right?
>
> This looks about right to me. We currently don't call the
> fbdev suspend/resume notifiers at all. The correct call
> would be drm_fbdev_cma_set_suspend_unlocked instead of
> drm_fb_helper_set_suspend_unlocked, though.
>
> Since drm_fb_cma_fbdev_init registers its fb_helper with the
> drm_device via drm_fb_helper_init, both (currently) do exactly
> the same thing.
Is drm_fbdev_cma_set_suspend expected to ever do anything extra on top
of drm_fb_helper_set_suspend? Since it only handles memory management
my answer would be "no".
Grepping for drm_fbdev_cma_set_suspend I only find two users: rcar-du
and hdlcd. The suspend/resume code in both of those drivers looks
entirely equivalent to drm_mode_config_helper_suspend/resume.
So maybe drm_fbdev_cma_set_suspend itself should be dropped in favor of
drm_mode_config_helper_suspend everywhere.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-07-17 14:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-17 12:11 [PATCH] drm/imx: Remove unused field imx_drm_device.pipes Leonard Crestez
2018-07-17 13:21 ` Philipp Zabel
2018-07-17 14:34 ` Leonard Crestez
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).