On Fri, Mar 04, 2016 at 05:20:00PM -0500, Alex Deucher wrote: > Code complexities and internal abstractions aside, we've implemented > the API and tested it and it appears to work. What sort of additional > semantics does atomic imply that you don't think DAL would handle? Michel asked me to follow up here. I thought I've followed up on irc. Anyway the big problem and reason why I didnt' fully dig into things are that reading 100kloc for an atomic review when it's clear the code overall needs some serious love is something I didn't want to do. I'm still up to reading things once they look a bit more reasonable, and it's not massively more work than all the other atomic drivers/conversions I looked at. So without looking again, with a few months of brain memory degradation: - The glue looked fairly incomplete, e.g. it wasn't using all the legacy2atomic helpers we have. Which likely means it wasn't really tested much. - Not universal planes, hence atomic plane updates not possible. - It did roll it's own commit (which is how this is meant to be really in the end), but I did not see the a clear reason, and it didn't seem to have been closely modelled after atomic helpers. Not necessarily bad, just increases chances that some of the semantics are wrong. And the more fundamental thing, but the one I couldn't check because abstraction: - Doesn't seem to stage derived state needed to check limits (e.g. clocks, memory bandwidth) in something subclassing drm_*_state structures. Either that means you're rolling your own atomic machinery (no-go) or you get it wrong (no-go). But since DAL completely forgoes drm structures I didn't have a map to read the code and got lost. - It looked liked DAL works with a try_commit()/rollback() model, but atomic requires a check()/commit() model. And the check phase is not allowed to change _any_ persistent state (whether sw or hw). That's why atomic needs to stage everything that might change, and which might need to be checked up-front in these free-standing state structures. I think for me to be able to actually give you a clear answer here we'd need 2 things: - Handle all the list of cleanup tasks to get rid of reinvented code. - Embed drm_* structs in corresponding DAL structs to really link concepts together, and have some kind of map to be able to read the code. I think only with that it's possible to have a clear idea whether DAL needs to be redesigned to be able to implement atomic, or not. Again: I din't look at the code, this is purely from memory. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
