| Message ID | 1436784555-26155-1-git-send-email-sonika.jindal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > During init_connector set the edid, then edid will be set/unset only during > hotplug. For the sake of older platforms where HPD is not stable, let edid > read happen from detect as well only if it is forced to do so. > > v2: Removing the 'force' check, instead let detect call read the edid for > platforms older than gen 7 (Shashank) That's enough worse. We now have a random gen check with no rationale for why you suddenly believe all manufacturers have fixed their wiring. There is no reason to believe that gen7 suddenly works is there? If there is, why don't you mention it? -Chris
On 7/13/2015 5:10 PM, Chris Wilson wrote: > On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: >> During init_connector set the edid, then edid will be set/unset only during >> hotplug. For the sake of older platforms where HPD is not stable, let edid >> read happen from detect as well only if it is forced to do so. >> >> v2: Removing the 'force' check, instead let detect call read the edid for >> platforms older than gen 7 (Shashank) > > That's enough worse. We now have a random gen check with no rationale > for why you suddenly believe all manufacturers have fixed their wiring. > There is no reason to believe that gen7 suddenly works is there? If > there is, why don't you mention it? > -Chris > This gen7 check is to be on the safer side not to affect older paltforms. For CHV/VLV, already the live status check is stable enough to determine if we can read edid or not. In VPG production branches we have this patch already available and it was tested with variety of monitors extensively. So we now read the edid only during init and during hotplug. For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" patch makes HPD stable enough. So, we can rely on the edid read from init_connector instead of reading edid on every detect call. Regards, Sonika
On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > >>During init_connector set the edid, then edid will be set/unset only during > >>hotplug. For the sake of older platforms where HPD is not stable, let edid > >>read happen from detect as well only if it is forced to do so. > >> > >>v2: Removing the 'force' check, instead let detect call read the edid for > >>platforms older than gen 7 (Shashank) > > > >That's enough worse. We now have a random gen check with no rationale > >for why you suddenly believe all manufacturers have fixed their wiring. > >There is no reason to believe that gen7 suddenly works is there? If > >there is, why don't you mention it? > >-Chris > > > This gen7 check is to be on the safer side not to affect older paltforms. > For CHV/VLV, already the live status check is stable enough to determine if > we can read edid or not. In VPG production branches we have this patch > already available and it was tested with variety of monitors extensively. So > we now read the edid only during init and during hotplug. > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > patch makes HPD stable enough. > So, we can rely on the edid read from init_connector instead of reading edid > on every detect call. Again, not going to take this with random gen checks. I want your fix for handling hpd on other platforms, then roll this out everywhere. -Daniel
Hi Daniel, Chris Gen7 and Gen8 platforms have a different live status register (0x61114) and we need not to rely on ISR on that. As Sonika mentioned, We have been using this register for our commercial releases, and found them reliable across a wide range of monitors. On the other hand, Bsepc clearly mentions, to check the live status before even try to read EDID. The current DRM nightly code doesn't do that, and we have received few errors from Gen7 Chromebooks where you can still read valid EDID on HDMI hot-unplug. So I think this patch and solution is ready, and it should go in. Regards Shashank -----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Monday, July 13, 2015 8:27 PM To: Jindal, Sonika Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > >>During init_connector set the edid, then edid will be set/unset only > >>during hotplug. For the sake of older platforms where HPD is not > >>stable, let edid read happen from detect as well only if it is forced to do so. > >> > >>v2: Removing the 'force' check, instead let detect call read the > >>edid for platforms older than gen 7 (Shashank) > > > >That's enough worse. We now have a random gen check with no rationale > >for why you suddenly believe all manufacturers have fixed their wiring. > >There is no reason to believe that gen7 suddenly works is there? If > >there is, why don't you mention it? > >-Chris > > > This gen7 check is to be on the safer side not to affect older paltforms. > For CHV/VLV, already the live status check is stable enough to > determine if we can read edid or not. In VPG production branches we > have this patch already available and it was tested with variety of > monitors extensively. So we now read the edid only during init and during hotplug. > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > patch makes HPD stable enough. > So, we can rely on the edid read from init_connector instead of > reading edid on every detect call. Again, not going to take this with random gen checks. I want your fix for handling hpd on other platforms, then roll this out everywhere. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Tue, Jul 14, 2015 at 03:48:36AM +0000, Sharma, Shashank wrote: > Hi Daniel, Chris > > Gen7 and Gen8 platforms have a different live status register (0x61114) and we need not to rely on ISR on that. > As Sonika mentioned, We have been using this register for our commercial releases, and found them reliable across a wide range of monitors. > > On the other hand, Bsepc clearly mentions, to check the live status before even try to read EDID. > The current DRM nightly code doesn't do that, and we have received few errors from Gen7 Chromebooks where you can still read valid EDID on HDMI hot-unplug. > > So I think this patch and solution is ready, and it should go in. I have a gen7 machine here which is currently (and it's somewhat random) broken wrt hpd and hdmi. And afaics this patch series doesn't have the bugfix for the hpd handling - or did I miss it? -Daniel > > Regards > Shashank > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Monday, July 13, 2015 8:27 PM > To: Jindal, Sonika > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect > > On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > > >>During init_connector set the edid, then edid will be set/unset only > > >>during hotplug. For the sake of older platforms where HPD is not > > >>stable, let edid read happen from detect as well only if it is forced to do so. > > >> > > >>v2: Removing the 'force' check, instead let detect call read the > > >>edid for platforms older than gen 7 (Shashank) > > > > > >That's enough worse. We now have a random gen check with no rationale > > >for why you suddenly believe all manufacturers have fixed their wiring. > > >There is no reason to believe that gen7 suddenly works is there? If > > >there is, why don't you mention it? > > >-Chris > > > > > This gen7 check is to be on the safer side not to affect older paltforms. > > For CHV/VLV, already the live status check is stable enough to > > determine if we can read edid or not. In VPG production branches we > > have this patch already available and it was tested with variety of > > monitors extensively. So we now read the edid only during init and during hotplug. > > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > > patch makes HPD stable enough. > > So, we can rely on the edid read from init_connector instead of > > reading edid on every detect call. > > Again, not going to take this with random gen checks. I want your fix for handling hpd on other platforms, then roll this out everywhere. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Please apply this patch series along with the Interrupt handling fix patch Sonika shared. With these 4 patches applied, we should not see any problems with HPDs (Until the HW is broken). On a similar note, the corresponding chrome team applied the live status check patch, along with others, and they are not seeing the problems with HPD, hence they are also interested in this patch series. Please let us know if you observe something else, we would love to dig further. But as we previously mentioned, this patch series is available and running across various MCG Gen7 devices, and available with Gen8 PV production branches also, and there the hotplug stability is pretty good. Regards Shashank -----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, July 14, 2015 1:29 PM To: Sharma, Shashank Cc: Daniel Vetter; Jindal, Sonika; Chris Wilson; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect On Tue, Jul 14, 2015 at 03:48:36AM +0000, Sharma, Shashank wrote: > Hi Daniel, Chris > > Gen7 and Gen8 platforms have a different live status register (0x61114) and we need not to rely on ISR on that. > As Sonika mentioned, We have been using this register for our commercial releases, and found them reliable across a wide range of monitors. > > On the other hand, Bsepc clearly mentions, to check the live status before even try to read EDID. > The current DRM nightly code doesn't do that, and we have received few errors from Gen7 Chromebooks where you can still read valid EDID on HDMI hot-unplug. > > So I think this patch and solution is ready, and it should go in. I have a gen7 machine here which is currently (and it's somewhat random) broken wrt hpd and hdmi. And afaics this patch series doesn't have the bugfix for the hpd handling - or did I miss it? -Daniel > > Regards > Shashank > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Monday, July 13, 2015 8:27 PM > To: Jindal, Sonika > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not > from detect > > On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > > >>During init_connector set the edid, then edid will be set/unset > > >>only during hotplug. For the sake of older platforms where HPD is > > >>not stable, let edid read happen from detect as well only if it is forced to do so. > > >> > > >>v2: Removing the 'force' check, instead let detect call read the > > >>edid for platforms older than gen 7 (Shashank) > > > > > >That's enough worse. We now have a random gen check with no > > >rationale for why you suddenly believe all manufacturers have fixed their wiring. > > >There is no reason to believe that gen7 suddenly works is there? If > > >there is, why don't you mention it? > > >-Chris > > > > > This gen7 check is to be on the safer side not to affect older paltforms. > > For CHV/VLV, already the live status check is stable enough to > > determine if we can read edid or not. In VPG production branches we > > have this patch already available and it was tested with variety of > > monitors extensively. So we now read the edid only during init and during hotplug. > > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > > patch makes HPD stable enough. > > So, we can rely on the edid read from init_connector instead of > > reading edid on every detect call. > > Again, not going to take this with random gen checks. I want your fix for handling hpd on other platforms, then roll this out everywhere. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Tue, Jul 14, 2015 at 08:09:21AM +0000, Sharma, Shashank wrote: > Please apply this patch series along with the Interrupt handling fix patch Sonika shared. > With these 4 patches applied, we should not see any problems with HPDs (Until the HW is broken). Ok, that explains things. If you have depencies please include them all in your patch series and don't put it all over the mailing lists. I won't be able to keep track of everything. I was simply confused about why it should suddenly work without that fix. Can you please resend the entire series with all ingredients required? And enabled on all platforms for maximum testing? Thanks, Daniel > On a similar note, the corresponding chrome team applied the live status check patch, along with others, and they > are not seeing the problems with HPD, hence they are also interested in this patch series. > > Please let us know if you observe something else, we would love to dig further. > But as we previously mentioned, this patch series is available and running across various MCG Gen7 devices, and available with > Gen8 PV production branches also, and there the hotplug stability is pretty good. > > Regards > Shashank > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Tuesday, July 14, 2015 1:29 PM > To: Sharma, Shashank > Cc: Daniel Vetter; Jindal, Sonika; Chris Wilson; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect > > On Tue, Jul 14, 2015 at 03:48:36AM +0000, Sharma, Shashank wrote: > > Hi Daniel, Chris > > > > Gen7 and Gen8 platforms have a different live status register (0x61114) and we need not to rely on ISR on that. > > As Sonika mentioned, We have been using this register for our commercial releases, and found them reliable across a wide range of monitors. > > > > On the other hand, Bsepc clearly mentions, to check the live status before even try to read EDID. > > The current DRM nightly code doesn't do that, and we have received few errors from Gen7 Chromebooks where you can still read valid EDID on HDMI hot-unplug. > > > > So I think this patch and solution is ready, and it should go in. > > I have a gen7 machine here which is currently (and it's somewhat random) broken wrt hpd and hdmi. And afaics this patch series doesn't have the bugfix for the hpd handling - or did I miss it? > -Daniel > > > > > Regards > > Shashank > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of > > Daniel Vetter > > Sent: Monday, July 13, 2015 8:27 PM > > To: Jindal, Sonika > > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not > > from detect > > > > On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > > > > > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > > > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > > > >>During init_connector set the edid, then edid will be set/unset > > > >>only during hotplug. For the sake of older platforms where HPD is > > > >>not stable, let edid read happen from detect as well only if it is forced to do so. > > > >> > > > >>v2: Removing the 'force' check, instead let detect call read the > > > >>edid for platforms older than gen 7 (Shashank) > > > > > > > >That's enough worse. We now have a random gen check with no > > > >rationale for why you suddenly believe all manufacturers have fixed their wiring. > > > >There is no reason to believe that gen7 suddenly works is there? If > > > >there is, why don't you mention it? > > > >-Chris > > > > > > > This gen7 check is to be on the safer side not to affect older paltforms. > > > For CHV/VLV, already the live status check is stable enough to > > > determine if we can read edid or not. In VPG production branches we > > > have this patch already available and it was tested with variety of > > > monitors extensively. So we now read the edid only during init and during hotplug. > > > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > > > patch makes HPD stable enough. > > > So, we can rely on the edid read from init_connector instead of > > > reading edid on every detect call. > > > > Again, not going to take this with random gen checks. I want your fix for handling hpd on other platforms, then roll this out everywhere. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Sure, Daniel, Thanks. Sonika will send the patches in some time. Regards Shashank -----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, July 14, 2015 2:33 PM To: Sharma, Shashank Cc: Daniel Vetter; Jindal, Sonika; Chris Wilson; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect On Tue, Jul 14, 2015 at 08:09:21AM +0000, Sharma, Shashank wrote: > Please apply this patch series along with the Interrupt handling fix patch Sonika shared. > With these 4 patches applied, we should not see any problems with HPDs (Until the HW is broken). Ok, that explains things. If you have depencies please include them all in your patch series and don't put it all over the mailing lists. I won't be able to keep track of everything. I was simply confused about why it should suddenly work without that fix. Can you please resend the entire series with all ingredients required? And enabled on all platforms for maximum testing? Thanks, Daniel > On a similar note, the corresponding chrome team applied the live > status check patch, along with others, and they are not seeing the problems with HPD, hence they are also interested in this patch series. > > Please let us know if you observe something else, we would love to dig further. > But as we previously mentioned, this patch series is available and > running across various MCG Gen7 devices, and available with > Gen8 PV production branches also, and there the hotplug stability is pretty good. > > Regards > Shashank > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Tuesday, July 14, 2015 1:29 PM > To: Sharma, Shashank > Cc: Daniel Vetter; Jindal, Sonika; Chris Wilson; > intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not > from detect > > On Tue, Jul 14, 2015 at 03:48:36AM +0000, Sharma, Shashank wrote: > > Hi Daniel, Chris > > > > Gen7 and Gen8 platforms have a different live status register (0x61114) and we need not to rely on ISR on that. > > As Sonika mentioned, We have been using this register for our commercial releases, and found them reliable across a wide range of monitors. > > > > On the other hand, Bsepc clearly mentions, to check the live status before even try to read EDID. > > The current DRM nightly code doesn't do that, and we have received few errors from Gen7 Chromebooks where you can still read valid EDID on HDMI hot-unplug. > > > > So I think this patch and solution is ready, and it should go in. > > I have a gen7 machine here which is currently (and it's somewhat random) broken wrt hpd and hdmi. And afaics this patch series doesn't have the bugfix for the hpd handling - or did I miss it? > -Daniel > > > > > Regards > > Shashank > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of > > Daniel Vetter > > Sent: Monday, July 13, 2015 8:27 PM > > To: Jindal, Sonika > > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and > > not from detect > > > > On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > > > > > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > > > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > > > >>During init_connector set the edid, then edid will be set/unset > > > >>only during hotplug. For the sake of older platforms where HPD > > > >>is not stable, let edid read happen from detect as well only if it is forced to do so. > > > >> > > > >>v2: Removing the 'force' check, instead let detect call read the > > > >>edid for platforms older than gen 7 (Shashank) > > > > > > > >That's enough worse. We now have a random gen check with no > > > >rationale for why you suddenly believe all manufacturers have fixed their wiring. > > > >There is no reason to believe that gen7 suddenly works is there? > > > >If there is, why don't you mention it? > > > >-Chris > > > > > > > This gen7 check is to be on the safer side not to affect older paltforms. > > > For CHV/VLV, already the live status check is stable enough to > > > determine if we can read edid or not. In VPG production branches > > > we have this patch already available and it was tested with > > > variety of monitors extensively. So we now read the edid only during init and during hotplug. > > > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > > > patch makes HPD stable enough. > > > So, we can rely on the edid read from init_connector instead of > > > reading edid on every detect call. > > > > Again, not going to take this with random gen checks. I want your fix for handling hpd on other platforms, then roll this out everywhere. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation http://blog.ffwll.ch > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 66af388..44e7160 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1399,6 +1399,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) enum drm_connector_status status; struct intel_connector *intel_connector = to_intel_connector(connector); + struct drm_device *dev = connector->dev; DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); @@ -1412,7 +1413,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) * based on availability of cached EDID. This will avoid many of * these race conditions and timing problems. */ - if (force) + if (INTEL_INFO(dev)->gen < 7) intel_hdmi_probe(intel_connector->encoder); if (intel_connector->detect_edid) { @@ -2070,6 +2071,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, drm_connector_register(connector); intel_hdmi->attached_connector = intel_connector; + /* Set edid during init */ + intel_hdmi_probe(intel_encoder); + /* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written * 0xd. Failure to do so will result in spurious interrupts being * generated on the port when a cable is not attached.
During init_connector set the edid, then edid will be set/unset only during hotplug. For the sake of older platforms where HPD is not stable, let edid read happen from detect as well only if it is forced to do so. v2: Removing the 'force' check, instead let detect call read the edid for platforms older than gen 7 (Shashank) Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> --- drivers/gpu/drm/i915/intel_hdmi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)