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:
- 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.
- 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.
- 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 ...
- 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch