Re: [Intel-gfx] [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Kandpal,

Thank you for the patch.

On Wed, Feb 02, 2022 at 02:24:28PM +0530, Kandpal Suraj wrote:
> Changing rcar_du driver to accomadate the change of
> drm_writeback_connector.base and drm_writeback_connector.encoder
> to a pointer the reason for which is explained in the
> Patch(drm: add writeback pointers to drm_connector).
> 
> Signed-off-by: Kandpal Suraj <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h      | 2 ++
>  drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 8 +++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> index 66e8839db708..68f387a04502 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> @@ -72,6 +72,8 @@ struct rcar_du_crtc {
>  	const char *const *sources;
>  	unsigned int sources_count;
>  
> +	struct drm_connector connector;
> +	struct drm_encoder encoder;

Those fields are, at best, poorly named. Furthermore, there's no need in
this driver or in other drivers using drm_writeback_connector to create
an encoder or connector manually. Let's not polute all drivers because
i915 doesn't have its abstractions right.

Nack.

>  	struct drm_writeback_connector writeback;
>  };
>  
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
> index c79d1259e49b..5b1e83380c47 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
> @@ -200,8 +200,10 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu,
>  {
>  	struct drm_writeback_connector *wb_conn = &rcrtc->writeback;
>  
> -	wb_conn->encoder.possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
> -	drm_connector_helper_add(&wb_conn->base,
> +	wb_conn->base = &rcrtc->connector;
> +	wb_conn->encoder = &rcrtc->encoder;
> +	wb_conn->encoder->possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
> +	drm_connector_helper_add(wb_conn->base,
>  				 &rcar_du_wb_conn_helper_funcs);
>  
>  	return drm_writeback_connector_init(&rcdu->ddev, wb_conn,
> @@ -220,7 +222,7 @@ void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc,
>  	struct drm_framebuffer *fb;
>  	unsigned int i;
>  
> -	state = rcrtc->writeback.base.state;
> +	state = rcrtc->writeback.base->state;
>  	if (!state || !state->writeback_job)
>  		return;
>  

-- 
Regards,

Laurent Pinchart