Re: [PATCH v3 0/3] Send a hotplug when edid changes

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Mon, 2019-08-19 at 09:05 +0000, Lisovskiy, Stanislav wrote:
> On Mon, 2019-08-19 at 08:35 +0100, Peres, Martin wrote:
> > On 19/08/2019 10:23, Lisovskiy, Stanislav wrote:
> > > On Wed, 2019-08-07 at 23:07 +0200, Daniel Vetter wrote:
> > > > 
> > > > 
> > > > So igt isn't valid userspace (it's just good testcases). Can we
> > > > repro
> > > > the
> > > > same on real userspace? Does this work with real userspace?
> > > > We've
> > > > had
> > > > userspace which tries to be clever and filters out what looks
> > > > like
> > > > redundant hotplug events. And then gets it wrong in cases like
> > > > this.
> > > > 
> > > > Also, we've had forever an unconditional uevent on resume,
> > > > exactly
> > > > because
> > > > anything could have changed. Did we loose this one on the way
> > > > somewhere?
> > > > Or maybe I misremember ...
> > > > 
> > > > If all we care about is resume re-adding that uncondtional
> > > > uevent
> > > > on
> > > > resume is going to be a lot easier than this here.
> > > > -Daniel
> > > 
> > > Sorry for long reply(was on vacation), that is a good question
> > > regarding reproducing this in real life scenario. My obvious
> > > guess
> > > was to suspend the machine and meanwhile change connected display
> > > to
> > > another one. However this situation seems to be already handled
> > > by
> > > kernel nicely(tried few times and we always get a hotplug event).
> > > So
> > > that edid change during suspend chamelium test case seems to be
> > > a bit different. I will talk to our guys who wrote this about
> > > what
> > > is
> > > the real life scenario for this, because I'm now curious as well.
> > 
> > Thanks Daniel for the feedback.
> > 
> > I also now wonder why our IGT test (chamelium-based) does not pass
> > if
> > a
> > uevent is sent on resume automatically and all the test is
> > expecting
> > is
> > a uevent...
> > 
> > Martin
> 
> In fact I was wrong - when it worked, it was using exactly those
> patches :). With clean drm-tip - it seems to work ocassionally and it
> doesn't update the actual display edid and other stuff, so even when
> displays are changed we still see the old info/edid from userspace.
> 
> We always get a hpd irq when suspend/resume however it doesn't always
> result in uevent being sent. So there is a real need in those
> patches.
> 

Just decided to "ping" this discussion again. The issue is already some
years old and still nothing is fixed. I do agree that may be something
needs to be fixed/changed here in those patches, but something must be
agreed at least I guess, as discussions themself do not fix bugs.
Currently those patches address a particular issue which occurs, if
display is changed during suspend. 
On ocassional basis, userspace might not get a hotplug event at all,
causing different kind of problems(like wrong mode set on display or
diaply not working at all). Also some kms_chamelium hotplug tests fail
because of that. 

> > 
> > > 
> > > - Stanislav
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > -Stanislav
> > > > > 
> > > > > > 
> > > > > > Cheers, Daniel
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > -Stanislav
> > > > > > > 
> > > > > > > 
> > > > > > > > -Daniel
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Stanislav Lisovskiy (3):
> > > > > > > > >   drm: Add helper to compare edids.
> > > > > > > > >   drm: Introduce change counter to drm_connector
> > > > > > > > >   drm/i915: Send hotplug event if edid had changed.
> > > > > > > > > 
> > > > > > > > >  drivers/gpu/drm/drm_connector.c              |  1 +
> > > > > > > > >  drivers/gpu/drm/drm_edid.c                   | 33
> > > > > > > > > ++++++++++++++++++++
> > > > > > > > >  drivers/gpu/drm/drm_probe_helper.c           | 29
> > > > > > > > > +++++++++++++++-
> > > > > > > > > -
> > > > > > > > >  drivers/gpu/drm/i915/display/intel_dp.c      | 16
> > > > > > > > > +++++++++-
> > > > > > > > >  drivers/gpu/drm/i915/display/intel_hdmi.c    | 16
> > > > > > > > > ++++++++--
> > > > > > > > >  drivers/gpu/drm/i915/display/intel_hotplug.c | 21
> > > > > > > > > ++++++++++
> > > > > > > > > ---
> > > > > > > > >  include/drm/drm_connector.h                  |  3 ++
> > > > > > > > >  include/drm/drm_edid.h                       |  9
> > > > > > > > > ++++++
> > > > > > > > >  8 files changed, 117 insertions(+), 11 deletions(-)
> > > > > > > > > 
> > > > > > > > > --
> > > > > > > > > 2.17.1
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > 
> > > > 
> > 
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel