On Wed, May 25, 2016 at 6:30 PM, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Wed, May 25, 2016 at 05:37:41PM +0800, Ying Liu wrote: >> On Tue, May 24, 2016 at 6:57 PM, Daniel Vetter <daniel@xxxxxxxx> wrote: >> > On Tue, May 24, 2016 at 06:10:44PM +0800, Liu Ying wrote: >> >> Since CRTC has already been disabled in crtc_funcs->prepare(), it doesn't hurt >> >> to disable the primary plane in drm_helper_crtc_mode_set() before enabling it >> >> in drm_helper_crtc_mode_set_base(). This makes those who reject active plane >> >> update in plane_funcs->atomic_check() happy. >> > >> > How/where exactly do you blow up? >> >> >> >> Signed-off-by: Liu Ying <gnuiyl@xxxxxxxxx> >> >> --- >> >> drivers/gpu/drm/drm_crtc_helper.c | 9 +++++++++ >> >> 1 file changed, 9 insertions(+) >> >> >> >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c >> >> index 79555d2..7fabcd7 100644 >> >> --- a/drivers/gpu/drm/drm_crtc_helper.c >> >> +++ b/drivers/gpu/drm/drm_crtc_helper.c >> >> @@ -1013,6 +1013,15 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod >> >> goto out; >> >> } >> >> >> >> + /* >> >> + * It doesn't hurt to disable primary plane here since crtc is off >> >> + * and we'll enable it again in drm_helper_crtc_mode_set_base() >> >> + * below soon. >> >> + */ >> >> + ret = drm_plane_helper_disable(crtc->primary); >> > >> > You can't call this helper from crtc helpers. drm_plane_helper_disable >> > assumes that a) you've already extracted universal plane support for the >> > primary plane by registering a proper drm_plane for it b) that you're >> > driver is at least partially using atomic hooks already. >> > >> > Both assumptions are wrong for all current users of this function. >> > drm_helper_crtc_mode_set() is purely a legacy helper for legecy drivers. >> >> Isn't the function a transitional helper, just as the kdoc claims? >> It looks all current users are sort of atomic users already. > > drm_helper_crtc_mode_set is exclusively used by legacy drivers. > drm_plane_helper_disable can be used for transitioning to atomic. Again, please take a look at the kdoc of drm_helper_crtc_mode_set. It reads 'Besides the atomic plane helper functions for the primary plane the driver must also provide the ->mode_set_nofb callback to set up the CRTC. This is a transitional helper useful for converting drivers to the atomic interfaces.' > > Calling the latter from the former means you break every non-transition > legacy driver. drm_helper_crtc_mode_set calls drm_helper_crtc_mode_set_base. Both drm_helper_crtc_mode_set_base and drm_plane_helper_disable call drm_plane_helper_commit, so they are somewhat counterparts, that is to say, drm_helper_crtc_mode_set may call drm_plane_helper_disable technically. Regards, Liu Ying > -Daniel > >> >> Regards, >> Liu Ying >> >> > -Daniel >> > >> >> + if (ret) >> >> + goto out; >> >> + >> >> swap(crtc->state, crtc_state); >> >> >> >> crtc_funcs->mode_set_nofb(crtc); >> >> -- >> >> 2.7.4 >> >> >> >> _______________________________________________ >> >> dri-devel mailing list >> >> dri-devel@xxxxxxxxxxxxxxxxxxxxx >> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> > >> > -- >> > Daniel Vetter >> > Software Engineer, Intel Corporation >> > http://blog.ffwll.ch > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
