On 10/11/22 15:21, Thomas Zimmermann wrote: > Hi > > Am 10.10.22 um 19:02 schrieb Javier Martinez Canillas: >> Provides a default CRTC state check handler for CRTCs that only have one >> primary plane attached. >> >> There are some drivers that duplicate this logic in their helpers, such as >> simpledrm and ssd130x. Factor out this common code into a CRTC helper and >> make drivers use it. >> >> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> > > Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > > There really isn't much here for now. I suspect that there are more > drivers that could use this helper. If you merge this before ofdrm, I'll > rebase ofdrm on top. > Sure. I probably won't post a v2 until tomorrow and I believe ofdrm is ready to be merged, so I'll just rebase this series on top of that once just push it. > Please also see my comment below. > > [...] >> +/** >> + * drm_crtc_helper_atomic_check() - Helper to check CRTC atomic-state >> + * @crtc: CRTC to check >> + * @state: atomic state object >> + * >> + * Provides a default CRTC-state check handler for CRTCs that only have >> + * one primary plane attached to it. >> + * >> + * This is often the case for the CRTC of simple framebuffers. > > I'd add a reference to drm_plane_helper_atomic_check() to this > paragraph. Like > > See drm_plane_helper_atomic_check() for the respective plane helpers. > > And also reference back from the plane-check helper to the CRTC-check > helper. > Good idea, I'll do that. Thanks for your review. > Best regards > Thomas > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
