2019-12-24 08:43:54

by Kai-Heng Feng

Subject: [PATCH v3] drm/i915: Re-init lspcon after HPD if lspcon probe failed

On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port
becomes useless and never responds to cable hotplugging:
[ 3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
[ 3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port D

Seems like the lspcon chip on the system in question only gets powered
after the cable is plugged.

So let's call lspcon_init() dynamically to properly initialize the
lspcon chip and make HDMI port work.

Closes: https://gitlab.freedesktop.org/drm/intel/issues/203
Signed-off-by: Kai-Heng Feng <[email protected]>
---
v3:
- Make sure it's handled under long HPD case.

v2:
- Move lspcon_init() inside of intel_dp_hpd_pulse().

drivers/gpu/drm/i915/display/intel_dp.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index fe31bbfd6c62..a72c9c041c60 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6573,6 +6573,7 @@ enum irqreturn
intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
{
struct intel_dp *intel_dp = &intel_dig_port->dp;
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);

if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
/*
@@ -6593,7 +6594,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
long_hpd ? "long" : "short");

if (long_hpd) {
- intel_dp->reset_link_params = true;
+ if (intel_dig_port->base.type == INTEL_OUTPUT_DDI &&
+ HAS_LSPCON(dev_priv) && !intel_dig_port->lspcon.active)
+ lspcon_init(intel_dig_port);
+ else
+ intel_dp->reset_link_params = true;
+
return IRQ_NONE;
}

--
2.17.1

2020-01-06 08:36:26

by Kai-Heng Feng

Subject: Re: [PATCH v3] drm/i915: Re-init lspcon after HPD if lspcon probe failed

Hi Jani,

> On Dec 24, 2019, at 16:42, Kai-Heng Feng <[email protected]> wrote:
>
> On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port
> becomes useless and never responds to cable hotplugging:
> [ 3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
> [ 3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port D
>
> Seems like the lspcon chip on the system in question only gets powered
> after the cable is plugged.
>
> So let's call lspcon_init() dynamically to properly initialize the
> lspcon chip and make HDMI port work.

Do you have any further suggestion for this patch?

Kai-Heng

>
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/203
> Signed-off-by: Kai-Heng Feng <[email protected]>
> ---
> v3:
> - Make sure it's handled under long HPD case.
>
> v2:
> - Move lspcon_init() inside of intel_dp_hpd_pulse().
>
> drivers/gpu/drm/i915/display/intel_dp.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index fe31bbfd6c62..a72c9c041c60 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6573,6 +6573,7 @@ enum irqreturn
> intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> {
> struct intel_dp *intel_dp = &intel_dig_port->dp;
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>
> if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> /*
> @@ -6593,7 +6594,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> long_hpd ? "long" : "short");
>
> if (long_hpd) {
> - intel_dp->reset_link_params = true;
> + if (intel_dig_port->base.type == INTEL_OUTPUT_DDI &&
> + HAS_LSPCON(dev_priv) && !intel_dig_port->lspcon.active)
> + lspcon_init(intel_dig_port);
> + else
> + intel_dp->reset_link_params = true;
> +
> return IRQ_NONE;
> }
>
> --
> 2.17.1
>

2020-01-14 14:13:44

by Jani Nikula

Subject: Re: [PATCH v3] drm/i915: Re-init lspcon after HPD if lspcon probe failed

On Mon, 06 Jan 2020, Kai-Heng Feng <[email protected]> wrote:
> Hi Jani,
>
>> On Dec 24, 2019, at 16:42, Kai-Heng Feng <[email protected]> wrote:
>>
>> On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port
>> becomes useless and never responds to cable hotplugging:
>> [ 3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
>> [ 3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port D
>>
>> Seems like the lspcon chip on the system in question only gets powered
>> after the cable is plugged.
>>
>> So let's call lspcon_init() dynamically to properly initialize the
>> lspcon chip and make HDMI port work.
>
> Do you have any further suggestion for this patch?

Sorry for taking so long. Comments inline.

>
> Kai-Heng
>
>>
>> Closes: https://gitlab.freedesktop.org/drm/intel/issues/203
>> Signed-off-by: Kai-Heng Feng <[email protected]>
>> ---
>> v3:
>> - Make sure it's handled under long HPD case.
>>
>> v2:
>> - Move lspcon_init() inside of intel_dp_hpd_pulse().
>>
>> drivers/gpu/drm/i915/display/intel_dp.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index fe31bbfd6c62..a72c9c041c60 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -6573,6 +6573,7 @@ enum irqreturn
>> intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>> {
>> struct intel_dp *intel_dp = &intel_dig_port->dp;
>> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>>
>> if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
>> /*
>> @@ -6593,7 +6594,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>> long_hpd ? "long" : "short");
>>
>> if (long_hpd) {
>> - intel_dp->reset_link_params = true;
>> + if (intel_dig_port->base.type == INTEL_OUTPUT_DDI &&
>> + HAS_LSPCON(dev_priv) && !intel_dig_port->lspcon.active)
>> + lspcon_init(intel_dig_port);
>> + else
>> + intel_dp->reset_link_params = true;
>> +

Hmm, I think this will try to init lspcon on ports that do not have
lspcon. Also, why wouldn't we reset the params?

I think this boils down to just adding the following lines:

if (intel_bios_is_lspcon_present(dev_priv, intel_dig_port->base.port) &&
!intel_dig_port->lspcon.active)
lspcon_init(intel_dig_port);


Ville?

BR,
Jani.

>> return IRQ_NONE;
>> }
>>
>> --
>> 2.17.1
>>
>

--
Jani Nikula, Intel Open Source Graphics Center

2020-01-14 14:56:59

by Ville Syrjälä

Subject: Re: [PATCH v3] drm/i915: Re-init lspcon after HPD if lspcon probe failed

On Tue, Jan 14, 2020 at 04:11:40PM +0200, Jani Nikula wrote:
> On Mon, 06 Jan 2020, Kai-Heng Feng <[email protected]> wrote:
> > Hi Jani,
> >
> >> On Dec 24, 2019, at 16:42, Kai-Heng Feng <[email protected]> wrote:
> >>
> >> On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port
> >> becomes useless and never responds to cable hotplugging:
> >> [ 3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
> >> [ 3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port D
> >>
> >> Seems like the lspcon chip on the system in question only gets powered
> >> after the cable is plugged.
> >>
> >> So let's call lspcon_init() dynamically to properly initialize the
> >> lspcon chip and make HDMI port work.
> >
> > Do you have any further suggestion for this patch?
>
> Sorry for taking so long. Comments inline.
>
> >
> > Kai-Heng
> >
> >>
> >> Closes: https://gitlab.freedesktop.org/drm/intel/issues/203
> >> Signed-off-by: Kai-Heng Feng <[email protected]>
> >> ---
> >> v3:
> >> - Make sure it's handled under long HPD case.
> >>
> >> v2:
> >> - Move lspcon_init() inside of intel_dp_hpd_pulse().
> >>
> >> drivers/gpu/drm/i915/display/intel_dp.c | 8 +++++++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index fe31bbfd6c62..a72c9c041c60 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -6573,6 +6573,7 @@ enum irqreturn
> >> intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >> {
> >> struct intel_dp *intel_dp = &intel_dig_port->dp;
> >> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >>
> >> if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> >> /*
> >> @@ -6593,7 +6594,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >> long_hpd ? "long" : "short");
> >>
> >> if (long_hpd) {
> >> - intel_dp->reset_link_params = true;
> >> + if (intel_dig_port->base.type == INTEL_OUTPUT_DDI &&
> >> + HAS_LSPCON(dev_priv) && !intel_dig_port->lspcon.active)
> >> + lspcon_init(intel_dig_port);
> >> + else
> >> + intel_dp->reset_link_params = true;
> >> +
>
> Hmm, I think this will try to init lspcon on ports that do not have
> lspcon. Also, why wouldn't we reset the params?
>
> I think this boils down to just adding the following lines:
>
> if (intel_bios_is_lspcon_present(dev_priv, intel_dig_port->base.port) &&
> !intel_dig_port->lspcon.active)
> lspcon_init(intel_dig_port);
>
>
> Ville?

This won't work right. Eg. intel_infoframe_init() assumes that lspcon
init happens during driver load. We should probably change that to just
trust the VBT and simply move the lspcon probe (if we even need one)
into dp_detect() instead of sprinkling it around in several places.

--
Ville Syrj?l?
Intel