On Mon, Oct 18, 2021 at 08:14:04PM +0300, Ville Syrjälä wrote: > On Mon, Oct 18, 2021 at 03:06:34PM +0300, Lisovskiy, Stanislav wrote: > > On Mon, Oct 18, 2021 at 02:50:26PM +0300, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > Chop skl_program_plane() into two halves. Fist half becomes > > > the _noarm() variant, second part the _arm() variant. > > > > > > Fortunately I have already previously grouped the register > > > writes into roughtly the correct order, so the split looks > > > surprisingly clean. > > > > > > A few notable oddities I did not realize were self arming > > > are AUX_DIST and COLOR_CTL. > > > > > > i915_update_info doesn't look too terrible on my cfl running > > > kms_atomic_transition --r plane-all-transition --extended: > > > w/o patch w/ patch > > > Updates: 2178 Updates: 2018 > > > | | > > > 1us | 1us | > > > | | > > > 4us | 4us |***** > > > |********* |********** > > > 16us |********** 16us |******* > > > |*** | > > > 66us | 66us | > > > | | > > > 262us | 262us | > > > | | > > > 1ms | 1ms | > > > | | > > > 4ms | 4ms | > > > | | > > > 17ms | 17ms | > > > | | > > > Min update: 8332ns Min update: 6164ns > > > Max update: 48758ns Max update: 31808ns > > > Average update: 19959ns Average update: 13159ns > > > Overruns > 100us: 0 Overruns > 100us: 0 > > > > > > And with lockdep enabled: > > > w/o patch w/ patch > > > Updates: 2177 Updates: 2172 > > > | | > > > 1us | 1us | > > > | | > > > 4us | 4us | > > > |******* |********* > > > 16us |********** 16us |********** > > > |******* |* > > > 66us | 66us | > > > | | > > > 262us | 262us | > > > | | > > > 1ms | 1ms | > > > | | > > > 4ms | 4ms | > > > | | > > > 17ms | 17ms | > > > | | > > > Min update: 12645ns Min update: 9980ns > > > Max update: 50153ns Max update: 33533ns > > > Average update: 25337ns Average update: 18245ns > > > Overruns > 250us: 0 Overruns > 250us: 0 > > > > > > TODO: On icl+ everything seems to be armed by PLANE_SURF, so we > > > can optimize this even further on modern platforms. But I > > > think there's a bit of refactoring to be done first to > > > figure out the best way to go about it (eg. just reusing > > > the current skl+ functions, or doing a lower level split). > > > > > > TODO: Split scaler programming as well, but IIRC the scaler > > > has some oddball double buffering behaviour on some > > > platforms, so needs proper reverse engineering > > > > > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Should I use that one as a base for further splitting, i.e for DG2? > > Which refactoring has to be done first, as I understand should be > > pretty safe to leave only PLANE_SURF update in arm section, and > > of course scaler is a different thing. > > I'm not really sure which way we should do the skl+ vs. icl+ split. > > Various ideas I've had: > - Definitly pull all the icl+ specific things out from the skl+ > functions and stuff them into icl_plane_update_noarm() > - After that just call the remaining skl_plane_update_noarm()+arm() > back to back from icl_update_noarm() maybe? I don't like this > idea much actually. > - Maybe instead pull some sequences of register writes into small > helpers (eg. colorkey registers could be one). But dunno if there > are other clear groups to make this super useful. > - Or perhaps just pull most fiddly register value calculations > (aux_dist,ckey+alpha things,maybe others) into small helpers > to avoid duplicating themm but otherwise fully duplicate all > the actual register writes? I guess that last thing is what I already did with skl_plane_surf() earlier in the series. So maybe we should just embrace it fully. -- Ville Syrjälä Intel
