Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v4)
On Fri, 4 Jan 2019 12:02:34 +0800 Chris Chiu wrote: > On Thu, Jan 3, 2019 at 1:50 AM Guang Bai wrote: > > > > On Wed, 2 Jan 2019 17:29:46 +0800 > > Chris Chiu wrote: > > > > > Happy New Year. > > > Sorry for bothering you guys again, I don't really want to make > > > myself a nuisance. > > > Is there any better idea for fixing this issue? > > I've already back ported this change into the kernel 4.18.17 and > > sent it to our customer for integration test - So far so good. > > Thanks, > > -Guang > > Thanks, Guang. > Can I expect to see it in next kernel release? Or we need to wait > until more positive results coming? - I'm not sure if and when my changes will pass the review and merged into the upstream kernel. I'm stuck on re-testing my fix with all those claimed failures from HDMI port live status checking happened sometimes before. I don't have any background info on those failures simply because I stepped into DRM/i915 area in recent one or two years after working on other OS/platform's gfx driver for more than two decades - My customer can't wait for this long, endless review time-period and just go ahead on proactive integration testing. They'll update us the results in a couple of days Thanks, -Guang > > > > > > > Chris > > > > > > On Mon, Dec 3, 2018 at 6:38 PM Chris Chiu > > > wrote: > > > > > > > > On Fri, Nov 30, 2018 at 1:15 AM Guang Bai > > > > wrote: > > > > > > > > > > On Thu, 29 Nov 2018 10:17:49 +0200 > > > > > Jani Nikula wrote: > > > > > > > > > > > On Wed, 28 Nov 2018, Guang Bai > > > > > > wrote: > > > > > > > On some GEN9 platforms, slowly unplugging (wiggling) the > > > > > > > HDMI cable makes the kernel to believe the HDMI display > > > > > > > is still connected. This is because the HDMI DDC lines are > > > > > > > disconnected a little bit later after the hot-plug > > > > > > > interrupt triggered thus an immediate edid fetch can be > > > > > > > made. This problem has been identified by more than one > > > > > > > customer recently. Use digital port live states to > > > > > > > authorize the edid read at HDMI detection point will > > > > > > > ensure most of the display related software states > > > > > > > updated and rest of them will be renewed accordingly when > > > > > > > the port is connected. > > > > > > > > > > > > > > v2: Fix the formatting issue > > > > > > > v3: Use digital port states to authorize the edid read > > > > > > > v4: Add comments on issue histories and rationale of the > > > > > > > fix (Chris W) > > > > > > > > > > > > You're not answering Chris Wilson's question. > > > > > > > > > > > > Why do you think the problems we've historically had with > > > > > > live status are no longer a problem? We've tried and > > > > > > reverted live status checks at least twice before because > > > > > > of regressions. Why do you think this time there won't be > > > > > > regressions? Why do you think this patch makes forward > > > > > > progress? > > > > > Jani, > > > > > I'm still new to kernel developments compared with all of you > > > > > working in this area for many years - Haven't got any > > > > > feedbacks on how exactly the HDMI live statue *not* fit for > > > > > HDMI hot-plug related port status checking, neither had time > > > > > to track all upstream bugzilla, plus not working directly > > > > > with Intel OTC teams > > > > > - What are those failing cases/regressions you mentioned > > > > > above? > > > > > - what were the kernel versions related with those > > > > > developments? > > > > > - Given the fact i915 architecture and implementation are > > > > > constantly evolving - Should we re-visit those issues with > > > > > current kernel implementation? > > > > > - Fundamentally, do you think the edid fetch is still *valid* > > > > > when the HDMI is unplugged (status either from PCH or DE)? Or > > > > > other platform configurations may present more complexities > > > > > such
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v4)
On Wed, 2 Jan 2019 17:29:46 +0800 Chris Chiu wrote: > Happy New Year. > Sorry for bothering you guys again, I don't really want to make myself > a nuisance. > Is there any better idea for fixing this issue? I've already back ported this change into the kernel 4.18.17 and sent it to our customer for integration test - So far so good. Thanks, -Guang > > Chris > > On Mon, Dec 3, 2018 at 6:38 PM Chris Chiu wrote: > > > > On Fri, Nov 30, 2018 at 1:15 AM Guang Bai > > wrote: > > > > > > On Thu, 29 Nov 2018 10:17:49 +0200 > > > Jani Nikula wrote: > > > > > > > On Wed, 28 Nov 2018, Guang Bai wrote: > > > > > On some GEN9 platforms, slowly unplugging (wiggling) the HDMI > > > > > cable makes the kernel to believe the HDMI display is still > > > > > connected. This is because the HDMI DDC lines are > > > > > disconnected a little bit later after the hot-plug interrupt > > > > > triggered thus an immediate edid fetch can be made. This > > > > > problem has been identified by more than one customer > > > > > recently. Use digital port live states to authorize the edid > > > > > read at HDMI detection point will ensure most of the display > > > > > related software states updated and rest of them will be > > > > > renewed accordingly when the port is connected. > > > > > > > > > > v2: Fix the formatting issue > > > > > v3: Use digital port states to authorize the edid read > > > > > v4: Add comments on issue histories and rationale of the fix > > > > > (Chris W) > > > > > > > > You're not answering Chris Wilson's question. > > > > > > > > Why do you think the problems we've historically had with live > > > > status are no longer a problem? We've tried and reverted live > > > > status checks at least twice before because of regressions. Why > > > > do you think this time there won't be regressions? Why do you > > > > think this patch makes forward progress? > > > Jani, > > > I'm still new to kernel developments compared with all of you > > > working in this area for many years - Haven't got any feedbacks > > > on how exactly the HDMI live statue *not* fit for HDMI hot-plug > > > related port status checking, neither had time to track all > > > upstream bugzilla, plus not working directly with Intel OTC teams > > > - What are those failing cases/regressions you mentioned above? > > > - what were the kernel versions related with those developments? > > > - Given the fact i915 architecture and implementation are > > > constantly evolving - Should we re-visit those issues with > > > current kernel implementation? > > > - Fundamentally, do you think the edid fetch is still *valid* > > > when the HDMI is unplugged (status either from PCH or DE)? Or > > > other platform configurations may present more complexities such > > > as kvm switches are used along with HDMI? > > > Again, if you could provide me more historical issue details, I'd > > > like to have some reviews/re-investigation for those cases with > > > current 4.20 kernel. > > > Thanks, > > > -Guang > > > > Hi Jani, > > I don't know the history and what kind of painful regression > > that you had run into. Could you maybe provide a test plan or some > > test cases for the regression verification? I can follow steps to > > try to verify whether if the patch can work on all cases. > > > > Chris > > > > > > > > > > I've *repeatedly* said from the beginning that I am very > > > > sceptical of using live status because we've been burned by it > > > > so many times before. I don't much care to repeat this anymore. > > > > > > > > > > > > BR, > > > > Jani. > > > > > > > > > > > > > > > > > > Cc: Jani Nikula > > > > > Cc: Chris Chiu > > > > > Cc: Chris Wilson > > > > > Signed-off-by: Guang Bai > > > > > --- > > > > > drivers/gpu/drm/i915/intel_hdmi.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > > > > > b/drivers/gpu/drm/i915/intel_hdmi.c index e2c6a2b..8cf7c78 > > > > > 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c > > > > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > > > > @@ -1929,7 +1929,7 @@ intel_hdmi_detect(struct drm_connector > > > > > *connector, bool force) > > > > > intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); > > > > > > > > > > - if (IS_ICELAKE(dev_priv) && > > > > > + if ((IS_ICELAKE(dev_priv) || IS_GEN9_BC(dev_priv)) && > > > > > !intel_digital_port_connected(encoder)) > > > > > goto out; > > > > > > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v4)
On Thu, 29 Nov 2018 10:17:49 +0200 Jani Nikula wrote: > On Wed, 28 Nov 2018, Guang Bai wrote: > > On some GEN9 platforms, slowly unplugging (wiggling) the HDMI cable > > makes the kernel to believe the HDMI display is still connected. > > This is because the HDMI DDC lines are disconnected a little bit > > later after the hot-plug interrupt triggered thus an immediate edid > > fetch can be made. This problem has been identified by more than > > one customer recently. Use digital port live states to authorize > > the edid read at HDMI detection point will ensure most of the > > display related software states updated and rest of them will be > > renewed accordingly when the port is connected. > > > > v2: Fix the formatting issue > > v3: Use digital port states to authorize the edid read > > v4: Add comments on issue histories and rationale of the fix (Chris > > W) > > You're not answering Chris Wilson's question. > > Why do you think the problems we've historically had with live status > are no longer a problem? We've tried and reverted live status checks > at least twice before because of regressions. Why do you think this > time there won't be regressions? Why do you think this patch makes > forward progress? Jani, I'm still new to kernel developments compared with all of you working in this area for many years - Haven't got any feedbacks on how exactly the HDMI live statue *not* fit for HDMI hot-plug related port status checking, neither had time to track all upstream bugzilla, plus not working directly with Intel OTC teams - What are those failing cases/regressions you mentioned above? - what were the kernel versions related with those developments? - Given the fact i915 architecture and implementation are constantly evolving - Should we re-visit those issues with current kernel implementation? - Fundamentally, do you think the edid fetch is still *valid* when the HDMI is unplugged (status either from PCH or DE)? Or other platform configurations may present more complexities such as kvm switches are used along with HDMI? Again, if you could provide me more historical issue details, I'd like to have some reviews/re-investigation for those cases with current 4.20 kernel. Thanks, -Guang > > I've *repeatedly* said from the beginning that I am very sceptical of > using live status because we've been burned by it so many times > before. I don't much care to repeat this anymore. > > > BR, > Jani. > > > > > > Cc: Jani Nikula > > Cc: Chris Chiu > > Cc: Chris Wilson > > Signed-off-by: Guang Bai > > --- > > drivers/gpu/drm/i915/intel_hdmi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > > b/drivers/gpu/drm/i915/intel_hdmi.c index e2c6a2b..8cf7c78 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -1929,7 +1929,7 @@ intel_hdmi_detect(struct drm_connector > > *connector, bool force) > > intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); > > > > - if (IS_ICELAKE(dev_priv) && > > + if ((IS_ICELAKE(dev_priv) || IS_GEN9_BC(dev_priv)) && > > !intel_digital_port_connected(encoder)) > > goto out; > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v4)
On some GEN9 platforms, slowly unplugging (wiggling) the HDMI cable makes the kernel to believe the HDMI display is still connected. This is because the HDMI DDC lines are disconnected a little bit later after the hot-plug interrupt triggered thus an immediate edid fetch can be made. This problem has been identified by more than one customer recently. Use digital port live states to authorize the edid read at HDMI detection point will ensure most of the display related software states updated and rest of them will be renewed accordingly when the port is connected. v2: Fix the formatting issue v3: Use digital port states to authorize the edid read v4: Add comments on issue histories and rationale of the fix (Chris W) Cc: Jani Nikula Cc: Chris Chiu Cc: Chris Wilson Signed-off-by: Guang Bai --- drivers/gpu/drm/i915/intel_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index e2c6a2b..8cf7c78 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1929,7 +1929,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); - if (IS_ICELAKE(dev_priv) && + if ((IS_ICELAKE(dev_priv) || IS_GEN9_BC(dev_priv)) && !intel_digital_port_connected(encoder)) goto out; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v2)
On Tue, 13 Nov 2018 13:07:46 +0200 Jani Nikula wrote: > On Mon, 12 Nov 2018, Guang Bai wrote: > > Actually I'm still working on it right now with > > DRM_MODE_CONNECTOR_HDMIA/HDMIB, recommended by James, I'm able to > > differentiate the HDMI or DP even the encoder type is the > > "INTEL_OUTPUT_DDI", I still have the "trybot" intermittent test > > failures with new DRM connector types. Even worse, there is phantom > > "intel_encoder_hotplug()" call following the correct one: > > When connecting both DP and HDMI on the platform, unplug the DP, the > > "i915_hotplug_work_func()" first calls the "intel_encoder_hotplug()" > > with DP encoder, then calls again with HDMI encoder. > > I haven't identified if the work function get queued twice or itself > > is incorrectly identifying wrong encoder hotplut status. Will try to > > get everything cleaned up ASAP. > > Frankly I liked the simplicity of [1] over the patch in this thread. > It fixed the real-world use case Chris Chiu has, but I understand > that it still failed the slow unplug HDMI test case you have. The > problem is, we have zero visibility to the test case you have. Is it > automated or manual? Can we see the specs or source code for the test > case? > > If we have to consider the live status unreliable, it's possible to > devise a pathological test case that will always fail, regardless of > what we do in the driver. Does the test case reflect real world usage? > > Also Cc: Ville for input. > > BR, > Jani. > Hi Jani, Ville & Chris (Chiu): Sorry for the late reply as I have beening bumping around different projects and also spent sometime to investigate [v2] failures with testing the codes on trybot 1. My testcase: I have one+ customer KBL laptop and uBuntu16.04 running, slowly unplug the HDMI cable (wiggling), the issue can be reproduced very easily - Just check kernel HDMI-A-1 connecting states 2. I belive Chris is using roughly the same test case? 3. I don't like the [v2] solution either since it's *not* clean and have intermittent trybot/patchwork failures 4. with latest drm-tip codes, I'm able to workout a very clean solution [v3] today and it passes the patchwork tests - Would you like to try it out and also review the change? Thanks, Guang > > > [1] > http://patchwork.freedesktop.org/patch/msgid/20180925071836.24711-1-jani.nik...@intel.com > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v3)
On some GEN9 platforms, slowly unplugging (wiggling) the HDMI cable makes the kernel to believe the HDMI display is still connected. This is because the HDMI DDC lines are disconnected a little bit later after the hot-plug interrupt triggered thus an immediate edid fetch can be made. Use digital port live states to authorize the edid read. v2: Fix the formatting issue v3: Use digital port states to authorize the edid read Cc: Jani Nikula Cc: Chris Chiu Signed-off-by: Guang Bai --- drivers/gpu/drm/i915/intel_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index e2c6a2b3e8f2..8cf7c78b8cdd 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1929,7 +1929,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); - if (IS_ICELAKE(dev_priv) && + if ((IS_ICELAKE(dev_priv) || IS_GEN9_BC(dev_priv)) && !intel_digital_port_connected(encoder)) goto out; -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v2)
On Tue, 13 Nov 2018 09:04:37 +0800
Chris Chiu wrote:
> Gentle ping. Just want to know if you have time for this so far.
> Thanks
>
> Chris
Actually I'm still working on it right now with
DRM_MODE_CONNECTOR_HDMIA/HDMIB, recommended by James, I'm able to
differentiate the HDMI or DP even the encoder type is the
"INTEL_OUTPUT_DDI", I still have the "trybot" intermittent test failures
with new DRM connector types. Even worse, there is phantom
"intel_encoder_hotplug()" call following the correct one:
When connecting both DP and HDMI on the platform, unplug the DP, the
"i915_hotplug_work_func()" first calls the "intel_encoder_hotplug()"
with DP encoder, then calls again with HDMI encoder.
I haven't identified if the work function get queued twice or itself
is incorrectly identifying wrong encoder hotplut status. Will try to
get everything cleaned up ASAP.
Thanks,
Guang
>
> On Tue, Oct 30, 2018 at 1:24 AM Guang Bai wrote:
>
> > On Tue, 23 Oct 2018 17:14:34 +0800
> > Chris Chiu wrote:
> >
> > > On Thu, Oct 11, 2018 at 2:04 AM Guang Bai
> > > wrote:
> > > > On Mon, 8 Oct 2018 08:56:20 -0700
> > > > Guang Bai wrote:
> > > >
> > > > > On Mon, 8 Oct 2018 22:35:34 +0800
> > > > > Chris Chiu wrote:
> > > > >
> > > > > > Thanks! I have no problem with this patch.
> > > > >
> > > > > There are Fi.CI.BAT failures with the v2 (only with
> > > > > formatting fix added) while the previous patch had passing
> > > > > results. Now trying to identify why the failures happened
> > > > > with trybot Thanks,
> > > > > Guang
> > > > The tribot run my patch twice and passes the tests without any
> > > > error however I'm recommended to chase down root causes of
> > > > Patchwork Fi.CI.BAT test errors still - WIP on that.
> > > > Thanks,
> > > > Guang
> > > >
> > >
> > > Gentle ping. Any good news on this?
> > >
> > > Chris
> > >
> > Sorry...was distracted by other dev taks...will get update ASAP.
> > -Guang
> > >
> > > > >
> > > > > >
> > > > > > On Thu, Oct 4, 2018 at 2:08 AM Guang Bai
> > > > > > wrote:
> > > > > > > On some platforms, slowly unplugging (wiggling) the HDMI
> > > > > > > cable makes the kernel to believe the HDMI display still
> > > > > > > connected. This is because the HDMI DDC lines are
> > > > > > > disconnected sometimes later after the hot-plug interrupt
> > > > > > > triggered. Use the hot plug live states to honor HDMI hot
> > > > > > > plug status in addtion to access the DDC channels.
> > > > > > >
> > > > > > > v2: Fix the formatting issue
> > > > > > >
> > > > > > > Cc: Jani Nikula
> > > > > > > Cc: Chris Chiu
> > > > > > > Signed-off-by: Guang Bai
> > > > > > > ---
> > > > > > > drivers/gpu/drm/i915/intel_hotplug.c | 32
> > > > > > > +--- 1 file changed, 29
> > > > > > > insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > > > > > > b/drivers/gpu/drm/i915/intel_hotplug.c
> > > > > > > index 648a13c..98ab1ab 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > > > > > > @@ -246,17 +246,43 @@ static void
> > > > > > > intel_hpd_irq_storm_reenable_work(struct work_struct
> > > > > > > *work) intel_runtime_pm_put(dev_priv);
> > > > > > > }
> > > > > > >
> > > > > > > +#define MAX_SHORT_PULSE_MS 100
> > > > > > > +#define PORT_CHECK_LOOP_COUNT 3
> > > > > > > +
> > > > > > > bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > > > > > >struct intel_connector
> > > > > > > *connector) {
> > > > > > > struct drm_device *dev = connector->base.dev;
> > > > > > > - enum drm_connector_status old_status;
> > > > > > > +
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v2)
On Tue, 23 Oct 2018 17:14:34 +0800
Chris Chiu wrote:
> On Thu, Oct 11, 2018 at 2:04 AM Guang Bai wrote:
>
> > On Mon, 8 Oct 2018 08:56:20 -0700
> > Guang Bai wrote:
> >
> > > On Mon, 8 Oct 2018 22:35:34 +0800
> > > Chris Chiu wrote:
> > >
> > > > Thanks! I have no problem with this patch.
> > >
> > > There are Fi.CI.BAT failures with the v2 (only with formatting fix
> > > added) while the previous patch had passing results.
> > > Now trying to identify why the failures happened with trybot
> > > Thanks,
> > > Guang
> > The tribot run my patch twice and passes the tests without any error
> > however I'm recommended to chase down root causes of Patchwork
> > Fi.CI.BAT test errors still - WIP on that.
> > Thanks,
> > Guang
> >
>
> Gentle ping. Any good news on this?
>
> Chris
>
Sorry...was distracted by other dev taks...will get update ASAP.
-Guang
>
> > >
> > > >
> > > > On Thu, Oct 4, 2018 at 2:08 AM Guang Bai
> > > > wrote:
> > > > > On some platforms, slowly unplugging (wiggling) the HDMI cable
> > > > > makes the kernel to believe the HDMI display still connected.
> > > > > This is because the HDMI DDC lines are disconnected sometimes
> > > > > later after the hot-plug interrupt triggered. Use the hot plug
> > > > > live states to honor HDMI hot plug status in addtion to access
> > > > > the DDC channels.
> > > > >
> > > > > v2: Fix the formatting issue
> > > > >
> > > > > Cc: Jani Nikula
> > > > > Cc: Chris Chiu
> > > > > Signed-off-by: Guang Bai
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_hotplug.c | 32
> > > > > +--- 1 file changed, 29
> > > > > insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > > > > b/drivers/gpu/drm/i915/intel_hotplug.c
> > > > > index 648a13c..98ab1ab 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > > > > @@ -246,17 +246,43 @@ static void
> > > > > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> > > > > intel_runtime_pm_put(dev_priv);
> > > > > }
> > > > >
> > > > > +#define MAX_SHORT_PULSE_MS 100
> > > > > +#define PORT_CHECK_LOOP_COUNT 3
> > > > > +
> > > > > bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > > > >struct intel_connector *connector)
> > > > > {
> > > > > struct drm_device *dev = connector->base.dev;
> > > > > - enum drm_connector_status old_status;
> > > > > + enum drm_connector_status old_status, new_status;
> > > > > + enum hpd_pin pin = encoder->hpd_pin;
> > > > > + struct drm_i915_private *dev_priv =
> > > > > to_i915(encoder->base.dev);
> > > > > + u32 count = 0;
> > > > >
> > > > > WARN_ON(!mutex_is_locked(>mode_config.mutex));
> > > > > old_status = connector->base.status;
> > > > >
> > > > > - connector->base.status =
> > > > > - drm_helper_probe_detect(>base,
> > > > > NULL, false);
> > > > > + /*
> > > > > +* Set HDMI connection status based on hot-plug live
> > > > > states and
> > > > > +* display probe results.
> > > > > +*/
> > > > > + if ((encoder->type == INTEL_OUTPUT_HDMI ||
> > > > > +encoder->type == INTEL_OUTPUT_DDI) &&
> > > > > + dev_priv->hotplug.stats[pin].state ==
> > > > > HPD_ENABLED) {
> > > > > + do {
> > > > > + new_status =
> > > > > connector_status_disconnected;
> > > > > + msleep(MAX_SHORT_PULSE_MS);
> > > > > +
> > > > > + if
> > > > > (intel_digital_port_connected(encoder))
> > > > > + new_status =
> > > > > drm_helper_probe_detect(>base,
> > > > > +
> > > > > NULL, false);
> > > > > + if (new_status ==
> > > > > connector_status_connected)
> > > > > + break;
> > > > > + } while (++count <= PORT_CHECK_LOOP_COUNT);
> > > > > + connector->base.status = new_status;
> > > > > + } else {
> > > > > + connector->base.status =
> > > > > +
> > > > > drm_helper_probe_detect(>base, NULL, false);
> > > > > + }
> > > > >
> > > > > if (old_status == connector->base.status)
> > > > > return false;
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > >
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v2)
On Mon, 8 Oct 2018 08:56:20 -0700
Guang Bai wrote:
> On Mon, 8 Oct 2018 22:35:34 +0800
> Chris Chiu wrote:
>
> > Thanks! I have no problem with this patch.
>
> There are Fi.CI.BAT failures with the v2 (only with formatting fix
> added) while the previous patch had passing results.
> Now trying to identify why the failures happened with trybot
> Thanks,
> Guang
The tribot run my patch twice and passes the tests without any error
however I'm recommended to chase down root causes of Patchwork Fi.CI.BAT
test errors still - WIP on that.
Thanks,
Guang
>
> >
> > On Thu, Oct 4, 2018 at 2:08 AM Guang Bai
> > wrote:
> > > On some platforms, slowly unplugging (wiggling) the HDMI cable
> > > makes the kernel to believe the HDMI display still connected.
> > > This is because the HDMI DDC lines are disconnected sometimes
> > > later after the hot-plug interrupt triggered. Use the hot plug
> > > live states to honor HDMI hot plug status in addtion to access
> > > the DDC channels.
> > >
> > > v2: Fix the formatting issue
> > >
> > > Cc: Jani Nikula
> > > Cc: Chris Chiu
> > > Signed-off-by: Guang Bai
> > > ---
> > > drivers/gpu/drm/i915/intel_hotplug.c | 32
> > > +--- 1 file changed, 29 insertions(+),
> > > 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > > b/drivers/gpu/drm/i915/intel_hotplug.c
> > > index 648a13c..98ab1ab 100644
> > > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > > @@ -246,17 +246,43 @@ static void
> > > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> > > intel_runtime_pm_put(dev_priv);
> > > }
> > >
> > > +#define MAX_SHORT_PULSE_MS 100
> > > +#define PORT_CHECK_LOOP_COUNT 3
> > > +
> > > bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > >struct intel_connector *connector)
> > > {
> > > struct drm_device *dev = connector->base.dev;
> > > - enum drm_connector_status old_status;
> > > + enum drm_connector_status old_status, new_status;
> > > + enum hpd_pin pin = encoder->hpd_pin;
> > > + struct drm_i915_private *dev_priv =
> > > to_i915(encoder->base.dev);
> > > + u32 count = 0;
> > >
> > > WARN_ON(!mutex_is_locked(>mode_config.mutex));
> > > old_status = connector->base.status;
> > >
> > > - connector->base.status =
> > > - drm_helper_probe_detect(>base, NULL,
> > > false);
> > > + /*
> > > +* Set HDMI connection status based on hot-plug live
> > > states and
> > > +* display probe results.
> > > +*/
> > > + if ((encoder->type == INTEL_OUTPUT_HDMI ||
> > > +encoder->type == INTEL_OUTPUT_DDI) &&
> > > + dev_priv->hotplug.stats[pin].state == HPD_ENABLED) {
> > > + do {
> > > + new_status =
> > > connector_status_disconnected;
> > > + msleep(MAX_SHORT_PULSE_MS);
> > > +
> > > + if (intel_digital_port_connected(encoder))
> > > + new_status =
> > > drm_helper_probe_detect(>base,
> > > +
> > > NULL, false);
> > > + if (new_status ==
> > > connector_status_connected)
> > > + break;
> > > + } while (++count <= PORT_CHECK_LOOP_COUNT);
> > > + connector->base.status = new_status;
> > > + } else {
> > > + connector->base.status =
> > > + drm_helper_probe_detect(>base,
> > > NULL, false);
> > > + }
> > >
> > > if (old_status == connector->base.status)
> > > return false;
> > > --
> > > 2.7.4
> > >
> > >
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v2)
On Mon, 8 Oct 2018 22:35:34 +0800
Chris Chiu wrote:
> Thanks! I have no problem with this patch.
There are Fi.CI.BAT failures with the v2 (only with formatting fix
added) while the previous patch had passing results.
Now trying to identify why the failures happened with trybot
Thanks,
Guang
>
> On Thu, Oct 4, 2018 at 2:08 AM Guang Bai wrote:
>
> > On some platforms, slowly unplugging (wiggling) the HDMI cable makes
> > the kernel to believe the HDMI display still connected. This is
> > because the HDMI DDC lines are disconnected sometimes later after
> > the hot-plug interrupt triggered. Use the hot plug live states to
> > honor HDMI hot plug status in addtion to access the DDC channels.
> >
> > v2: Fix the formatting issue
> >
> > Cc: Jani Nikula
> > Cc: Chris Chiu
> > Signed-off-by: Guang Bai
> > ---
> > drivers/gpu/drm/i915/intel_hotplug.c | 32
> > +--- 1 file changed, 29 insertions(+),
> > 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 648a13c..98ab1ab 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -246,17 +246,43 @@ static void
> > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> > intel_runtime_pm_put(dev_priv);
> > }
> >
> > +#define MAX_SHORT_PULSE_MS 100
> > +#define PORT_CHECK_LOOP_COUNT 3
> > +
> > bool intel_encoder_hotplug(struct intel_encoder *encoder,
> >struct intel_connector *connector)
> > {
> > struct drm_device *dev = connector->base.dev;
> > - enum drm_connector_status old_status;
> > + enum drm_connector_status old_status, new_status;
> > + enum hpd_pin pin = encoder->hpd_pin;
> > + struct drm_i915_private *dev_priv =
> > to_i915(encoder->base.dev);
> > + u32 count = 0;
> >
> > WARN_ON(!mutex_is_locked(>mode_config.mutex));
> > old_status = connector->base.status;
> >
> > - connector->base.status =
> > - drm_helper_probe_detect(>base, NULL,
> > false);
> > + /*
> > +* Set HDMI connection status based on hot-plug live states
> > and
> > +* display probe results.
> > +*/
> > + if ((encoder->type == INTEL_OUTPUT_HDMI ||
> > +encoder->type == INTEL_OUTPUT_DDI) &&
> > + dev_priv->hotplug.stats[pin].state == HPD_ENABLED) {
> > + do {
> > + new_status = connector_status_disconnected;
> > + msleep(MAX_SHORT_PULSE_MS);
> > +
> > + if (intel_digital_port_connected(encoder))
> > + new_status =
> > drm_helper_probe_detect(>base,
> > +
> > NULL, false);
> > + if (new_status ==
> > connector_status_connected)
> > + break;
> > + } while (++count <= PORT_CHECK_LOOP_COUNT);
> > + connector->base.status = new_status;
> > + } else {
> > + connector->base.status =
> > + drm_helper_probe_detect(>base,
> > NULL, false);
> > + }
> >
> > if (old_status == connector->base.status)
> > return false;
> > --
> > 2.7.4
> >
> >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v2)
On some platforms, slowly unplugging (wiggling) the HDMI cable makes
the kernel to believe the HDMI display still connected. This is because
the HDMI DDC lines are disconnected sometimes later after the hot-plug
interrupt triggered. Use the hot plug live states to honor HDMI hot plug
status in addtion to access the DDC channels.
v2: Fix the formatting issue
Cc: Jani Nikula
Cc: Chris Chiu
Signed-off-by: Guang Bai
---
drivers/gpu/drm/i915/intel_hotplug.c | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
b/drivers/gpu/drm/i915/intel_hotplug.c
index 648a13c..98ab1ab 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -246,17 +246,43 @@ static void intel_hpd_irq_storm_reenable_work(struct
work_struct *work)
intel_runtime_pm_put(dev_priv);
}
+#define MAX_SHORT_PULSE_MS 100
+#define PORT_CHECK_LOOP_COUNT 3
+
bool intel_encoder_hotplug(struct intel_encoder *encoder,
struct intel_connector *connector)
{
struct drm_device *dev = connector->base.dev;
- enum drm_connector_status old_status;
+ enum drm_connector_status old_status, new_status;
+ enum hpd_pin pin = encoder->hpd_pin;
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ u32 count = 0;
WARN_ON(!mutex_is_locked(>mode_config.mutex));
old_status = connector->base.status;
- connector->base.status =
- drm_helper_probe_detect(>base, NULL, false);
+ /*
+* Set HDMI connection status based on hot-plug live states and
+* display probe results.
+*/
+ if ((encoder->type == INTEL_OUTPUT_HDMI ||
+encoder->type == INTEL_OUTPUT_DDI) &&
+ dev_priv->hotplug.stats[pin].state == HPD_ENABLED) {
+ do {
+ new_status = connector_status_disconnected;
+ msleep(MAX_SHORT_PULSE_MS);
+
+ if (intel_digital_port_connected(encoder))
+ new_status =
drm_helper_probe_detect(>base,
+NULL,
false);
+ if (new_status == connector_status_connected)
+ break;
+ } while (++count <= PORT_CHECK_LOOP_COUNT);
+ connector->base.status = new_status;
+ } else {
+ connector->base.status =
+ drm_helper_probe_detect(>base, NULL, false);
+ }
if (old_status == connector->base.status)
return false;
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure
On some platforms, slowly unplugging (wiggling) the HDMI cable makes
the kernel to believe the HDMI display still connected. This is because
the HDMI DDC lines are disconnected sometimes later after the hot-plug
interrupt triggered. Use the hot plug live states to honor HDMI hot plug
status in addtion to access the DDC channels.
Cc: Jani Nikula
Cc: Chris Chiu
Signed-off-by: Guang Bai
---
drivers/gpu/drm/i915/intel_hotplug.c | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
b/drivers/gpu/drm/i915/intel_hotplug.c
index 648a13c..db6288f 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -246,17 +246,42 @@ static void intel_hpd_irq_storm_reenable_work(struct
work_struct *work)
intel_runtime_pm_put(dev_priv);
}
+#define MAX_SHORT_PULSE_MS 100
+#define PORT_CHECK_LOOP_COUNT 3
+
bool intel_encoder_hotplug(struct intel_encoder *encoder,
struct intel_connector *connector)
{
struct drm_device *dev = connector->base.dev;
- enum drm_connector_status old_status;
+ enum drm_connector_status old_status, new_status;
+ enum hpd_pin pin = encoder->hpd_pin;
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ u32 count = 0;
WARN_ON(!mutex_is_locked(>mode_config.mutex));
old_status = connector->base.status;
- connector->base.status =
- drm_helper_probe_detect(>base, NULL, false);
+ /*
+* Set HDMI connection status based on hot-plug live states and
+* display probe results.
+*/
+ if ((encoder->type == INTEL_OUTPUT_HDMI ||
+encoder->type == INTEL_OUTPUT_DDI) &&
+ dev_priv->hotplug.stats[pin].state == HPD_ENABLED) {
+ do {
+ new_status = connector_status_disconnected;
+ msleep(MAX_SHORT_PULSE_MS);
+
+ if (intel_digital_port_connected(encoder))
+ new_status =
drm_helper_probe_detect(>base,
+NULL,
false);
+ if (new_status == connector_status_connected)
+ break;
+ } while (++count <= PORT_CHECK_LOOP_COUNT);
+ connector->base.status = new_status;
+ } else
+ connector->base.status =
+ drm_helper_probe_detect(>base, NULL, false);
if (old_status == connector->base.status)
return false;
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: delay hotplug scheduling
On Fri, 28 Sep 2018 09:26:48 +0300
Jani Nikula wrote:
> On Thu, 27 Sep 2018, Guang Bai wrote:
> > On Thu, 27 Sep 2018 09:45:53 +0300
> > Jani Nikula wrote:
> >
> >> On Wed, 26 Sep 2018, Guang Bai wrote:
> >> > On Wed, 26 Sep 2018 16:44:16 -0700
> >> > Guang Bai wrote:
> >> >
> >> >> On Tue, 25 Sep 2018 10:18:36 +0300
> >> >> Jani Nikula wrote:
> >> >>
> >> >> > On some systems we get the hotplug irq on unplug before the
> >> >> > DDC pins have been disconnected, resulting in connector status
> >> >> > remaining connected. Since previous attempts at using hotplug
> >> >> > live status before DDC have failed, the only viable option is
> >> >> > to reduce the window for mistakes. Delay the hotplug
> >> >> > processing.
> >> >> >
> >> >> > Reference:
> >> >> > http://mid.mail-archive.com/CAB4CAwcfj32FovZwpTbekvHpoc3gG0sVbNtW8jeXRcp5qwEsmw@mail.gmail.com
> >> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107125
> >> >> > Reported-by: Chris Chiu Tested-by: Chris
> >> >> > Chiu Cc: Chris Chiu
> >> >> > Cc: Guang Bai
> >> >> > Cc: Ville Syrjälä
> >> >> > Signed-off-by: Jani Nikula
> >> >> > ---
> >> >> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> >> >> > drivers/gpu/drm/i915/intel_hotplug.c | 16 +++-
> >> >> > 2 files changed, 12 insertions(+), 6 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> >> > b/drivers/gpu/drm/i915/i915_drv.h index
> >> >> > 8624b4bdc242..4378be7a20d5 100644 ---
> >> >> > a/drivers/gpu/drm/i915/i915_drv.h +++
> >> >> > b/drivers/gpu/drm/i915/i915_drv.h @@ -286,7 +286,7 @@ enum
> >> >> > hpd_pin { #define HPD_STORM_DEFAULT_THRESHOLD 5
> >> >> >
> >> >> > struct i915_hotplug {
> >> >> > - struct work_struct hotplug_work;
> >> >> > + struct delayed_work hotplug_work;
> >> >> >
> >> >> > struct {
> >> >> > unsigned long last_jiffies;
> >> >> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> >> >> > b/drivers/gpu/drm/i915/intel_hotplug.c index
> >> >> > 648a13c6043c..ce9bedbedcd1 100644 ---
> >> >> > a/drivers/gpu/drm/i915/intel_hotplug.c +++
> >> >> > b/drivers/gpu/drm/i915/intel_hotplug.c @@ -110,6 +110,9 @@
> >> >> > enum hpd_pin intel_hpd_pin_default(struct drm_i915_private
> >> >> > *dev_priv, } }
> >> >> >
> >> >> > +/* Delay between irq and hotplug detect processing */
> >> >> > +#define HOTPLUG_DELAY_MS 300
> >> >> > +
> >> >> > #define HPD_STORM_DETECT_PERIOD 1000
> >> >> > #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000)
> >> >> >
> >> >> > @@ -319,7 +322,8 @@ static void i915_digport_work_func(struct
> >> >> > work_struct *work) spin_lock_irq(_priv->irq_lock);
> >> >> > dev_priv->hotplug.event_bits |= old_bits;
> >> >> > spin_unlock_irq(_priv->irq_lock);
> >> >> > -
> >> >> > schedule_work(_priv->hotplug.hotplug_work); +
> >> >> > schedule_delayed_work(_priv->hotplug.hotplug_work,
> >> >> > +
> >> >> > msecs_to_jiffies(HOTPLUG_DELAY_MS)); }
> >> >> > }
> >> >> >
> >> >> > @@ -329,7 +333,7 @@ static void i915_digport_work_func(struct
> >> >> > work_struct *work) static void i915_hotplug_work_func(struct
> >> >> > work_struct *work) {
> >> >> > struct drm_i915_private *dev_priv =
> >> >> > - container_of(work, struct drm_i915_private,
> >> >> > hotplug.hotplug_work);
> >> >> > + container_of(work, struct drm_i915_private,
> >> >> > hotplug.hotplug_work.work); struct drm_device *dev =
> >> >> > _priv->drm; struct intel_connector *intel_connector;
> >> >> > struct intel_encoder *intel_encoder;
> >> >> > @@ -466,7
Re: [Intel-gfx] [PATCH] drm/i915: delay hotplug scheduling
On Thu, 27 Sep 2018 09:45:53 +0300
Jani Nikula wrote:
> On Wed, 26 Sep 2018, Guang Bai wrote:
> > On Wed, 26 Sep 2018 16:44:16 -0700
> > Guang Bai wrote:
> >
> >> On Tue, 25 Sep 2018 10:18:36 +0300
> >> Jani Nikula wrote:
> >>
> >> > On some systems we get the hotplug irq on unplug before the DDC
> >> > pins have been disconnected, resulting in connector status
> >> > remaining connected. Since previous attempts at using hotplug
> >> > live status before DDC have failed, the only viable option is to
> >> > reduce the window for mistakes. Delay the hotplug processing.
> >> >
> >> > Reference:
> >> > http://mid.mail-archive.com/CAB4CAwcfj32FovZwpTbekvHpoc3gG0sVbNtW8jeXRcp5qwEsmw@mail.gmail.com
> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107125
> >> > Reported-by: Chris Chiu Tested-by: Chris Chiu
> >> > Cc: Chris Chiu
> >> > Cc: Guang Bai
> >> > Cc: Ville Syrjälä
> >> > Signed-off-by: Jani Nikula
> >> > ---
> >> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> >> > drivers/gpu/drm/i915/intel_hotplug.c | 16 +++-
> >> > 2 files changed, 12 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> > b/drivers/gpu/drm/i915/i915_drv.h index
> >> > 8624b4bdc242..4378be7a20d5 100644 ---
> >> > a/drivers/gpu/drm/i915/i915_drv.h +++
> >> > b/drivers/gpu/drm/i915/i915_drv.h @@ -286,7 +286,7 @@ enum
> >> > hpd_pin { #define HPD_STORM_DEFAULT_THRESHOLD 5
> >> >
> >> > struct i915_hotplug {
> >> > -struct work_struct hotplug_work;
> >> > +struct delayed_work hotplug_work;
> >> >
> >> > struct {
> >> > unsigned long last_jiffies;
> >> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> >> > b/drivers/gpu/drm/i915/intel_hotplug.c index
> >> > 648a13c6043c..ce9bedbedcd1 100644 ---
> >> > a/drivers/gpu/drm/i915/intel_hotplug.c +++
> >> > b/drivers/gpu/drm/i915/intel_hotplug.c @@ -110,6 +110,9 @@ enum
> >> > hpd_pin intel_hpd_pin_default(struct drm_i915_private
> >> > *dev_priv, } }
> >> >
> >> > +/* Delay between irq and hotplug detect processing */
> >> > +#define HOTPLUG_DELAY_MS300
> >> > +
> >> > #define HPD_STORM_DETECT_PERIOD 1000
> >> > #define HPD_STORM_REENABLE_DELAY(2 * 60 * 1000)
> >> >
> >> > @@ -319,7 +322,8 @@ static void i915_digport_work_func(struct
> >> > work_struct *work) spin_lock_irq(_priv->irq_lock);
> >> > dev_priv->hotplug.event_bits |= old_bits;
> >> > spin_unlock_irq(_priv->irq_lock);
> >> > -schedule_work(_priv->hotplug.hotplug_work);
> >> > +
> >> > schedule_delayed_work(_priv->hotplug.hotplug_work,
> >> > +
> >> > msecs_to_jiffies(HOTPLUG_DELAY_MS)); }
> >> > }
> >> >
> >> > @@ -329,7 +333,7 @@ static void i915_digport_work_func(struct
> >> > work_struct *work) static void i915_hotplug_work_func(struct
> >> > work_struct *work) {
> >> > struct drm_i915_private *dev_priv =
> >> > -container_of(work, struct drm_i915_private,
> >> > hotplug.hotplug_work);
> >> > +container_of(work, struct drm_i915_private,
> >> > hotplug.hotplug_work.work); struct drm_device *dev =
> >> > _priv->drm; struct intel_connector *intel_connector;
> >> > struct intel_encoder *intel_encoder;
> >> > @@ -466,7 +470,8 @@ void intel_hpd_irq_handler(struct
> >> > drm_i915_private *dev_priv, if (queue_dig)
> >> > queue_work(dev_priv->hotplug.dp_wq,
> >> > _priv->hotplug.dig_port_work); if (queue_hp)
> >> > -schedule_work(_priv->hotplug.hotplug_work);
> >> > +
> >> > schedule_delayed_work(_priv->hotplug.hotplug_work,
> >> > +
> >> > msecs_to_jiffies(HOTPLUG_DELAY_MS)); }
> >> >
> >> > /**
> >> > @@ -586,7 +591,8 @@ void intel_hpd_poll_init(struct
> >> > drm_i915_private *dev_priv)
> >> > void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> >> > {
> >> > -
Re: [Intel-gfx] [PATCH] drm/i915: delay hotplug scheduling
On Wed, 26 Sep 2018 16:44:16 -0700
Guang Bai wrote:
> On Tue, 25 Sep 2018 10:18:36 +0300
> Jani Nikula wrote:
>
> > On some systems we get the hotplug irq on unplug before the DDC pins
> > have been disconnected, resulting in connector status remaining
> > connected. Since previous attempts at using hotplug live status
> > before DDC have failed, the only viable option is to reduce the
> > window for mistakes. Delay the hotplug processing.
> >
> > Reference:
> > http://mid.mail-archive.com/CAB4CAwcfj32FovZwpTbekvHpoc3gG0sVbNtW8jeXRcp5qwEsmw@mail.gmail.com
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107125
> > Reported-by: Chris Chiu Tested-by: Chris Chiu
> > Cc: Chris Chiu
> > Cc: Guang Bai
> > Cc: Ville Syrjälä
> > Signed-off-by: Jani Nikula
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > drivers/gpu/drm/i915/intel_hotplug.c | 16 +++-
> > 2 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index 8624b4bdc242..4378be7a20d5
> > 100644 --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -286,7 +286,7 @@ enum hpd_pin {
> > #define HPD_STORM_DEFAULT_THRESHOLD 5
> >
> > struct i915_hotplug {
> > - struct work_struct hotplug_work;
> > + struct delayed_work hotplug_work;
> >
> > struct {
> > unsigned long last_jiffies;
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c index
> > 648a13c6043c..ce9bedbedcd1 100644 ---
> > a/drivers/gpu/drm/i915/intel_hotplug.c +++
> > b/drivers/gpu/drm/i915/intel_hotplug.c @@ -110,6 +110,9 @@ enum
> > hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv, }
> > }
> >
> > +/* Delay between irq and hotplug detect processing */
> > +#define HOTPLUG_DELAY_MS 300
> > +
> > #define HPD_STORM_DETECT_PERIOD1000
> > #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000)
> >
> > @@ -319,7 +322,8 @@ static void i915_digport_work_func(struct
> > work_struct *work) spin_lock_irq(_priv->irq_lock);
> > dev_priv->hotplug.event_bits |= old_bits;
> > spin_unlock_irq(_priv->irq_lock);
> > - schedule_work(_priv->hotplug.hotplug_work);
> > +
> > schedule_delayed_work(_priv->hotplug.hotplug_work,
> > +
> > msecs_to_jiffies(HOTPLUG_DELAY_MS)); }
> > }
> >
> > @@ -329,7 +333,7 @@ static void i915_digport_work_func(struct
> > work_struct *work) static void i915_hotplug_work_func(struct
> > work_struct *work) {
> > struct drm_i915_private *dev_priv =
> > - container_of(work, struct drm_i915_private,
> > hotplug.hotplug_work);
> > + container_of(work, struct drm_i915_private,
> > hotplug.hotplug_work.work); struct drm_device *dev = _priv->drm;
> > struct intel_connector *intel_connector;
> > struct intel_encoder *intel_encoder;
> > @@ -466,7 +470,8 @@ void intel_hpd_irq_handler(struct
> > drm_i915_private *dev_priv, if (queue_dig)
> > queue_work(dev_priv->hotplug.dp_wq,
> > _priv->hotplug.dig_port_work); if (queue_hp)
> > - schedule_work(_priv->hotplug.hotplug_work);
> > +
> > schedule_delayed_work(_priv->hotplug.hotplug_work,
> > +
> > msecs_to_jiffies(HOTPLUG_DELAY_MS)); }
> >
> > /**
> > @@ -586,7 +591,8 @@ void intel_hpd_poll_init(struct drm_i915_private
> > *dev_priv)
> > void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> > {
> > - INIT_WORK(_priv->hotplug.hotplug_work,
> > i915_hotplug_work_func);
> > + INIT_DELAYED_WORK(_priv->hotplug.hotplug_work,
> > + i915_hotplug_work_func);
> > INIT_WORK(_priv->hotplug.dig_port_work,
> > i915_digport_work_func);
> > INIT_WORK(_priv->hotplug.poll_init_work,
> > i915_hpd_poll_init_work);
> > INIT_DELAYED_WORK(_priv->hotplug.reenable_work, @@ -604,7
> > +610,7 @@ void intel_hpd_cancel_work(struct drm_i915_private
> > *dev_priv) spin_unlock_irq(_priv->irq_lock);
> > cancel_work_sync(_priv->hotplug.dig_port_work);
> > - cancel_work_sync(_priv->hotplug.hotplug_work);
> > + cancel_delayed_work_sync(_priv->hotplug.hotplug_work);
> > cancel_work_sync(_priv->hotplug.poll_init_work);
> > cancel_delayed_work_sync(_priv->hotplug.reenable_work);
> > }
> Jani - With these changes
Re: [Intel-gfx] [PATCH] drm/i915: delay hotplug scheduling
On Tue, 25 Sep 2018 10:18:36 +0300
Jani Nikula wrote:
> On some systems we get the hotplug irq on unplug before the DDC pins
> have been disconnected, resulting in connector status remaining
> connected. Since previous attempts at using hotplug live status before
> DDC have failed, the only viable option is to reduce the window for
> mistakes. Delay the hotplug processing.
>
> Reference:
> http://mid.mail-archive.com/CAB4CAwcfj32FovZwpTbekvHpoc3gG0sVbNtW8jeXRcp5qwEsmw@mail.gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107125
> Reported-by: Chris Chiu Tested-by: Chris Chiu
> Cc: Chris Chiu
> Cc: Guang Bai
> Cc: Ville Syrjälä
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_hotplug.c | 16 +++-
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 8624b4bdc242..4378be7a20d5
> 100644 --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -286,7 +286,7 @@ enum hpd_pin {
> #define HPD_STORM_DEFAULT_THRESHOLD 5
>
> struct i915_hotplug {
> - struct work_struct hotplug_work;
> + struct delayed_work hotplug_work;
>
> struct {
> unsigned long last_jiffies;
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c index
> 648a13c6043c..ce9bedbedcd1 100644 ---
> a/drivers/gpu/drm/i915/intel_hotplug.c +++
> b/drivers/gpu/drm/i915/intel_hotplug.c @@ -110,6 +110,9 @@ enum
> hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv, }
> }
>
> +/* Delay between irq and hotplug detect processing */
> +#define HOTPLUG_DELAY_MS 300
> +
> #define HPD_STORM_DETECT_PERIOD 1000
> #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000)
>
> @@ -319,7 +322,8 @@ static void i915_digport_work_func(struct
> work_struct *work) spin_lock_irq(_priv->irq_lock);
> dev_priv->hotplug.event_bits |= old_bits;
> spin_unlock_irq(_priv->irq_lock);
> - schedule_work(_priv->hotplug.hotplug_work);
> +
> schedule_delayed_work(_priv->hotplug.hotplug_work,
> +
> msecs_to_jiffies(HOTPLUG_DELAY_MS)); }
> }
>
> @@ -329,7 +333,7 @@ static void i915_digport_work_func(struct
> work_struct *work) static void i915_hotplug_work_func(struct
> work_struct *work) {
> struct drm_i915_private *dev_priv =
> - container_of(work, struct drm_i915_private,
> hotplug.hotplug_work);
> + container_of(work, struct drm_i915_private,
> hotplug.hotplug_work.work); struct drm_device *dev = _priv->drm;
> struct intel_connector *intel_connector;
> struct intel_encoder *intel_encoder;
> @@ -466,7 +470,8 @@ void intel_hpd_irq_handler(struct
> drm_i915_private *dev_priv, if (queue_dig)
> queue_work(dev_priv->hotplug.dp_wq,
> _priv->hotplug.dig_port_work); if (queue_hp)
> - schedule_work(_priv->hotplug.hotplug_work);
> +
> schedule_delayed_work(_priv->hotplug.hotplug_work,
> +
> msecs_to_jiffies(HOTPLUG_DELAY_MS)); }
>
> /**
> @@ -586,7 +591,8 @@ void intel_hpd_poll_init(struct drm_i915_private
> *dev_priv)
> void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> {
> - INIT_WORK(_priv->hotplug.hotplug_work,
> i915_hotplug_work_func);
> + INIT_DELAYED_WORK(_priv->hotplug.hotplug_work,
> + i915_hotplug_work_func);
> INIT_WORK(_priv->hotplug.dig_port_work,
> i915_digport_work_func); INIT_WORK(_priv->hotplug.poll_init_work,
> i915_hpd_poll_init_work);
> INIT_DELAYED_WORK(_priv->hotplug.reenable_work, @@ -604,7 +610,7
> @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> spin_unlock_irq(_priv->irq_lock);
> cancel_work_sync(_priv->hotplug.dig_port_work);
> - cancel_work_sync(_priv->hotplug.hotplug_work);
> + cancel_delayed_work_sync(_priv->hotplug.hotplug_work);
> cancel_work_sync(_priv->hotplug.poll_init_work);
> cancel_delayed_work_sync(_priv->hotplug.reenable_work);
> }
Jani - With these changes, my customer platform still fails the
HDMI hot-plug test - slowly unplug the HDMI cable,
the /sys/class/drm/card0/card0-HDMI-A-1/status still shows connected
state. My previous experiments indicated adding bigger delay before
calling "drm_kms_helper_hotplug_event(dev)", which lead to reading the
EDID, didn't have solid passing test results. The POC changes with
passing results from onsite utilize following algorithm -
1. in the hot-plug worker function, check the PCH hot-plug status pin
to see it's disco
[Intel-gfx] [PATCH xf86-video-intel] sna/io: Align the linear source buffer to cache line for 2d source copy
On BDW+ the linear source buffer of 2d source copy needs to start from
cache line boundary based on hardware design. Apply this alignment policy
to all platforms accordingly.
v2: Apply these changes only to SKL+ for not breaking old platforms
based on code review (Chris).
v3: Apply these changes for BDW+ onward chipsets based on hardware
design teams' inputs.
Cc: Chris Wilson
Signed-off-by: Guang Bai
---
src/sna/sna_io.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/sna/sna_io.c b/src/sna/sna_io.c
index d32bd58..35b90e5 100644
--- a/src/sna/sna_io.c
+++ b/src/sna/sna_io.c
@@ -1077,12 +1077,16 @@ tile:
/* Count the total number of bytes to be read and
allocate a
* single buffer large enough. Or if it is very small,
combine
-* with other allocations. */
+* with other allocations. Each sub-buffer starting
point
+* (offset) should be aligned to 64-byte cache line
boundary
+* for BDW+ according to hardware design specifications.
+*/
offset = 0;
for (n = 0; n < nbox_this_time; n++) {
int height = box[n].y2 - box[n].y1;
int width = box[n].x2 - box[n].x1;
offset += PITCH(width,
dst->drawable.bitsPerPixel >> 3) * height;
+ offset = ALIGN(offset, 64);
}
src_bo = kgem_create_buffer(kgem, offset,
@@ -1143,6 +1147,7 @@ tile:
box++;
offset += pitch * height;
+ offset = ALIGN(offset, 64);
} while (--nbox_this_time);
assert(offset == __kgem_buffer_size(src_bo));
sigtrap_put();
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xf86-video-intel] sna/io: Align the linear source buffer to cache line for 2d blt of SKL+
On SKL+ the linear source buffer has to start from cache line boundary
to meet the 2d engine source copy requirements. Apply this cache line
alignment policy for SKL+ only.
v2: Apply these changes only to SKL+ for not breaking old platforms
based on Chris Wilson's reviews.
Cc: Chris Wilson
Signed-off-by: Guang Bai
---
src/sna/sna_io.c | 47 +++
1 file changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/sna/sna_io.c b/src/sna/sna_io.c
index d32bd58..ae82d1f 100644
--- a/src/sna/sna_io.c
+++ b/src/sna/sna_io.c
@@ -1064,7 +1064,7 @@ tile:
if (kgem->gen >= 0100) {
cmd |= 8;
do {
- int nbox_this_time, rem;
+ int nbox_this_time, rem, pitch_aligned;
nbox_this_time = nbox;
rem = kgem_batch_space(kgem);
@@ -1077,12 +1077,19 @@ tile:
/* Count the total number of bytes to be read and
allocate a
* single buffer large enough. Or if it is very small,
combine
-* with other allocations. */
+* with other allocations. Each sub-buffer starting
point has
+* to be aligned to 64 bytes to conform SKL+ hardware
requirments.
+* Align the pitch of each sub-buffer to 64 bytes for
simplicities.
+*/
offset = 0;
for (n = 0; n < nbox_this_time; n++) {
int height = box[n].y2 - box[n].y1;
int width = box[n].x2 - box[n].x1;
- offset += PITCH(width,
dst->drawable.bitsPerPixel >> 3) * height;
+ if (kgem->gen >= 0110) {
+ pitch_aligned = ALIGN(PITCH(width,
dst->drawable.bitsPerPixel >> 3), 64);
+ offset += pitch_aligned * height;
+ } else
+ offset += PITCH(width,
dst->drawable.bitsPerPixel >> 3) * height;
}
src_bo = kgem_create_buffer(kgem, offset,
@@ -1113,14 +1120,24 @@ tile:
assert(box->x1 + dst_dx >= 0);
assert(box->y1 + dst_dy >= 0);
- memcpy_blt(src, (char *)ptr + offset,
- dst->drawable.bitsPerPixel,
- stride, pitch,
- box->x1 + src_dx, box->y1 +
src_dy,
- 0, 0,
- width, height);
+ if (kgem->gen >= 0110) {
+ pitch_aligned = ALIGN(pitch,
64);
+ memcpy_blt(src, (char *)ptr +
offset,
+
dst->drawable.bitsPerPixel,
+ stride,
pitch_aligned,
+ box->x1 + src_dx,
box->y1 + src_dy,
+ 0, 0,
+ width, height);
+ } else
+ memcpy_blt(src, (char *)ptr +
offset,
+
dst->drawable.bitsPerPixel,
+ stride, pitch,
+ box->x1 + src_dx,
box->y1 + src_dy,
+ 0, 0,
+ width, height);
assert(kgem->mode == KGEM_BLT);
+
b = kgem->batch + kgem->nbatch;
b[0] = cmd;
b[1] = br13;
@@ -1133,16 +1150,22 @@ tile:
KGEM_RELOC_FENCED,
0);
b[6] = 0;
- b[7] = pitch;
+ if (kgem->gen >= 0110)
+ b[7] = pitch_aligned;
+ else
+ b[7] = pitch;
+
*(uint6
[Intel-gfx] [PATCH xf86-video-intel] sna/io: Align the linear source buffer to cache line for 2d blt of SKL+
On SKL+ the linear source buffer has to start from cache line boundary
to meet the 2d engine source copy requirements. Apply this cache line
alignment policy for SKL+ only.
Signed-off-by: Guang Bai
---
src/sna/sna_io.c | 47 +++
1 file changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/sna/sna_io.c b/src/sna/sna_io.c
index d32bd58..48d9354 100644
--- a/src/sna/sna_io.c
+++ b/src/sna/sna_io.c
@@ -1064,7 +1064,7 @@ tile:
if (kgem->gen >= 0100) {
cmd |= 8;
do {
- int nbox_this_time, rem;
+ int nbox_this_time, rem, pitch_aligned;
nbox_this_time = nbox;
rem = kgem_batch_space(kgem);
@@ -1077,12 +1077,19 @@ tile:
/* Count the total number of bytes to be read and
allocate a
* single buffer large enough. Or if it is very small,
combine
-* with other allocations. */
+* with other allocations. Each sub-buffer starting
point has
+* to be aligned to 64 bytes to conform latest hardware
requirments.
+* Align the pitch of each sub-buffer to 64 bytes for
simplicities.
+*/
offset = 0;
for (n = 0; n < nbox_this_time; n++) {
int height = box[n].y2 - box[n].y1;
int width = box[n].x2 - box[n].x1;
- offset += PITCH(width,
dst->drawable.bitsPerPixel >> 3) * height;
+ if (kgem->gen >= 0110) {
+ pitch_aligned = ALIGN(PITCH(width,
dst->drawable.bitsPerPixel >> 3), 64);
+ offset += pitch_aligned * height;
+ } else
+ offset += PITCH(width,
dst->drawable.bitsPerPixel >> 3) * height;
}
src_bo = kgem_create_buffer(kgem, offset,
@@ -1113,14 +1120,24 @@ tile:
assert(box->x1 + dst_dx >= 0);
assert(box->y1 + dst_dy >= 0);
- memcpy_blt(src, (char *)ptr + offset,
- dst->drawable.bitsPerPixel,
- stride, pitch,
- box->x1 + src_dx, box->y1 +
src_dy,
- 0, 0,
- width, height);
+ if (kgem->gen >= 0110) {
+ pitch_aligned = ALIGN(pitch,
64);
+ memcpy_blt(src, (char *)ptr +
offset,
+
dst->drawable.bitsPerPixel,
+ stride,
pitch_aligned,
+ box->x1 + src_dx,
box->y1 + src_dy,
+ 0, 0,
+ width, height);
+ } else
+ memcpy_blt(src, (char *)ptr +
offset,
+
dst->drawable.bitsPerPixel,
+ stride, pitch,
+ box->x1 + src_dx,
box->y1 + src_dy,
+ 0, 0,
+ width, height);
assert(kgem->mode == KGEM_BLT);
+
b = kgem->batch + kgem->nbatch;
b[0] = cmd;
b[1] = br13;
@@ -1133,16 +1150,22 @@ tile:
KGEM_RELOC_FENCED,
0);
b[6] = 0;
- b[7] = pitch;
+ if (kgem->gen >= 0110)
+ b[7] = pitch_aligned;
+ else
+ b[7] = pitch;
+
*(uint64_t *)(b+8) =
kgem_add_reloc64(kgem,
kgem->nbatch +
Re: [Intel-gfx] [PATCH xf86-video-intel] sna/io: Align the linear source buffer to cache line for 2d blt
On Tue, 4 Sep 2018 08:01:36 +0100 Chris Wilson wrote: > Quoting Guang Bai (2018-09-04 06:37:31) > > On SKL+ the linear source buffer has to start from cache line > > boundary to meet the 2d engine source copy requirements. > > First update the requirement function. > > Are you sure about this? As this would be quite a reduction in > functionality. There's the bug on bdw+ for not starting on a > cacheline... > -Chris Yes. The latest B-spec for SKL+ XY_FAST_COPY_BLT has this 64-byte cache line requirment for both linear and tiled source surfaces. And I'm not aware of the bdw+ related issues. Without this cache line alignment added, the corruptions (I show you before) will happen. The corrutpion goes away after this alignment added. -Guang ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xf86-video-intel] sna/io: Align the linear source buffer to cache line for 2d blt
On SKL+ the linear source buffer has to start from cache line boundary
to meet the 2d engine source copy requirements.
Signed-off-by: Guang Bai
---
src/sna/sna_io.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/sna/sna_io.c b/src/sna/sna_io.c
index d32bd58..5bfbdbb 100644
--- a/src/sna/sna_io.c
+++ b/src/sna/sna_io.c
@@ -1064,7 +1064,7 @@ tile:
if (kgem->gen >= 0100) {
cmd |= 8;
do {
- int nbox_this_time, rem;
+ int nbox_this_time, rem, pitch_aligned;
nbox_this_time = nbox;
rem = kgem_batch_space(kgem);
@@ -1077,12 +1077,16 @@ tile:
/* Count the total number of bytes to be read and
allocate a
* single buffer large enough. Or if it is very small,
combine
-* with other allocations. */
+* with other allocations. Each sub-buffer starting
point has
+* to be aligned to 64 bytes to conform latest hardware
requirments.
+* Align the pitch of each sub-buffer to 64 bytes for
simplicities.
+*/
offset = 0;
for (n = 0; n < nbox_this_time; n++) {
int height = box[n].y2 - box[n].y1;
int width = box[n].x2 - box[n].x1;
- offset += PITCH(width,
dst->drawable.bitsPerPixel >> 3) * height;
+ pitch_aligned = ALIGN(PITCH(width,
dst->drawable.bitsPerPixel >> 3), 64);
+ offset += pitch_aligned * height;
}
src_bo = kgem_create_buffer(kgem, offset,
@@ -1113,9 +1117,10 @@ tile:
assert(box->x1 + dst_dx >= 0);
assert(box->y1 + dst_dy >= 0);
+ pitch_aligned = ALIGN(pitch, 64);
memcpy_blt(src, (char *)ptr + offset,
dst->drawable.bitsPerPixel,
- stride, pitch,
+ stride, pitch_aligned,
box->x1 + src_dx, box->y1 +
src_dy,
0, 0,
width, height);
@@ -1133,7 +1138,7 @@ tile:
KGEM_RELOC_FENCED,
0);
b[6] = 0;
- b[7] = pitch;
+ b[7] = pitch_aligned;
*(uint64_t *)(b+8) =
kgem_add_reloc64(kgem,
kgem->nbatch + 8, src_bo,
I915_GEM_DOMAIN_RENDER << 16 |
@@ -1142,7 +1147,7 @@ tile:
kgem->nbatch += 10;
box++;
- offset += pitch * height;
+ offset += pitch_aligned * height;
} while (--nbox_this_time);
assert(offset == __kgem_buffer_size(src_bo));
sigtrap_put();
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG] i915 HDMI connector status is connected after disconnection
On Fri, 6 Jul 2018 13:38:06 -0700 Rodrigo Vivi wrote: > On Fri, Jul 06, 2018 at 10:41:18AM -0700, Guang Bai wrote: > > On Fri, 6 Jul 2018 14:44:58 +0800 > > Chris Chiu wrote: > > > > > On Thu, Jul 5, 2018 at 10:40 PM, Ville Syrjälä > > > wrote: > > > > On Thu, Jul 05, 2018 at 03:58:36PM +0800, Chris Chiu wrote: > > > >> Hi, > > > >> We have few ASUS laptops X705FD (The new WiskyLake), X560UD > > > >> (intel i5-8250U), X530UN (intel i7-8550U) share the same > > > >> problem, which is the HDMI connector status stays connected > > > >> even the HDMI cable has been unplugged. Look into the > > > >> "/sys/class/drm/card0-HDMI-A-1/status" for checking the status > > > >> while plug/unplug the HDMI, it shows "disconnected" before > > > >> plug in HDMI cable, then switch to "connected" after plugin, > > > >> and still stay "connected" after unplug. This would cause the > > > >> audio output path cannot correctly switch from HDMI to > > > >> internal speaker after unplugging the HDMI. > > > >> > > > >> I then try to verify with the latest kernel 4.18.0-rc3+, the > > > >> bug still present. The full "dmesg" log is here. > > > >> https://gist.github.com/mschiu77/d761d7c5cf191b7868d4d7788ae087f1 > > > >> > > > >> The HDMI cable is plugged in at ~26th second. > > > >> "[ 26.214371] [drm:drm_detect_monitor_audio [drm]] Monitor has > > > >> basic audio support" > > > >> then unplug the HDMI at ~73th second. > > > >> "[ 73.328361] [drm:drm_detect_monitor_audio [drm]] Monitor has > > > >> basic audio support" > > > >> > > > >> Please advise what I can do to fix this. Thanks > > > > > > > > Pull the cable out faster? > > > > > > > > I presume this is the same old case of hpd disconnecting > > > > slightly before ddc and we still manage to read the EDID when > > > > processing the hpd irq. We kinda tried to fix that with the > > > > live status check but that thing failed spectacularly. > > > > > > > > -- > > > > Ville Syrjälä > > > > Intel > > > > > > Thanks for the suggestion. I tried pulling the cable out faster, > > > the status shows correctly. I also tried branch drm-tip of > > > https://cgit.freedesktop.org/drm/drm-tip > > > but the symptom persists. > > > > > > Anything I can help here? Or any old commit/patch I can try to do > > > some experiments? > > > > > > Chris > > > ___ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > I'm working on a same issue with HP KBL ThinPro laptop where both > > > > kernel 4.9.79 & 4.15.7 are failing the same way: > > - Unplug the HDMI cable slowly the connector status is still > > "connected" > > > > Debugging shows from kernel 4.9 and up to 4.18 drim/i915 behaves the > > same way: > > - When the HDMI calbe is unplugged, there is a transition time when > > the DDC lines are still connected and i915 can read back the EDID > > and honors "connected" state > > > > This problem does not happen on Windows7 & Windows 10 on the same > > failing platform - Windows KMD does *not* read the DDC when seeing > > the corresponding PCH/HPD pins indicating "disconnected" within > > 300ms-400ms period - This checking is done during bottom-half > > interrupt routines > > > > I worked patches with both 4.15.7 and 4.17.1 intercepting this > > Windows KMD logics; It seems these patches work for HP KBL ThinPro > > laptop - My patches are being tested by HP team - I was just about > > to post the open discussion on this topic to collect inputs from > > our community > > please send them out anyways ;) > > > - Do we have to refactor the HDMI hot-plug handling codes to cope > > with this long standing issue? > > What is your proposal for re-work? > > > - Is that OK to add 300ms-500ms delay "msleep(100)" in a loop in the > > bottom half of interrupt routines? > > Well, We fight to add the delays only when they are required by specs, > rather than experimental
Re: [Intel-gfx] [BUG] i915 HDMI connector status is connected after disconnection
On Fri, 6 Jul 2018 14:44:58 +0800 Chris Chiu wrote: > On Thu, Jul 5, 2018 at 10:40 PM, Ville Syrjälä > wrote: > > On Thu, Jul 05, 2018 at 03:58:36PM +0800, Chris Chiu wrote: > >> Hi, > >> We have few ASUS laptops X705FD (The new WiskyLake), X560UD > >> (intel i5-8250U), X530UN (intel i7-8550U) share the same problem, > >> which is the HDMI connector status stays connected even the HDMI > >> cable has been unplugged. Look into the > >> "/sys/class/drm/card0-HDMI-A-1/status" for checking the status > >> while plug/unplug the HDMI, it shows "disconnected" before plug in > >> HDMI cable, then switch to "connected" after plugin, and still > >> stay "connected" after unplug. This would cause the audio output > >> path cannot correctly switch from HDMI to internal speaker after > >> unplugging the HDMI. > >> > >> I then try to verify with the latest kernel 4.18.0-rc3+, the bug > >> still present. The full "dmesg" log is here. > >> https://gist.github.com/mschiu77/d761d7c5cf191b7868d4d7788ae087f1 > >> > >> The HDMI cable is plugged in at ~26th second. > >> "[ 26.214371] [drm:drm_detect_monitor_audio [drm]] Monitor has > >> basic audio support" > >> then unplug the HDMI at ~73th second. > >> "[ 73.328361] [drm:drm_detect_monitor_audio [drm]] Monitor has > >> basic audio support" > >> > >> Please advise what I can do to fix this. Thanks > > > > Pull the cable out faster? > > > > I presume this is the same old case of hpd disconnecting slightly > > before ddc and we still manage to read the EDID when processing > > the hpd irq. We kinda tried to fix that with the live status > > check but that thing failed spectacularly. > > > > -- > > Ville Syrjälä > > Intel > > Thanks for the suggestion. I tried pulling the cable out faster, the > status shows correctly. I also tried branch drm-tip of > https://cgit.freedesktop.org/drm/drm-tip > but the symptom persists. > > Anything I can help here? Or any old commit/patch I can try to do some > experiments? > > Chris > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx I'm working on a same issue with HP KBL ThinPro laptop where both kernel 4.9.79 & 4.15.7 are failing the same way: - Unplug the HDMI cable slowly the connector status is still "connected" Debugging shows from kernel 4.9 and up to 4.18 drim/i915 behaves the same way: - When the HDMI calbe is unplugged, there is a transition time when the DDC lines are still connected and i915 can read back the EDID and honors "connected" state This problem does not happen on Windows7 & Windows 10 on the same failing platform - Windows KMD does *not* read the DDC when seeing the corresponding PCH/HPD pins indicating "disconnected" within 300ms-400ms period - This checking is done during bottom-half interrupt routines I worked patches with both 4.15.7 and 4.17.1 intercepting this Windows KMD logics; It seems these patches work for HP KBL ThinPro laptop - My patches are being tested by HP team - I was just about to post the open discussion on this topic to collect inputs from our community - Do we have to refactor the HDMI hot-plug handling codes to cope with this long standing issue? - Is that OK to add 300ms-500ms delay "msleep(100)" in a loop in the bottom half of interrupt routines? Regards, -Guang ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix the frame buffer fence starvation
Currently a tiled frame buffer to be scanned out is always installed
with a fence, leading to fence starvation on gen9lp virtualization
use case where graphics stacks of service and guest OSes compete for
fences.
By design, this fence is always needed by i965-(GEN4-) platforms.
For GEN4 and above, the fence is only required when the frame buffer
compression is enabled.
Changes are made to follow graphics hardware design principles.
Signed-off-by: Guang Bai <guang@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 44 +++-
drivers/gpu/drm/i915/intel_drv.h | 4 +++-
drivers/gpu/drm/i915/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/intel_fbdev.c | 3 ++-
4 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3dbf5ed..59004f5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2068,7 +2068,8 @@ static unsigned int intel_surf_alignment(const struct
drm_framebuffer *fb,
}
struct i915_vma *
-intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
+intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation,
+ struct drm_plane *plane, enum pipe pipe)
{
struct drm_device *dev = fb->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -2076,6 +2077,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
unsigned int rotation)
struct i915_ggtt_view view;
struct i915_vma *vma;
u32 alignment;
+ bool needs_fence = false;
+ struct intel_plane *intel_plane;
WARN_ON(!mutex_is_locked(>struct_mutex));
@@ -2105,13 +2108,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
unsigned int rotation)
vma = i915_gem_object_pin_to_display_plane(obj, alignment, );
if (IS_ERR(vma))
goto err;
+ /*
+* Install the fence for pre-i965(GEN4-) tiled frame buffers all the
+* time but only do it for i965(GEN4) and beyond when the frame buffer
+* compression is enabled during boot up or runtime.
+*/
+ intel_plane = to_intel_plane(plane);
+ if (INTEL_GEN(dev_priv) < 4 || (intel_fbc_can_enable(dev_priv) &&
+ (pipe == PIPE_A) && intel_plane &&
+ (intel_plane->id == PLANE_PRIMARY)))
+ needs_fence = true;
if (i915_vma_is_map_and_fenceable(vma)) {
- /* Install a fence for tiled scan-out. Pre-i965 always needs a
-* fence, whereas 965+ only requires a fence if using
-* framebuffer compression. For simplicity, we always, when
-* possible, install a fence as the cost is not that onerous.
-*
+ /*
* If we fail to fence the tiled scanout, then either the
* modeset will reject the change (which is highly unlikely as
* the affected systems, all but one, do not have unmappable
@@ -2123,7 +2132,16 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
unsigned int rotation)
* something and try to run the system in a "less than optimal"
* mode that matches the user configuration.
*/
- i915_vma_pin_fence(vma);
+ if (needs_fence)
+ i915_vma_pin_fence(vma);
+ else if (vma->fence)
+ /*
+* For a reused fence, increase its ref count even if
+* it's not pinned to maintain the count consistancy.
+* This is because the count is unconditionally
+* decreased when the fence is unpinned.
+*/
+ vma->fence->pin_count++;
}
i915_vma_get(vma);
@@ -2808,7 +2826,8 @@ intel_find_initial_plane_obj(struct intel_crtc
*intel_crtc,
valid_fb:
mutex_lock(>struct_mutex);
intel_state->vma =
- intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+ intel_pin_and_fence_fb_obj(fb, primary->state->rotation,
+ primary, intel_crtc->pipe);
mutex_unlock(>struct_mutex);
if (IS_ERR(intel_state->vma)) {
DRM_ERROR("failed to pin boot fb on pipe %d: %li\n",
@@ -12711,8 +12730,9 @@ intel_prepare_plane_fb(struct drm_plane *plane,
ret = i915_gem_object_attach_phys(obj, align);
} else {
struct i915_vma *vma;
-
- vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
+ struct intel_crtc *temp_crtc = to_intel_crtc(new_state->crtc);
+ vma = intel_pin_and_fence_fb_obj(fb, new_state->rotatio
