[PATCH v3] drm/tidss: dispc: Rewrite naive plane positioning code

[Jyri Sarha]

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