Re: [PATCH v4 05/11] drm/sun4i: abstract a engine type

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]

On Tue, Apr 18, 2017 at 07:05:12PM +0800, Icenowy Zheng wrote:
> >> @@ -56,7 +55,7 @@ static void sun4i_crtc_atomic_flush(struct drm_crtc
> >*crtc,
> >>  
> >>  	DRM_DEBUG_DRIVER("Committing plane changes\n");
> >>  
> >> -	sun4i_backend_commit(scrtc->backend);
> >> +	scrtc->engine_ops->commit(scrtc->engine);
> >
> >You rely on the backend having setup things properly, which is pretty
> >fragile. Ideally, you should have a function to check that engine_ops
> >and commit is !NULL, and call it, and the consumers would use that
> >function...
> 
> If it's really NULL how should the function return?

It depends on the return code. ENOSYS if it returns an int, and simply
does nothing if it's a void. I don't think any of the current
functions return an error code at the moment though, so I'd just keep
the current behaviour and just call the function if it's set.

You cannot fail in atomic_flush anyway.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com