| Message ID | 20210707153318.13903-1-shawn.c.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/dp: follow DP link CTS spec to read link status back | expand |
On Wed, 07 Jul 2021, Lee Shawn C <shawn.c.lee@intel.com> wrote: > Refer to DP link CTS 1.2/1.4 spec, the following test case request > source read DPCD 200h - 205h to get latest link status from sink. > > (4.3.2.4) Handling of IRQ HPD Pulse with No Error Status Bits Set > (400.3.2.1) Successful Link Re-training After IRQ HPD Pulse > Due to Loss of Symbol Lock: HBR2 Extension > (400.3.2.2) Successful Link Re-training After IRQ HPD Pulse Due > to Loss of Clock Recovery Lock: HBR2 Extension > (400.3.2.3) Successful Link Re-training After IRQ HPD Pulse Due > to Loss of Inter-lane Alignment Lock: HBR2 Extension > > So far, DRM DP driver just read back the link status from 202h > to 207h. DPR-120 would judge source can't pass these cases and > shows below error messages. > > "Test FAILED, Source DUT does not read DPCD registers 200h-205h > within 100 ms". Acked-by: Jani Nikula <jani.nikula@intel.com> for making the test pass iff everything else seems to work. The underlying question is, though, should we look at 0x200-0x201 for some status we don't look at? > > v2: Use sizeof() to retrieve array size. > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Lyude Paul <lyude@redhat.com> > Cc: Simon Ser <contact@emersion.fr> > Cc: Cooper Chiou <cooper.chiou@intel.com> > Cc: William Tseng <william.tseng@intel.com> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > --- > drivers/gpu/drm/drm_dp_helper.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 24bbc710c825..4f03df317d62 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -410,17 +410,19 @@ int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux, > u8 link_status[DP_LINK_STATUS_SIZE]) > { > int ret; > + u8 full_link_stat[DP_LINK_STATUS_SIZE + 2]; > > if (dp_phy == DP_PHY_DPRX) { > ret = drm_dp_dpcd_read(aux, > - DP_LANE0_1_STATUS, > - link_status, > - DP_LINK_STATUS_SIZE); > + DP_SINK_COUNT, > + full_link_stat, > + sizeof(full_link_stat)); > > if (ret < 0) > return ret; > > - WARN_ON(ret != DP_LINK_STATUS_SIZE); > + memcpy(link_status, full_link_stat + 2, DP_LINK_STATUS_SIZE); > + WARN_ON(ret != DP_LINK_STATUS_SIZE + 2); > > return 0; > }
On Wed, 07 Jul 2021, 4:14 p.m, Jani Nikula <jani.nikula@intel.com> wrote: >On Wed, 07 Jul 2021, Lee Shawn C <shawn.c.lee@intel.com> wrote: >> Refer to DP link CTS 1.2/1.4 spec, the following test case request >> source read DPCD 200h - 205h to get latest link status from sink. >> >> (4.3.2.4) Handling of IRQ HPD Pulse with No Error Status Bits Set >> (400.3.2.1) Successful Link Re-training After IRQ HPD Pulse >> Due to Loss of Symbol Lock: HBR2 Extension >> (400.3.2.2) Successful Link Re-training After IRQ HPD Pulse Due >> to Loss of Clock Recovery Lock: HBR2 Extension >> (400.3.2.3) Successful Link Re-training After IRQ HPD Pulse Due >> to Loss of Inter-lane Alignment Lock: HBR2 Extension >> >> So far, DRM DP driver just read back the link status from 202h to >> 207h. DPR-120 would judge source can't pass these cases and shows >> below error messages. >> >> "Test FAILED, Source DUT does not read DPCD registers 200h-205h within >> 100 ms". > >Acked-by: Jani Nikula <jani.nikula@intel.com> > >for making the test pass iff everything else seems to work. > >The underlying question is, though, should we look at 0x200-0x201 for some status we don't look at? > Look into 200h. While doing link train with DPRX. In my opinion, sink_count and cp_ready status should be constant. And sink would trigger HPD to source to notify 201h value was changed. Seems source driver don't need this value at link training stage as well. What do you think? Best regards, Shawn > >> >> v2: Use sizeof() to retrieve array size. >> >> Cc: Jani Nikula <jani.nikula@intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Lyude Paul <lyude@redhat.com> >> Cc: Simon Ser <contact@emersion.fr> >> Cc: Cooper Chiou <cooper.chiou@intel.com> >> Cc: William Tseng <william.tseng@intel.com> >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >> --- >> drivers/gpu/drm/drm_dp_helper.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_dp_helper.c >> b/drivers/gpu/drm/drm_dp_helper.c index 24bbc710c825..4f03df317d62 >> 100644 >> --- a/drivers/gpu/drm/drm_dp_helper.c >> +++ b/drivers/gpu/drm/drm_dp_helper.c >> @@ -410,17 +410,19 @@ int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux, >> u8 link_status[DP_LINK_STATUS_SIZE]) { >> int ret; >> + u8 full_link_stat[DP_LINK_STATUS_SIZE + 2]; >> >> if (dp_phy == DP_PHY_DPRX) { >> ret = drm_dp_dpcd_read(aux, >> - DP_LANE0_1_STATUS, >> - link_status, >> - DP_LINK_STATUS_SIZE); >> + DP_SINK_COUNT, >> + full_link_stat, >> + sizeof(full_link_stat)); >> >> if (ret < 0) >> return ret; >> >> - WARN_ON(ret != DP_LINK_STATUS_SIZE); >> + memcpy(link_status, full_link_stat + 2, DP_LINK_STATUS_SIZE); >> + WARN_ON(ret != DP_LINK_STATUS_SIZE + 2); >> >> return 0; >> } > >-- >Jani Nikula, Intel Open Source Graphics Center
On Wed, 08 Jul 2021, Lee Shawn C <shawn.c.lee@intel.com> wrote: >On Wed, 07 Jul 2021, 4:14 p.m, Jani Nikula <jani.nikula@intel.com> wrote: >>On Wed, 07 Jul 2021, Lee Shawn C <shawn.c.lee@intel.com> wrote: >>> Refer to DP link CTS 1.2/1.4 spec, the following test case request >>> source read DPCD 200h - 205h to get latest link status from sink. >>> >>> (4.3.2.4) Handling of IRQ HPD Pulse with No Error Status Bits Set >>> (400.3.2.1) Successful Link Re-training After IRQ HPD Pulse >>> Due to Loss of Symbol Lock: HBR2 Extension >>> (400.3.2.2) Successful Link Re-training After IRQ HPD Pulse Due >>> to Loss of Clock Recovery Lock: HBR2 Extension >>> (400.3.2.3) Successful Link Re-training After IRQ HPD Pulse Due >>> to Loss of Inter-lane Alignment Lock: HBR2 Extension >>> >>> So far, DRM DP driver just read back the link status from 202h to >>> 207h. DPR-120 would judge source can't pass these cases and shows >>> below error messages. >>> >>> "Test FAILED, Source DUT does not read DPCD registers 200h-205h >>> within >>> 100 ms". >> >>Acked-by: Jani Nikula <jani.nikula@intel.com> >> >>for making the test pass iff everything else seems to work. >> >>The underlying question is, though, should we look at 0x200-0x201 for some status we don't look at? >> > >Look into 200h. While doing link train with DPRX. In my opinion, sink_count and cp_ready status should be constant. >And sink would trigger HPD to source to notify 201h value was changed. Seems source driver don't need this value at link training stage as well. What do you think? > >Best regards, >Shawn > Hi Simon and all, Please share your recommendations for this patch to pass DP link layer compliance test. Thanks! Best regards, Shawn >> >>> >>> v2: Use sizeof() to retrieve array size. >>> >>> Cc: Jani Nikula <jani.nikula@intel.com> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> Cc: Lyude Paul <lyude@redhat.com> >>> Cc: Simon Ser <contact@emersion.fr> >>> Cc: Cooper Chiou <cooper.chiou@intel.com> >>> Cc: William Tseng <william.tseng@intel.com> >>> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >>> --- >>> drivers/gpu/drm/drm_dp_helper.c | 10 ++++++---- >>> 1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_dp_helper.c >>> b/drivers/gpu/drm/drm_dp_helper.c index 24bbc710c825..4f03df317d62 >>> 100644 >>> --- a/drivers/gpu/drm/drm_dp_helper.c >>> +++ b/drivers/gpu/drm/drm_dp_helper.c >>> @@ -410,17 +410,19 @@ int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux, >>> u8 link_status[DP_LINK_STATUS_SIZE]) { >>> int ret; >>> + u8 full_link_stat[DP_LINK_STATUS_SIZE + 2]; >>> >>> if (dp_phy == DP_PHY_DPRX) { >>> ret = drm_dp_dpcd_read(aux, >>> - DP_LANE0_1_STATUS, >>> - link_status, >>> - DP_LINK_STATUS_SIZE); >>> + DP_SINK_COUNT, >>> + full_link_stat, >>> + sizeof(full_link_stat)); >>> >>> if (ret < 0) >>> return ret; >>> >>> - WARN_ON(ret != DP_LINK_STATUS_SIZE); >>> + memcpy(link_status, full_link_stat + 2, DP_LINK_STATUS_SIZE); >>> + WARN_ON(ret != DP_LINK_STATUS_SIZE + 2); >>> >>> return 0; >>> } >> >>-- >>Jani Nikula, Intel Open Source Graphics Center >
On Wed, 08 Jul 2021, Lee Shawn C <shawn.c.lee@intel.com> wrote: >On Wed, 07 Jul 2021, 4:14 p.m, Jani Nikula <jani.nikula@intel.com> wrote: >>On Wed, 07 Jul 2021, Lee Shawn C <shawn.c.lee@intel.com> wrote: >>> Refer to DP link CTS 1.2/1.4 spec, the following test case request >>> source read DPCD 200h - 205h to get latest link status from sink. >>> >>> (4.3.2.4) Handling of IRQ HPD Pulse with No Error Status Bits Set >>> (400.3.2.1) Successful Link Re-training After IRQ HPD Pulse >>> Due to Loss of Symbol Lock: HBR2 Extension >>> (400.3.2.2) Successful Link Re-training After IRQ HPD Pulse Due >>> to Loss of Clock Recovery Lock: HBR2 Extension >>> (400.3.2.3) Successful Link Re-training After IRQ HPD Pulse Due >>> to Loss of Inter-lane Alignment Lock: HBR2 Extension >>> >>> So far, DRM DP driver just read back the link status from 202h to >>> 207h. DPR-120 would judge source can't pass these cases and shows >>> below error messages. >>> >>> "Test FAILED, Source DUT does not read DPCD registers 200h-205h >>> within >>> 100 ms". >> >>Acked-by: Jani Nikula <jani.nikula@intel.com> >> >>for making the test pass iff everything else seems to work. >> >>The underlying question is, though, should we look at 0x200-0x201 for some status we don't look at? >> > >Look into 200h. While doing link train with DPRX. In my opinion, sink_count and cp_ready status should be constant. >And sink would trigger HPD to source to notify 201h value was changed. Seems source driver don't need this value at link training stage as well. What do you think? > >Best regards, >Shawn > Hi Simon and all, Please share your recommendations for this patch to pass DP link layer compliance test. Thanks! Best regards, Shawn >> >>> >>> v2: Use sizeof() to retrieve array size. >>> >>> Cc: Jani Nikula <jani.nikula@intel.com> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> Cc: Lyude Paul <lyude@redhat.com> >>> Cc: Simon Ser <contact@emersion.fr> >>> Cc: Cooper Chiou <cooper.chiou@intel.com> >>> Cc: William Tseng <william.tseng@intel.com> >>> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >>> --- >>> drivers/gpu/drm/drm_dp_helper.c | 10 ++++++---- >>> 1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_dp_helper.c >>> b/drivers/gpu/drm/drm_dp_helper.c index 24bbc710c825..4f03df317d62 >>> 100644 >>> --- a/drivers/gpu/drm/drm_dp_helper.c >>> +++ b/drivers/gpu/drm/drm_dp_helper.c >>> @@ -410,17 +410,19 @@ int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux, >>> u8 link_status[DP_LINK_STATUS_SIZE]) { >>> int ret; >>> + u8 full_link_stat[DP_LINK_STATUS_SIZE + 2]; >>> >>> if (dp_phy == DP_PHY_DPRX) { >>> ret = drm_dp_dpcd_read(aux, >>> - DP_LANE0_1_STATUS, >>> - link_status, >>> - DP_LINK_STATUS_SIZE); >>> + DP_SINK_COUNT, >>> + full_link_stat, >>> + sizeof(full_link_stat)); >>> >>> if (ret < 0) >>> return ret; >>> >>> - WARN_ON(ret != DP_LINK_STATUS_SIZE); >>> + memcpy(link_status, full_link_stat + 2, DP_LINK_STATUS_SIZE); >>> + WARN_ON(ret != DP_LINK_STATUS_SIZE + 2); >>> >>> return 0; >>> } >> >>-- >>Jani Nikula, Intel Open Source Graphics Center >
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 24bbc710c825..4f03df317d62 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -410,17 +410,19 @@ int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux, u8 link_status[DP_LINK_STATUS_SIZE]) { int ret; + u8 full_link_stat[DP_LINK_STATUS_SIZE + 2]; if (dp_phy == DP_PHY_DPRX) { ret = drm_dp_dpcd_read(aux, - DP_LANE0_1_STATUS, - link_status, - DP_LINK_STATUS_SIZE); + DP_SINK_COUNT, + full_link_stat, + sizeof(full_link_stat)); if (ret < 0) return ret; - WARN_ON(ret != DP_LINK_STATUS_SIZE); + memcpy(link_status, full_link_stat + 2, DP_LINK_STATUS_SIZE); + WARN_ON(ret != DP_LINK_STATUS_SIZE + 2); return 0; }
Refer to DP link CTS 1.2/1.4 spec, the following test case request source read DPCD 200h - 205h to get latest link status from sink. (4.3.2.4) Handling of IRQ HPD Pulse with No Error Status Bits Set (400.3.2.1) Successful Link Re-training After IRQ HPD Pulse Due to Loss of Symbol Lock: HBR2 Extension (400.3.2.2) Successful Link Re-training After IRQ HPD Pulse Due to Loss of Clock Recovery Lock: HBR2 Extension (400.3.2.3) Successful Link Re-training After IRQ HPD Pulse Due to Loss of Inter-lane Alignment Lock: HBR2 Extension So far, DRM DP driver just read back the link status from 202h to 207h. DPR-120 would judge source can't pass these cases and shows below error messages. "Test FAILED, Source DUT does not read DPCD registers 200h-205h within 100 ms". v2: Use sizeof() to retrieve array size. Cc: Jani Nikula <jani.nikula@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Lyude Paul <lyude@redhat.com> Cc: Simon Ser <contact@emersion.fr> Cc: Cooper Chiou <cooper.chiou@intel.com> Cc: William Tseng <william.tseng@intel.com> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> --- drivers/gpu/drm/drm_dp_helper.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)