Hi Maxime, On 17/07/17 07:32, Maxime Ripard wrote: > On Thu, Jul 13, 2017 at 05:25:08PM +0100, Kieran Bingham wrote: >>>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c >>>>> index 82b978a5dae6..c2f382feca07 100644 >>>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c >>>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c >>>>> @@ -255,9 +255,9 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state) >>>>> >>>>> /* Apply the atomic update. */ >>>>> drm_atomic_helper_commit_modeset_disables(dev, old_state); >>>>> - drm_atomic_helper_commit_modeset_enables(dev, old_state); >>>>> drm_atomic_helper_commit_planes(dev, old_state, >>>>> DRM_PLANE_COMMIT_ACTIVE_ONLY); >>>> >>>> Except for DRM_PLANE_COMMIT_ACTIVE_ONLY, this function now looks very much like >>>> the default drm_atomic_helper_commit_tail() code. >>>> >>>> Reading around other uses /variants of commit_tail() style functions in other >>>> drivers has left me confused as to how the ordering affects things here. >>>> >>>> Could be worth adding a comment at least to describe why we can't use the >>>> default helper... >>> >>> Or better still ... Use Maxime's new : >>> >>> [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users >> >> Never mind - I've just looked again, and seen that this new helper function is >> the ordering previous to *this* patch, and therefore isn't the same. >> >> However - it's worth noting that Maxime's patch converts this function to the >> new helper - which contradicts this patch's motivations. > > So I guess I should drop the renesas modifications in my serie then? Yes, Please. I think we have a few extra modifications in this function as well which will take it further away from a default implementation. Regards Kieran > > Maxime > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel