Re: [RFC 3/6] drm/exynos: introduce struct exynos_drm_plane_config

[Gustavo Padovan <gustavo@padovan.org>]

Hi Tobias,

2015-04-15 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:

> Hello Gustavo!
> 
> Gustavo Padovan wrote:
> > Hi Tobias,
> > 
> > 2015-04-15 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
> > 
> >> This struct encapsulates the configuration for a plane
> >> object. The pixel format config is currently unused.
> >>
> >> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> >> ---
> >>  drivers/gpu/drm/exynos/exynos7_drm_decon.c | 17 ++++++++++-------
> >>  drivers/gpu/drm/exynos/exynos_drm_drv.h    | 19 +++++++++++++++++++
> >>  drivers/gpu/drm/exynos/exynos_drm_fimd.c   | 17 ++++++++++-------
> >>  drivers/gpu/drm/exynos/exynos_drm_plane.c  | 14 +++++++-------
> >>  drivers/gpu/drm/exynos/exynos_drm_plane.h  |  3 +--
> >>  drivers/gpu/drm/exynos/exynos_drm_vidi.c   | 17 ++++++++++-------
> >>  drivers/gpu/drm/exynos/exynos_mixer.c      | 17 ++++++++++-------
> >>  7 files changed, 67 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> >> index 84a3638..ca70599 100644
> >> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> >> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> >> @@ -756,8 +756,8 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
> >>  	struct decon_context *ctx = dev_get_drvdata(dev);
> >>  	struct drm_device *drm_dev = data;
> >>  	struct exynos_drm_plane *exynos_plane;
> >> -	enum drm_plane_type type;
> >> -	unsigned int zpos;
> >> +	struct exynos_drm_plane_config plane_config = { 0 };
> >> +	unsigned int i;
> >>  	int ret;
> >>  
> >>  	ret = decon_ctx_initialize(ctx, drm_dev);
> >> @@ -766,11 +766,14 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
> >>  		return ret;
> >>  	}
> >>  
> >> -	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
> >> -		type = (zpos == ctx->default_win) ? DRM_PLANE_TYPE_PRIMARY :
> >> -						DRM_PLANE_TYPE_OVERLAY;
> >> -		ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
> >> -					1 << ctx->pipe, type, zpos);
> >> +	plane_config.possible_crtcs = 1 << ctx->pipe;
> >> +
> >> +	for (i = 0; i < WINDOWS_NR; i++) {
> >> +		plane_config.type = (i == ctx->default_win) ?
> >> +			DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
> >> +		plane_config.zpos = i;
> >> +
> >> +		ret = exynos_plane_init(drm_dev, &ctx->planes[i], &plane_config);
> >>  		if (ret)
> >>  			return ret;
> >>  	}
> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> >> index 4c14a89..35698f3 100644
> >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> >> @@ -116,6 +116,25 @@ struct exynos_drm_plane {
> >>  };
> >>  
> >>  /*
> >> + * Exynos DRM plane configuration structure.
> >> + *
> >> + * @possible_crtcs: bitfield describing the valid CRTCs
> >> + *			for this plane.
> >> + * @type: plane type (primary, overlay, etc.)
> >> + * @zpos: z-position of the plane.
> >> + * @pixel_formats: supported pixel formats.
> >> + * @num_pixel_formats: number of elements in 'pixel_formats'.
> >> + */
> >> +
> >> +struct exynos_drm_plane_config {
> >> +	unsigned long possible_crtcs;
> >> +	enum drm_plane_type type;
> >> +	unsigned int zpos;
> >> +	const uint32_t *pixel_formats;
> >> +	unsigned int num_pixel_formats;
> >> +};
> > 
> > As a follow up of my atomic series I started cleaning up exynos drm a bit more
> > and right now I'm removing most of struct exynos_drm_plane. Most of the plane
> > information there is also present in plane->state thus I'm basically removing
> > all the duplicated information there.
> Sounds like a good plan.
> 
> 
> > That said, I think we avoid creating exynos_drm_plane_config and stuff
> > everything directly in struct exynos_drm_plane, it will be quite small and
> > easier to manipulate.
> So that would imply that we then just have:
> int exynos_plane_init(struct drm_device *dev, struct exynos_drm_plane
> *exynos_plane);
> 
> Correct?

Correct, passing exynos_drm_plane simplifies things a lot for us.

> 
> Anyway, I'm going to wait then until the cleanups are posted and rebase
> this series onto it.

Right, I'll probably have that the next week or the other one, I'm currently
working on some testing scripts to speed up my testing and make sure I'm not
breaking anything.

	Gustavo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel