| Message ID | 1520029558-12219-1-git-send-email-matthew.s.atwood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote: > From: Matt Atwood <matthew.s.atwood@intel.com> > > For panels that do not follow Display Port specifications mask off invalid > values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable > values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e > describes another feature. Currently the code uses all of DPCD 0x0000e and > can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. > This address is read for both clock recovery and channel equalization. > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/drm_dp_helper.c | 4 ++-- > include/drm/drm_dp_helper.h | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index adf79be..a7e9b75 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) > if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > udelay(100); > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); > } > EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); > > @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { > if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > udelay(400); > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); > } > EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index da58a42..77ba003 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -118,6 +118,7 @@ > # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher */ > > #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ > +# define DP_TRAINING_AUX_RD_MASK 0x7 /* 1.4 */ I confirmed it is already part of 1.2 and 1.3 So probably a good idea to already to s/XXX 1.2?/1.2/ but this is optional and up to you. With s/1.4/1.2 feel free to add: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> (well... in a hope that no other bad panel uses 5,6 or 7, that are also reserved values. But I believe these cases shouldn't be as bad as this one you faced here anyways) > > #define DP_ADAPTER_CAP 0x00f /* 1.2 */ > # define DP_FORCE_LOAD_SENSE_CAP (1 << 0) > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Thanks for the patch. Masking the training AUX RD interval value to get only the allowed values sounds of 0-4 is absolutely needed. Just one nit below: On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote: > From: Matt Atwood <matthew.s.atwood@intel.com> > > For panels that do not follow Display Port specifications mask off invalid > values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable It would be good to call out DP specification and a particular version number if possible So DP 1.2 specification. With that change, Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> > values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e > describes another feature. Currently the code uses all of DPCD 0x0000e and > can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. > This address is read for both clock recovery and channel equalization. > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/drm_dp_helper.c | 4 ++-- > include/drm/drm_dp_helper.h | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index adf79be..a7e9b75 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) > if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > udelay(100); > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); > } > EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); > > @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { > if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > udelay(400); > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); > } > EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index da58a42..77ba003 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -118,6 +118,7 @@ > # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher */ > > #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ > +# define DP_TRAINING_AUX_RD_MASK 0x7 /* 1.4 */ > > #define DP_ADAPTER_CAP 0x00f /* 1.2 */ > # define DP_FORCE_LOAD_SENSE_CAP (1 << 0) > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Matt, On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote: > From: Matt Atwood <matthew.s.atwood@intel.com> > > For panels that do not follow Display Port specifications mask off invalid > values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable > values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e > describes another feature. Currently the code uses all of DPCD 0x0000e and > can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. > This address is read for both clock recovery and channel equalization. > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> Tested-by: Benson Leung <bleung@chromium.org> I tested this patch on that particularly problematic panel, with the out of spec value of 9. With the masking, the panel link trains successfully. Thanks!
On Fri, 02 Mar 2018, matthew.s.atwood@intel.com wrote: > From: Matt Atwood <matthew.s.atwood@intel.com> > > For panels that do not follow Display Port specifications mask off invalid > values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable > values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e > describes another feature. Currently the code uses all of DPCD 0x0000e and > can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. > This address is read for both clock recovery and channel equalization. > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/drm_dp_helper.c | 4 ++-- > include/drm/drm_dp_helper.h | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index adf79be..a7e9b75 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) > if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > udelay(100); > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); > } > EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); > > @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { > if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > udelay(400); > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); > } > EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index da58a42..77ba003 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -118,6 +118,7 @@ > # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher */ > > #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ > +# define DP_TRAINING_AUX_RD_MASK 0x7 /* 1.4 */ > > #define DP_ADAPTER_CAP 0x00f /* 1.2 */ > # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
On Fri, 02 Mar 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote: >> From: Matt Atwood <matthew.s.atwood@intel.com> >> >> For panels that do not follow Display Port specifications mask off invalid >> values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable >> values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e >> describes another feature. Currently the code uses all of DPCD 0x0000e and >> can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. >> This address is read for both clock recovery and channel equalization. >> >> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> >> --- >> drivers/gpu/drm/drm_dp_helper.c | 4 ++-- >> include/drm/drm_dp_helper.h | 1 + >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c >> index adf79be..a7e9b75 100644 >> --- a/drivers/gpu/drm/drm_dp_helper.c >> +++ b/drivers/gpu/drm/drm_dp_helper.c >> @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) >> if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) >> udelay(100); >> else >> - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); >> + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); >> } >> EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); >> >> @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { >> if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) >> udelay(400); >> else >> - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); >> + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); >> } >> EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); >> >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index da58a42..77ba003 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -118,6 +118,7 @@ >> # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher */ >> >> #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ >> +# define DP_TRAINING_AUX_RD_MASK 0x7 /* 1.4 */ > > I confirmed it is already part of 1.2 and 1.3 > So probably a good idea to already to s/XXX 1.2?/1.2/ > but this is optional and up to you. > > With s/1.4/1.2 feel free to add: > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Please send v2 Cc: intel-gfx so we get i915 CI on this. It doesn't automatically happen on dri-devel yet. BR, Jani. > > (well... in a hope that no other bad panel uses 5,6 or 7, > that are also reserved values. But I believe these cases shouldn't > be as bad as this one you faced here anyways) > >> >> #define DP_ADAPTER_CAP 0x00f /* 1.2 */ >> # define DP_FORCE_LOAD_SENSE_CAP (1 << 0) >> -- >> 2.7.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a7e9b75 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(100); else - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); } EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(400); else - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); } EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index da58a42..77ba003 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,7 @@ # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher */ #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ +# define DP_TRAINING_AUX_RD_MASK 0x7 /* 1.4 */ #define DP_ADAPTER_CAP 0x00f /* 1.2 */ # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)