[Intel-gfx] [PATCH v5 0/2] drm: Add detection of changing of edid on between suspend and resume
gwan-gyeong.mun at intel.com
Wed Apr 17 19:40:30 UTC 2019
On Mon, 2019-04-15 at 18:32 +0200, Daniel Vetter wrote:
> On Thu, Apr 11, 2019 at 05:36:30PM +0300, Gwan-gyeong Mun wrote:
> > This patch series fix missed detection of changing of edid on
> > between
> > suspend and resume.
> > First patch fixes drm_helper_hdp_irq_event() in order to fix a
> > below use
> > case.
> > Following scenario requires detection of changing of edid.
> > 1) plug display device to a connector
> > 2) system suspend
> > 3) unplug 1)'s display device and plug the other display device
> > to a
> > connector
> > 4) system resume
> > It adds edid check routine when a connector status still remains as
> > "connector_status_connected".
> > Second patch adds a missed update of edid property of drm connector
> > on i915.
> > v2: Add NULL check before comparing of EDIDs.
> > v3: Make it as part of existing drm_helper_hpd_irq_event() (Stan,
> > Mika)
> > v4: Rebased
> > v5: Use a cached edid property blob data of connector instead of
> > adding
> > a new detected_edid variable. (Maarten)
> > Add an using of reference count for getting a cached edid
> > property
> > blob data. (Maarten)
> > Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
> > Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
> > Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
> > Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
> > v1, v2: https://patchwork.freedesktop.org/series/47680/
> > v3: https://patchwork.freedesktop.org/series/49298/
> > v4: https://patchwork.freedesktop.org/series/57397/
> I guess I missed this, but there's a few fundamental things here
> - patch 2 is fallout from the fairly abitrary split between ->detect
> ->get_modes. There's not really any good reason for that, I think
> we can
> just always call ->get_modes if ->detect says the connector is
> connected. There's more than just dp and hdmi (that's simply the 2
> things we test in CI using chamelium).
Thank you for checking it.
I agree. I'll modify first patch with your guide and will resend it.
> - the problem is bigger than just the edid changing (e.g. when
> change, or when something else changes aside from the edid, e.g. in
> aux, like how many lanes the dp cable has). Plus we have this long-
> idea to give userspace a better idea about which connector exactly
> changed. Therefor:
Yes, you pointed an essential problem.
> 1. add a new drm_connector->detect_epoque counter.
> 2. Increment that counter every time something has changed, i.e.
> probe helpers (we can push this down into the detect wrappers) when
> connector->status has changed, or when we update the edid blob.
> 3. probe helpers look for changes in ->detect_epoque to decided
> to fire the uevent or not.
> 4. long term we could expose the ->detect_epoque as a read-only
> property, or at least supply information about which connector
> the uevent using the new drm_sysfs_connector_status_event() (see
> patch from Ram "[PATCH v3 09/10] drm: uevent for connector status
I'll make new patches which followes your guide.
Thanks for reviewing it and giving me a new approach.
> I think that gives us a much better long term foundation than adding
> of hacks.
> Cheers, Daniel
> > Gwan-gyeong Mun (2):
> > drm: Add detection of changing of edid on between suspend and
> > resume
> > drm/i915: Add a missed update of edid property of drm connector
> > drivers/gpu/drm/drm_probe_helper.c | 34
> > +++++++++++++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_dp.c | 1 +
> > drivers/gpu/drm/i915/intel_hdmi.c | 1 +
> > 3 files changed, 35 insertions(+), 1 deletion(-)
> > --
> > 2.21.0
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx