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

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

On Wed, 2019-09-04 at 11:23 +0200, Daniel Vetter wrote:
> 
> > Sure this will work, but still we need somehow to be able to
> > determine
> > this "if it's different" state. In your solution we just move that
> > comparison to drm_connector_update_edid_property, which is quite
> > fine
> > for me.
> 
> Yes we need to compare edid somewhere, that much is clear. I'm not
> disputing that. I just want something where we don't have to roll
> this out
> over all the drivers, because that's a hopeless endeavour.
> 
> > I would say that yes, this idea may be is even better because 
> > drivers won't need to implement this comparison in encoder->hotplug 
> > in
> > each driver.
> > However: 
> > we still need a comparison in
> > drm_connector_update_edid_property(drm_edid_equal) and also I'm not
> > sure we can send a hotplug event based on that as
> > drm_connector_update_edid_property seems
> > to get called only during connector init or during reprobe from
> > userspace from connector->get_modes hook.
> > Also it is called from drm_kms_helper_hotplug_event from, but this
> > one
> > is called from i915 only if connection status had changed.
> 
> So ditch the optimization to only call ->get_modes when called from
> userspace? We've been talking about this one too in the past ...
> 
> I'd really like a solution where it will work for most drivers out of
> the
> box.

So I guess the conclusion would be to try to use
drm_connector_update_edid_property that way we will avoid duplicating
drm_edid_equal code in all drivers. However this might
require ensuring that drm_connector_update_edid_property is always
called when we get a hotplug, so there we can check if edid had changed
and send uevent, if needed.

- Stanislav


> -Daniel
> 
> > 
> > - Stanislav
> > 
> > 
> > > -Daniel
> > > 
> > > > 
> > > > Cheers,
> > > > 
> > > > Stanislav
> > > > 
> > > > 
> > > > > -Daniel
> > > > > 
> > > > > > 
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > - 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_hotpl
> > > > > > > > > > > > > > > ug.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
> > > > > 
> > > > > 
> > > 
> > > 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx