On 12.05.2017 09:32, Daniel Vetter wrote: > On Thu, May 11, 2017 at 10:05:56AM +0100, Jose Abreu wrote: >> This series is a follow up from the discussion at [1]. We start by >> introducing crtc->mode_valid(), encoder->mode_valid() and >> bridge->mode_valid() callbacks which will be used in followup >> patches and also by cleaning the documentation a little bit. >> >> We proceed by introducing new helpers to call this new callbacks >> at 2/6. >> >> At 3/6 a helper function is introduced that calls all mode_valid() >> from a set of bridges. >> >> Next, at 4/6 we modify the connector probe helper so that only modes >> which are supported by a given bridge+encoder+crtc combination are >> probbed. >> >> At 5/6 we call all the mode_valid() callbacks for a given pipeline, >> except the connector->mode_valid one, so that the mode is validated. >> This is done before calling mode_fixup(). >> >> Finally, at 6/6 we use the new crtc->mode_valid() callback in arcpgu >> and remove the atomic_check() callback. >> >> [1] https://patchwork.kernel.org/patch/9702233/ >> >> Jose Abreu (6): >> drm: Add crtc/encoder/bridge->mode_valid() callbacks >> drm: Add drm_{crtc/encoder/connector}_mode_valid() >> drm: Introduce drm_bridge_mode_valid() >> drm: Use new mode_valid() helpers in connector probe helper >> drm: Use mode_valid() in atomic modeset >> drm: arc: Use crtc->mode_valid() callback >> >> Cc: Carlos Palminha <palminha@xxxxxxxxxxxx> >> Cc: Alexey Brodkin <abrodkin@xxxxxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >> Cc: Dave Airlie <airlied@xxxxxxxx> >> Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx> >> Cc: Archit Taneja <architt@xxxxxxxxxxxxxx> > Commented with an entire patch on patch 1, patches 2-5 are all > > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > But I think some more acks/r-bs would be really good, since this is quite > a bit change in the helper infrastructure. But otherwise ready for merging > imo. Can you pls also review my proposal for patch 1? > > Thanks, Daniel As the patchset improves many things, I would like to point here that there are still issues with mode probing at least in case of panels. Panels in general does not provide discrete list of supported modes, but they provide list of supported mode ranges (named display_timings), at the moment this is only addressed in drm_panel_funcs::get_timings, but drm core does not use it at all. Currently most of the panel drivers advertises only fixed list of arbitrary chosen modes, and if bridge/connector/encoder/crtc does not support it pipeline does not work, even if slightly different configuration could work. Of course there are workarounds but maybe it would be good to replace drm_connector::modes with drm_connector::timings. This way set of valid timings of the whole pipeline will be intersection of sets of valid timings of every component of the pipeline - quite straightforward and simple construct. As this is just an idea which came to me during patchset review, it is not backed by any code, but maybe it can be interesting for quick brainstorm. Regards Andrzej > >> drivers/gpu/drm/arc/arcpgu_crtc.c | 39 ++++++---- >> drivers/gpu/drm/drm_atomic_helper.c | 76 +++++++++++++++++++- >> drivers/gpu/drm/drm_bridge.c | 33 +++++++++ >> drivers/gpu/drm/drm_crtc_helper_internal.h | 13 ++++ >> drivers/gpu/drm/drm_probe_helper.c | 103 ++++++++++++++++++++++++++- >> include/drm/drm_bridge.h | 22 ++++++ >> include/drm/drm_modeset_helper_vtables.h | 110 ++++++++++++++++++++++------- >> 7 files changed, 348 insertions(+), 48 deletions(-) >> >> -- >> 1.9.1 >> >> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
