On 13/02/2020 12:49, Tomi Valkeinen wrote: > On 13/02/2020 12:44, Jyri Sarha wrote: > >> + /* >> + * If a plane on a CRTC changes add all active planes on that >> + * CRTC to the atomic state. This is needed for updating the >> + * plane positions in tidss_crtc_position_planes() which is >> + * called from crtc_atomic_enable() and crtc_atomic_flush(). >> + * The update is needed for x,y-position changes too, so >> + * zpos_changed condition is not enough and we need this if >> + * planes_changed is true too. >> + */ >> + for_each_new_crtc_in_state(state, crtc, cstate, i) { >> + if (cstate->zpos_changed || cstate->planes_changed) { >> + ret = drm_atomic_add_affected_planes(state, crtc); >> + if (ret) >> + return ret; >> + } >> + } > > I think 99.99...% of the commits are such where only planes' fb address > is changed. I think "planes_changed" is true for these. I wonder if it > would be a sensible optimization to skip this for those 99.99...% cases. > Can we detect that easily? > Sure by looping all planes in the state through with for_each_oldnew_plane_in_state() and checking if crtc_x or crtc_y has changed. But then again writing the positions of max 4 planes is really not that heavy operation. There is more calculation to do and more registers to write when updating the fp, so I do not think avoiding the OVR update justifies the extra complexity. > Well, we haven't optimized for those 99.99...% cases anywhere else > either, so it's possible it doesn't matter. > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel