Re: [PATCH 5/6] drm/exynos: remove struct *_win_data abstraction on planes

Hi,

On 01/30/2015 11:42 PM, Gustavo Padovan wrote:

Hi Joonyoung,

2015-01-30 Joonyoung Shim jy0922.shim@samsung.com:

...

+Cc Inki,

Hi,

On 01/23/2015 09:42 PM, Gustavo Padovan wrote:

...

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

struct {fimd,mixer,vidi}_win_data was just keeping the same data as struct exynos_drm_plane thus get ride of it and use exynos_drm_plane directly.

It changes how planes are created and remove .win_mode_set() callback that was only filling all *_win_data structs.

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk

drivers/gpu/drm/exynos/exynos_drm_crtc.c | 9 +- drivers/gpu/drm/exynos/exynos_drm_crtc.h | 1 + drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 -- drivers/gpu/drm/exynos/exynos_drm_drv.h | 5 +- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 181 ++++++++++--------------- drivers/gpu/drm/exynos/exynos_drm_plane.c | 20 +-- drivers/gpu/drm/exynos/exynos_drm_plane.h | 6 +- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 123 ++++------------- drivers/gpu/drm/exynos/exynos_mixer.c | 212 +++++++++++------------------- 9 files changed, 182 insertions(+), 389 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index d0f4e1b..5cd6c1a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -287,13 +287,13 @@ static void exynos_drm_crtc_attach_mode_property(struct drm_crtc *crtc) }

struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,

• 			       struct drm_plane *plane,
  		       int pipe,
  		       enum exynos_drm_output_type type,
  		       struct exynos_drm_crtc_ops *ops,
  		       void *ctx)
  

{ struct exynos_drm_crtc *exynos_crtc;

• struct drm_plane *plane; struct exynos_drm_private *private = drm_dev->dev_private; struct drm_crtc *crtc; int ret;

@@ -309,12 +309,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev, exynos_crtc->type = type; exynos_crtc->ops = ops; exynos_crtc->ctx = ctx;

• plane = exynos_plane_init(drm_dev, 1 << pipe,

• 		  DRM_PLANE_TYPE_PRIMARY);
  

• if (IS_ERR(plane)) {

• ret = PTR_ERR(plane);
  

• goto err_plane;
  

• }

The crtc should have one primary plane, i think it is more reasonable exynos_drm_crtc_create function creates primary plane.

Yes and it has a primary plane. They are defined in the drivers' struct *_context the same way *_win_data was defined. And they allocated together wit the context struct and and initialized by exynos_plane_init() right before the call to exynos_drm_crtc_create(). Check the fimd_bind() part of this patch for example.

Your approach cannot stop to corrupt primary plane data by overlay plane. We need to separate plane data used by primary plane and plane data used by overlay plane.

Thanks.