On Tue, 2018-03-06 at 17:36 -0800, Manasi Navare wrote:
> On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:
> >
> >
> >
> > On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
> > > On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@xxxxxxxxx wrote:
> > > > From: Matt Atwood <matthew.s.atwood@xxxxxxxxx>
> > > >
> > > > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8
> > > > bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for
> > > > panels that use this new feature, this would cause a wait interval for
> > > > clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.
> > > > This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.
> > > > To avoid breaking panels
> >
> > See comment below:
> >
> > > that are not spec compliant we now warn on
> > > > invalid values.
> > > >
> > > > V2: commit title/message, masking all 7 bits, warn on out of spec values.
> > >
> > > this approach is even better imho.
> > >
> > > >
> > > > Signed-off-by: Matt Atwood <matthew.s.atwood@xxxxxxxxx>
> > >
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > >
> > > > ---
> > > > drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
> > > > include/drm/drm_dp_helper.h | 1 +
> > > > 2 files changed, 15 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > > index adf79be..a718ccc 100644
> > > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > > @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI
> > > > EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
> > > >
> > > > void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> > > > - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > > > + int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> > > > +
> > > > + if (rd_interval > 4)
> > > > + DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> >
> > Some default for panels without a valid value?
> > rd_interval = 4;
> > "AUX read interval out of range, using max %d ms"
> >
>
> The problem with setting the upper bound to 4 is that there are panels
> that do not follow the spec and expect a longer than 16 ms delay. So
> if we set the upper bound to 4 in those cases the panels might not work.
>
> So we decided to go with this approach where we tell the users that panel is requesting
> out of range AUX value but then set it to the value * 4 in the else part.
>
Thanks for the clarification. My concern is if the DPCD is advertizing
an out of spec value, it might as well be advertizing a delay that the
panel doesn't need. And I thought panel quirks were supposed to be used
for working around things like this. To be clear, this is not a big
enough concern to block this fix.
Like I said in the other email, this patch refers to DP 1.4, shouldn't
the clock recovery delay be updated too (in a separate patch)?
> Manasi
>
> >
> > > > +
> > > > + if (rd_interval == 0)
> > > > udelay(100);
> > > > else
> > > > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > > > + mdelay(rd_interval * 4);
> > > > }
> > > > EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
> > > >
> > > > void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> > > > - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > > > + int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> > > > +
> > > > + if (rd_interval > 4)
> > > > + DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> > > > +
> > > > + if (rd_interval == 0)
> > > > udelay(400);
> > > > else
> > > > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > > > + mdelay(rd_interval * 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..f80acf1 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 0x7F /* 1.3 */
> > > >
> > > > #define DP_ADAPTER_CAP 0x00f /* 1.2 */
> > > > # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
> > > > --
> > > > 2.7.4
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
