Re: [PATCH v2 2/8] drm/connector: Pass a drm_connector_state to ->atomic_commit()

From: Daniel Vetter <daniel@ffwll.ch>
To: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: David Airlie <airlied@linux.ie>,
	Boris Brezillon <boris.brezillon@bootlin.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 2/8] drm/connector: Pass a drm_connector_state to ->atomic_commit()
Date: Mon, 2 Jul 2018 09:51:46 +0200	[thread overview]
Message-ID: <20180702075146.GC13978@phenom.ffwll.local> (raw)
In-Reply-To: <20180629113710.GN2037@e110455-lin.cambridge.arm.com>

On Fri, Jun 29, 2018 at 12:37:10PM +0100, Liviu Dudau wrote:
> On Fri, Jun 29, 2018 at 01:17:15PM +0200, Boris Brezillon wrote:
> > Other atomic hooks are passed state objects, let's change this one to
> > be consistent.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c      | 2 +-
> >  include/drm/drm_modeset_helper_vtables.h | 4 +++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 17baf5057132..69063bcf2334 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1187,7 +1187,7 @@ static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
> >  
> >  		if (new_conn_state->writeback_job && new_conn_state->writeback_job->fb) {
> >  			WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK);
> > -			funcs->atomic_commit(connector, new_conn_state->writeback_job);
> > +			funcs->atomic_commit(connector, new_conn_state);
> 
> Forgot to add: I think it is worth adding a check here that the hook has
> been implemented by the driver, AFAIK it is not a mandatory hook, even
> for writeback enabled drivers.

Either way this should be documented in the hook (atm it says nothing
about whether it's mandatory/optional and for whom).
> 
> Let me know what you plan to do with this patch as I will have to update
> the malidp patches as well, otherwise linux-next is going to flag me.

Probably simplest if you send a pull to Dave right away with the current
malidp code, then we backmerge that into drm-misc-next, and Boris rebases.
There's no point in hanging onto pull requests until right before the
feature freeze, just makes things more complicated.

Patch itself lgtm.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> Best regards,
> Liviu
> 
> >  		}
> >  	}
> >  }
> > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> > index 3b289773297c..fb841f44949c 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -980,11 +980,13 @@ struct drm_connector_helper_funcs {
> >  	 *
> >  	 * This hook is to be used by drivers implementing writeback connectors
> >  	 * that need a point when to commit the writeback job to the hardware.
> > +	 * The writeback_job to commit is available in
> > +	 * &drm_connector_state.writeback_job.
> >  	 *
> >  	 * This callback is used by the atomic modeset helpers.
> >  	 */
> >  	void (*atomic_commit)(struct drm_connector *connector,
> > -			      struct drm_writeback_job *writeback_job);
> > +			      struct drm_connector_state *state);
> >  };
> >  
> >  /**
> > -- 
> > 2.14.1
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel