On Tue, Feb 14, 2017 at 11:38 AM, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Tue, Feb 14, 2017 at 8:25 PM, John Stultz <john.stultz@xxxxxxxxxx> wrote: >> Currently, on the hikey board, we have the adv7511 bridge wired >> up to the kirin ade drm driver. Unfortunately, the kirin ade >> core cannot generate accurate byteclocks for all pixel clock >> values. >> >> Thus if a mode clock is selected that we cannot calculate a >> matching byteclock, the device will boot with a blank screen. >> >> Unfortunately, currently the only place we can properly check >> potential modes for this issue in the connector mode_valid >> helper. Again, hikey uses the adv7511 bridge, which is shared >> between a number of different devices, so its improper to put >> restrictions caused by the kirin drm driver in the adv7511 >> logic. >> >> So this patch tries to correct for that, by adding some >> infrastructure so that the drm_crtc_helper_funcs can optionally >> implement a mode_valid check, so that the probe helpers can >> check to make sure there are not any restrictions at the crtc >> level as well. >> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> >> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> >> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx> >> Cc: David Airlie <airlied@xxxxxxxx> >> Cc: Rob Clark <robdclark@xxxxxxxxx> >> Cc: Xinliang Liu <xinliang.liu@xxxxxxxxxx> >> Cc: Xinliang Liu <z.liuxinliang@xxxxxxxxxxxxx> >> Cc: Rongrong Zou <zourongrong@xxxxxxxxx> >> Cc: Xinwei Kong <kong.kongxinwei@xxxxxxxxxxxxx> >> Cc: Chen Feng <puck.chen@xxxxxxxxxxxxx> >> Cc: Archit Taneja <architt@xxxxxxxxxxxxxx> >> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx >> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx> > > So I'm going to be super-annoying here and ask for a complete > solution. This here is defacto what ever driver already does (or has > too), but it doesn't really solve the overall issue of having entirely > separate validation paths for probe and atomic_check paths. I think if > we wan to solve this, we need to solve this properly, with a generic > solution. That would mean: > - still in helpers, to make it all opt-int Just to be clear, I believe what I proposed is opt-in, but I assume you want that in addition to the following, right? > - covers crtc and encoders and bridges So you'd like similar mode_valid() calls in the crtc/encoder/bridge helpers? right? > - allows you to implement the current mode_valid in terms of the new > stuff (maybe as a default hook) This bit I'm not sure I'm following. The current drm_connector's mode_valid in terms of a new mode_valid call that also looks at crtc/encoder/bridges? Or do you mean something else? > - allows you to implement the current assortment of mode_fixup and/or > atomic_check in terms of the new stuff, or at least to not have to > duplicate logic in there This is over my head, so I'll have to research to better understand. > Docs for all this, especially updating all the warnings on how to use > the existing hooks correctly. That's fair. > I think just pushing this specific case into the helpers, without > rethinking the overall big picture, isn't gaining us all that much. > For just this I'd say just put it into drivers, until we have some Not following here. What do you mean by "put it into drivers"? Where? thanks -john _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
