On Thu, Nov 29, 2018 at 11:43:18AM -0800, Manasi Navare wrote:
> On Thu, Nov 29, 2018 at 11:28:34AM -0800, Rodrigo Vivi wrote:
> > On Thu, Nov 29, 2018 at 10:34:05AM -0800, Manasi Navare wrote:
> > > From: Matt Atwood <matthew.s.atwood@xxxxxxxxx>
> > >
> > > According to DP spec (2.9.3.1 of DP 1.4) if
> > > EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD
> > > 02200h through 0220Fh shall contain the DPRX's true capability. These
> > > values will match 00000h through 0000Fh, except for DPCD_REV,
> > > MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
> > >
> > > Read from DPCD once for all 3 values as this is an expensive operation.
> > > Spec mentions that all of address space 02200h through 0220Fh should
> > > contain the right information however currently only 3 values can
> > > differ.
> > >
> > > There is no address space in the intel_dp->dpcd struct for addresses
> > > 02200h through 0220Fh, and since so much of the data is a identical,
> > > simply overwrite the values stored in 00000h through 0000Fh with the
> > > values that can be overwritten from addresses 02200h through 0220Fh.
> > >
> > > This patch helps with backward compatibility for devices pre DP1.3.
> > >
> > > v2: read only dpcd values which can be affected, remove incorrect check,
> > > split into drm include changes into separate patch, commit message,
> > > verbose debugging statements during overwrite.
> > > v3: white space fixes
> > > v4: make path dependent on DPCD revision > 1.2
> > > v5: split into function, removed DPCD rev check
> > > v6: add debugging prints for early exit conditions
> > > v7 (From Manasi):
> > > * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> > > * Exit early (Jani N)
> > > v8 (From Manasi):
> >
> > it seems you should have signed-off this commit.
>
> Oh yes will do
>
> >
> > > * Get rid of superfluous debug prints (Jani N)
> > > * Print entire base DPCD before memcpy (Jani N)
> > >
> > > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> > > Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@xxxxxxxxx>
> > > Tested-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > > Acked-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> >
> > This changed a lot that I'm not sure my rv-b should remain ;)
>
> Yes agree, i will remove that
>
> >
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 35 +++++++++++++++++++++++++++++++++
> > > 1 file changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 70ae3d57316b..5698441abe47 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3802,6 +3802,39 @@ intel_dp_link_down(struct intel_encoder *encoder,
> > > }
> > > }
> > >
> > > +static void
> > > +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> > > +{
> > > + u8 dpcd_ext[6];
> > > +
> > > + /*
> > > + * Prior to DP1.3 the bit represented by
> > > + * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> > > + * if it is set DP_DPCD_REV at 0000h could be at a value less than
> > > + * the true capability of the panel. The only way to check is to
> > > + * then compare 0000h and 2200h.
> > > + */
> > > + if (!(intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > > + DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT))
> > > + return;
> > > +
> > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > > + &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
> > > + DRM_ERROR("DPCD failed read at extended capabilities\n");
> > > + return;
> > > + }
> > > + if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> > > + DRM_DEBUG_KMS("DPCD extended DPCD rev less than base DPCD rev\n");
> > > + return;
> > > + }
> >
> > no strong feeling, just yet-another bikesheding:
> > but the mix with blank lines and no blank lines is at least strange to my OCD.
>
> Guess the newlines are just residues from spins, will make it uniform
>
> >
> > > + if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
> > > + return;
> > > +
> >
> > > + DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> > > + (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> > > + memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> >
> > hm... I'm afraid the full copy might be dangerous.
>
> We are not doing full copy here since the size of dpcd_ext is only 6 bytes.
> We are only restricting it from 0x2200 to 0x2205
>
> And the reason we are doing all 6 is because memcmp returned that something changed.
>
> Manasi
>
> >
> > 0x2215 to 0x22FF is
> > "RESERVED for Extended Receiver V Capability Read all 0s"
> >
> > On this range of the original one we have
> > 0x0021 which contains DP_MST_CAP bit that we use in our code.
> >
> > This full copy seems to have the potential to break MST if not
> > more cases.
> >
> > I prefer the individual copy of the bits that we know that we need.
> >
> > but if you prefer the full copy it seems that it would be better
> > to limit memcopy to 0x0015
Duh! I replied and went out for lunch.
while walking to the cafeteria I realized that I didn't check
the size local dpcd_ext... but it was too late! ;)
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> >
> > Thanks,
> > Rodrigo.
> >
> > > +}
> > > +
> > > bool
> > > intel_dp_read_dpcd(struct intel_dp *intel_dp)
> > > {
> > > @@ -3809,6 +3842,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
> > > sizeof(intel_dp->dpcd)) < 0)
> > > return false; /* aux transfer failed */
> > >
> > > + intel_dp_extended_receiver_capabilities(intel_dp);
> > > +
> > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
> > >
> > > return intel_dp->dpcd[DP_DPCD_REV] != 0;
> > > --
> > > 2.19.1
> > >
> > > _______________________________________________
> > > 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
