On Tue, Dec 27, 2016 at 07:41:47PM +0100, Daniel Vetter wrote: > On Tue, Dec 27, 2016 at 06:21:47PM +0200, Jani Nikula wrote: > > Hi all - > > > > This series aims at three goals: > > > > 1) Most drivers do similar things around drm_get_edid (namely convert > > edid to eld, add modes, and update edid blob property). Add a helper for > > the drivers, to reduce code and unify. > > I like. > > > 2) Move override and firmware EDID handling to a lower level, in the > > helper. This makes them more transparent to the rest of the stack, > > avoiding special casing. For example, any drm_detect_hdmi_monitor calls > > typically use the EDID from the display, not the override EDID. > > I replied to that patch, I think we need to rethink the helper callbacks > to make this work. The general direction make sense to me though. > > > 3) Make EDID caching easier and unified across drivers. Currently, > > plenty of drivers have their own hacks for caching EDID reads. This > > could be made more transparent and unified at the helper level. > > Caching is Real Hard, and I don't think something generic will work: > - On DP, hpd works and will reliable tell you when you need to invalidate > stuff. Except when you have a downstream port to something else (hdmi, > vga, whatever). I think this is best solved by making the shared helpers > for DP a lot better. > > - HDMI is supposed to work, except it's not. You can't rely on hpd, any > caching needs to have a time limit. I guess we could share some code > here. > > - Everything else is much worse, and caching is a no-go. At most a > time-based cache that invalidates and re-probes. > > - Built-in panels are special. > > - One issue with all this is that currently the hpd helpers we have (not > the stuff in i915) don't even forward which hpd signalled which > connector. > > - Another fun is handling invalidations in general. System suspend/resume > is real fun that way ... > > 4) Fix the locking (well, entire lack thereof) between the probe paths > (protected by mode_config.mutex), and the atomic modeset paths (protected > by mode_config.connection_mutex). > > Yes that's feature creep but I think any redesign of the probe code should > have a answer to that too. 5) Also integrate the connector status lifetime counter thing I discussed with Chris, so that we can start to catch&report changes besides connector->status (which is atm the only thing the probe helpers bother to track). E.g. if you have hdmi repeaters, they indicate downstream changes in a changed edid. Currently we fail to send out a hdp event for that. -Daniel > > > All of this is opt-in, using the helper from patch 6/7. This is mostly > > because converting everything at one go is pretty much impossible. The > > main wrinkle is having to leave override/firmware EDID handling in two > > places for now, but this could be fixed once enough drivers have > > switched to using the common helper. > > I feel like we'd need a bit more conversion when merging, to make sure it > all makes sense. Ending up with 2 not really useful ways to handle probing > in the helpers would be worse than what we have now. > -Daniel > > > > > BR, > > Jani. > > > > > > Jani Nikula (7): > > drm: reset ELD if NULL edid is passed to drm_edid_to_eld > > drm: move edid property update and add modes out of edid firmware > > loader > > drm: abstract override and firmware edid > > drm: export load edid firmware > > drm: make drm_get_displayid() available outside of drm_edid.c > > drm: add new drm_mode_connector_get_edid to do drm_get_edid and > > friends > > drm/i915: replace intel_ddc_get_modes with drm_mode_connector_get_edid > > > > drivers/gpu/drm/drm_connector.c | 60 ++++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/drm_edid.c | 10 +++---- > > drivers/gpu/drm/drm_edid_load.c | 18 ++++-------- > > drivers/gpu/drm/drm_probe_helper.c | 45 +++++++++++++++++++++------- > > drivers/gpu/drm/i915/intel_drv.h | 1 - > > drivers/gpu/drm/i915/intel_dvo.c | 5 ++-- > > drivers/gpu/drm/i915/intel_modes.c | 23 --------------- > > drivers/gpu/drm/i915/intel_sdvo.c | 2 +- > > include/drm/drm_connector.h | 6 ++++ > > include/drm/drm_edid.h | 8 +++-- > > 10 files changed, 120 insertions(+), 58 deletions(-) > > > > -- > > 2.1.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
