On Mon, Feb 27, 2017 at 10:03:20PM -0800, John Stultz wrote: > On Mon, Feb 20, 2017 at 2:32 PM, Daniel Vetter <daniel@xxxxxxxx> wrote: > > I thought about this some more, I think what we need to fix this mess > > properly is: > > - mode_valid helper callbacks for crtc, encoder, bridges, with the > > same interface as for connectors. > > - calling all these new mode_valid hooks from the probe helpers, but > > with the restriction that we only reject a mode if all possible > > crtc/encoders combos reject it. We need to filter by > > possible_encoders/crtcs for these checks. Bridges have a fixed routing > > to their encoder, so those are easy. > > So... my ignorance here my have complicated things a bit. > > Xinliang can correct me here, if I get this wrong. :) > > So in the past, when we ran into this issue, we just hacked in a mode > white list into the adv7511 bridge mode_valid check, as it was the > easiest place. But as I understood it, the limitation came from logic > in (what is now an early version of) the kirin ade driver. > > However, since kirin made it upstream, the code was refactored and I > believe the specific code was moved to the dsi_calc_phy_rate() which > is part of the dsi encoder, not the crtc code. > > So in my limited understanding I initially tried to add the mode_check > logic in the ade crtc driver, but to me (and I could have this wrong > again) it seems it really ought to be in the encoder (where we > probably could do better then using a white list by doing the mode > calculations and seeing if we get a value that matches what was > requested). > > I'm not sure if that simplifies your proposal (possibly avoiding the > crtc/encoder combo checks?), or if you think we still need the big > grand solution? We'll need the grand plan for other drivers I think. Or at least that's what I'm trying to volunteer you for :-) If you just want to fix up your specific combo, then you can just hack up that driver. You don't need any core/helper changes for that. > > - add calls to mode_valid in the atomic helpers, right before we call > > mode_fixup or atomic_check in drm_atomic_helper_check_modesets. > > - convert drivers to move code from mode_fixup into mode_valid > > wherever possible, to make sure we can share as much of the check > > logic between probe and atomic comit code. > > - update docs for all the hooks, plus update the overview sections accordingly. > > > > I think this should give us a good approximation of nirvana. For I > > long time I thought we could get away without adding mode_valid > > everywhere, but in the probe paths we really can't fake the > > adjusted_mode (and other atomic state), so adding dedicated hooks > > which are called from both places is probably the only option. > > So my mental map of the DRM code is all pretty hazy and limited here, > but the above sounds reasonable. I'll try to trace through and better > understand what specifically you're asking for and get something > started here. Yeah, we're still lacking the nice overview graphs for the drm docs. I have them here, but the kbuild scripts need polish :( -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
