Hi Am 19.11.20 um 15:32 schrieb Maxime Ripard:
On Thu, Nov 19, 2020 at 10:12:43AM +0100, Thomas Zimmermann wrote:Hi Am 05.11.20 um 14:56 schrieb Maxime Ripard:The current HVS muxing code will consider the CRTCs in a given state to setup their muxing in the HVS, and disable the other CRTCs muxes. However, it's valid to only update a single CRTC with a state, and in this situation we would mux out a CRTC that was enabled but left untouched by the new state. Fix this by setting a flag on the CRTC state when the muxing has been changed, and only change the muxing configuration when that flag is there. Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically") Reviewed-by: Hoegeun Kwon <hoegeun.kwon@xxxxxxxxxxx> Tested-by: Hoegeun Kwon <hoegeun.kwon@xxxxxxxxxxx> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx> --- drivers/gpu/drm/vc4/vc4_drv.h | 1 + drivers/gpu/drm/vc4/vc4_kms.c | 82 ++++++++++++++++++++--------------- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 014113823647..325b53ff11b3 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -524,6 +524,7 @@ struct vc4_crtc_state { struct drm_mm_node mm; bool feed_txp; bool txp_armed; + bool needs_muxing;Maybe rather 'update_muxing'. More generally, I'd separate fields that contain actual CRTC state, such assigned_channel, from those that only contain transitional state during update-commit, such as needs_muxing.How would you separate them? I'll use your other suggestions, thanks!
I'd put the transitional state at the end of the struct and put a comment above. Something like 'Transitional state below; only valid during atomic commits'.
Best regards Thomas
Maxime
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
OpenPGP_0x680DC11D530B7A23.asc
Description: application/pgp-keys
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature