On Mon, 2 Jul 2018 12:14:20 +0100 Liviu Dudau Liviu.Dudau@arm.com wrote:
On Mon, Jul 02, 2018 at 11:49:11AM +0200, Boris Brezillon wrote:
On Mon, 2 Jul 2018 09:51:46 +0200 Daniel Vetter daniel@ffwll.ch wrote:
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.
I'm just curious, from where do you queue the writeback job if you don't have a connector->atomic_commit() hook implemented? AFAICT, the encoder->enable() method is only called when the encoder is being enabled, and not every time you update the FB_ID prop of the writeback connector. Am I missing something?
In malidp_drv.c:malidp_atomic_commit_tail() we call malidp_mw_atomic_commit() after drm_atomic_helper_commit_planes(drm, state, DRM_PLANE_COMMIT_ACTIVE_ONLY).
malidp_mw_atomic_commit() then checks the writeback job and if it has an fb associated with it then it calls drm_writeback_queue_job() before enabling the memwrite hardware bits.
Okay. That's a good reason to make it optional.
Thanks,
Boris