On 2019-06-28 at 17:06:09 +0530, Lisovskiy, Stanislav wrote:
> On Fri, 2019-06-28 at 09:54 +0530, Ramalingam C wrote:
> > On 2019-06-28 at 11:24:54 +0300, Stanislav Lisovskiy wrote:
> > > Added edid checking to dp and hdmi edid setting functions, which
> > > are called from detect hooks. The result currently is propagated
> > > to calling layer using drm_connector->change_counter(proposed by
> > > Daniel Vetter).
> > > drm_helper_hpd_irq_event and intel_encoder_hotplug are currently
> > > both
> > > responsible for checking if this counter or connection status is
> > > changed.
> > >
> > > There are conflicting parts in drm and i915 which attempt
> > > to do the same job - drm_helper_hpd_irq_event attempts to
> > > check connector status changes and then call hotplug,
> > > just as i915_hotplug_work_func, which calls encoder->hotplug
> > > hook which in turn calls generic intel_encoder_hotplug function
> > > which attempts to probe if output has changed.
> > > Looks like both needs to be changed, so added edid checking
> > > also to intel_encoder_hotplug function which is called both
> > > for hdmi and dp.
> > >
> > > v2: Renamed change counter to epoch counter. Fixed type name.
> > >
> > >
>
> > > @@ -2524,11 +2524,22 @@ intel_hdmi_set_edid(struct drm_connector
> > > *connector)
> > >
> > > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
> > >
> > > + old_edid = to_intel_connector(connector)->detect_edid;
> > > +
> > > + if (!drm_edid_are_equal(edid, old_edid)) {
> > > + intel_connector_update_modes(connector, edid);
> > > + DRM_DEBUG_KMS("Updating change counter to %llu\n",
> > > connector->epoch_counter);
> > > + connector->epoch_counter += 1;
> > > +
> > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed!
> > > Updating blob property.\n",
> > > + connector->base.id, connector->name);
> > > + }
> > > + intel_hdmi_unset_edid(connector);
> > > to_intel_connector(connector)->detect_edid = edid;
> > > +
>
> >
> > This and next changes are unrelated with this commit. Might want to
> > keep
> > it for separate patch.
>
> What do you mean by unrelated? I thought dependent changes should go in
> one patch series and here we are taking into use epoch_counter for
> i915, which was introduced in previous 2 separate drm patches from that
> series.
>
> The drm changes should obviously always go first here, otherwise this
> patch will fail - I wouldn't even be able to send it for testing if
> that would be in another series.
Meant the new line addition and line removal. just usual suggestions.
you can ignore if you prefer. apart from that changes looks good.
-Ram
>
> >
> > -Ram
> > > if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
> > > intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> > > intel_hdmi->has_hdmi_sink =
> > > drm_detect_hdmi_monitor(edid);
> > > -
> > > connected = true;
> > > }
> > >
> > > @@ -2555,7 +2566,6 @@ intel_hdmi_detect(struct drm_connector
> > > *connector, bool force)
> > > !intel_digital_port_connected(encoder))
> > > goto out;
> > >
> > > - intel_hdmi_unset_edid(connector);
> > >
> > > if (intel_hdmi_set_edid(connector))
> > > status = connector_status_connected;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > index ea3de4acc850..f9d9e963196a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > @@ -271,23 +271,33 @@ bool intel_encoder_hotplug(struct
> > > intel_encoder *encoder,
> > > {
> > > struct drm_device *dev = connector->base.dev;
> > > enum drm_connector_status old_status;
> > > + u64 old_epoch_counter;
> > > + bool ret = false;
> > >
> > > WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
> > > old_status = connector->base.status;
> > >
> > > + old_epoch_counter = connector->base.epoch_counter;
> > > +
> > > connector->base.status =
> > > drm_helper_probe_detect(&connector->base, NULL, false);
> > >
> > > - if (old_status == connector->base.status)
> > > - return false;
> > > + if (old_epoch_counter != connector->base.epoch_counter)
> > > + ret = true;
> > >
> > > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
> > > %s\n",
> > > + if (old_status != connector->base.status)
> > > + ret = true;
> > > +
> > > + if (ret) {
> > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s
> > > to %s(change counter %llu)\n",
> > > connector->base.base.id,
> > > connector->base.name,
> > > drm_get_connector_status_name(old_status),
> > > - drm_get_connector_status_name(connector-
> > > >base.status));
> > > + drm_get_connector_status_name(connector-
> > > >base.status),
> > > + connector->base.epoch_counter);
> > > + }
> > >
> > > - return true;
> > > + return ret;
> > > }
> > >
> > > static bool intel_encoder_has_hpd_pulse(struct intel_encoder
> > > *encoder)
> > > --
> > > 2.17.1
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
