On Fri, Jan 24, 2025 at 01:59:15PM +0100, Simona Vetter wrote: > On Fri, Jan 24, 2025 at 02:10:54PM +0200, Ville Syrjälä wrote: > > On Fri, Jan 24, 2025 at 01:14:18PM +0200, Dmitry Baryshkov wrote: > > > There are several drivers which attempt to upgrading the commit to the > > > full mode set from their per-object atomic_check() callbacks without > > > calling the drm_atomic_helper_check_modeset() or > > > drm_atomic_helper_check() again (as requested by those functions). > > > > I don't really understand why any of that is supposedly necessary. > > drm_atomic_helper_check_modeset() is really all about the > > connector routing stuff, so if none of that is changing then there > > is no point in calling it again. Eg. in i915 we call it just at > > the start, and later on we flag internal modesets all over the > > place, but none of those need drm_atomic_helper_check_modeset() > > because nothing routing related will have changed. > > i915 doesn't need this because as you say, it doesn't rely on the atomic > helper modeset tracking much at all, but it's all internal. This is for > drivers which rely more or less entirely on > drm_atomic_crtc_needs_modeset(). > > Also note that it's not just about connector routing, but about adding all > the necessary additional states with > drm_atomic_add_affected_connectors/planes and re-running all the various > state computation hooks for those. Again i915 hand-rolls that all. IIRC it only runs the connectors' atomic_check(), nothing else really. But maybe that's changed since I last looked at it. Anyways it feels like we're throwing everything and the kitchen sink into a single function here. Maybe it should be split into two or more functions with clear responsibilities? > > So yeah i915 doesn't need this. > -Sima > > > > > > > > > As discussed on IRC, add separate atomic_needs_modeset() callbacks, > > > whose only purpose is to allow the plane, connector, encoder or CRTC to > > > specify that for whatever reasons corresponding CRTC should undergo a > > > full modeset. The helpers will call these callbacks in a proper place, > > > adding affected objects and calling required functions as required. > > > > > > The MSM patches depend on the msm-next tree and the series at [1]. The > > > plan is to land core changes through drm-misc, then merge drm-misc-next > > > into msm-next and merge remaining msm-specific patches through the > > > msm-next tree. > > > > > > [1] https://lore.kernel.org/dri-devel/20250123-drm-dirty-modeset-v2-0-bbfd3a6cd1a4@xxxxxxxxxx/ > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > > --- > > > Dmitry Baryshkov (6): > > > drm/atomic-helper: add atomic_needs_modeset callbacks > > > drm/mgag200: move format check to drm_plane_helper.atomic_needs_modeset > > > drm/msm/dpu: stop upgrading commits by enabling allow_modeset > > > drm/msm/dpu: move CTM check to drm_crtc_helper.atomic_needs_modeset > > > drm/msm/dpu: use atomic_needs_modeset for CDM check > > > drm/msm: drop msm_atomic_check wrapper > > > > > > drivers/gpu/drm/drm_atomic_helper.c | 59 ++++++++++++++++++ > > > drivers/gpu/drm/mgag200/mgag200_drv.h | 2 + > > > drivers/gpu/drm/mgag200/mgag200_mode.c | 27 ++++++--- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 +++++ > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 44 +++++++++----- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 -- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 26 -------- > > > drivers/gpu/drm/msm/msm_atomic.c | 29 --------- > > > drivers/gpu/drm/msm/msm_drv.h | 1 - > > > drivers/gpu/drm/msm/msm_kms.c | 2 +- > > > drivers/gpu/drm/msm/msm_kms.h | 7 --- > > > include/drm/drm_modeset_helper_vtables.h | 92 +++++++++++++++++++++++++++++ > > > 12 files changed, 219 insertions(+), 89 deletions(-) > > > --- > > > base-commit: 0936f0e54426177b0f0263ddf806ed5e13487db6 > > > change-id: 20250123-atomic-needs-modeset-8f6a8243a3f7 > > > prerequisite-change-id: 20241222-drm-dirty-modeset-88079bd27ae6:v2 > > > prerequisite-patch-id: 0c61aabfcd13651203f476985380cbf4d3c299e6 > > > prerequisite-patch-id: c6026f08011c288fd301676e9fa6f46d0cc1dab7 > > > prerequisite-patch-id: b0cb06d5c88791d6e4755d879ced0d5050aa3cbf > > > prerequisite-patch-id: fd72ddde9dba0df053113bc505c213961a9760da > > > > > > Best regards, > > > -- > > > Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > > > -- > > Ville Syrjälä > > Intel > > -- > Simona Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel
