On Tue, Sep 08, 2020 at 01:51:56PM -0400, Lyude Paul wrote:
> On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Deal with more cases in drm_dp_downstream_max_bpc():
> > - DPCD 1.0 -> assume 8bpc for non-DP
> > - DPCD 1.1+ DP (or DP++ with DP sink) -> allow anything
> > - DPCD 1.1+ TMDS -> check the caps, assume 8bpc if the value is crap
> > - anything else -> assume 8bpc
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/drm_dp_helper.c | 69 +++++++++++--------
> > .../drm/i915/display/intel_display_debugfs.c | 3 +-
> > drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
> > include/drm/drm_dp_helper.h | 10 ++-
> > 4 files changed, 51 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index 0fcb94f7dbe5..ab87209c25d8 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -653,36 +653,44 @@ int drm_dp_downstream_max_clock(const u8
> > dpcd[DP_RECEIVER_CAP_SIZE],
> > EXPORT_SYMBOL(drm_dp_downstream_max_clock);
> >
> > /**
> > - * drm_dp_downstream_max_bpc() - extract branch device max
> > - * bits per component
> > - * @dpcd: DisplayPort configuration data
> > - * @port_cap: port capabilities
> > - *
> > - * See also:
> > - * drm_dp_read_downstream_info()
> > - * drm_dp_downstream_max_clock()
> > - *
> > - * Returns: Max bpc on success or 0 if max bpc not defined
> > - */
> > + * drm_dp_downstream_max_bpc() - extract downstream facing port max
> > + * bits per component
> > + * @dpcd: DisplayPort configuration data
> > + * @port_cap: downstream facing port capabilities
> > + * @edid: EDID
> > + *
> > + * Returns max bpc on success or 0 if max bpc not defined
> > + */
> > int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > - const u8 port_cap[4])
> > + const u8 port_cap[4],
> > + const struct edid *edid)
> > {
> > - int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> > - bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > - DP_DETAILED_CAP_INFO_AVAILABLE;
> > - int bpc;
> > -
> > - if (!detailed_cap_info)
>
> I don't think we can drop this check. There's a somewhat surprising blurb
> about downstream port caps in the DP 2.0 spec (section 5.3.3.1):
>
> In addition, the adapter shall set the Detailed Capabilities Info registers
> (DPCD Addresses 00080h through 0008Fh) to show all the downstream types,
> including DFP 0. Either one or four bytes are used, per DFP type
> indication. Therefore, up to 16 (with 1-byte descriptor) or four (with 4-
> byte descriptor) DFP capabilities can be stored.
>
> I've never once actually seen a sink do this, but this does mean it's
> technically possible tthat if we don't check the detailed caps bit then we
> might end up reading another port's DFP type instead of max_bpc info. Note
> though that we can make the assumption the four byte version of the field is
> used for DP 1.4+
The check is now ...
>
> > + if (!drm_dp_is_branch(dpcd))
> > return 0;
> >
> > - switch (type) {
> > - case DP_DS_PORT_TYPE_VGA:
> > - case DP_DS_PORT_TYPE_DVI:
> > - case DP_DS_PORT_TYPE_HDMI:
> > + if (dpcd[DP_DPCD_REV] < 0x11) {
> > + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > DP_DWN_STRM_PORT_TYPE_MASK) {
> > + case DP_DWN_STRM_PORT_TYPE_DP:
> > + return 0;
> > + default:
> > + return 8;
> > + }
> > + }
> > +
> > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> > + case DP_DS_PORT_TYPE_DP:
> > + return 0;
> > case DP_DS_PORT_TYPE_DP_DUALMODE:
> > - bpc = port_cap[2] & DP_DS_MAX_BPC_MASK;
> > + if (is_edid_digital_input_dp(edid))
> > + return 0;
> > + fallthrough;
> > + case DP_DS_PORT_TYPE_HDMI:
> > + case DP_DS_PORT_TYPE_DVI:
> > + case DP_DS_PORT_TYPE_VGA:
> > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> > + return 8;
... here
> >
> > - switch (bpc) {
> > + switch (port_cap[2] & DP_DS_MAX_BPC_MASK) {
> > case DP_DS_8BPC:
> > return 8;
> > case DP_DS_10BPC:
> > @@ -691,10 +699,12 @@ int drm_dp_downstream_max_bpc(const u8
> > dpcd[DP_RECEIVER_CAP_SIZE],
> > return 12;
> > case DP_DS_16BPC:
> > return 16;
> > + default:
> > + return 8;
> > }
> > - fallthrough;
> > + break;
> > default:
> > - return 0;
> > + return 8;
> > }
> > }
> > EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
> > @@ -717,12 +727,15 @@ EXPORT_SYMBOL(drm_dp_downstream_id);
> > * @m: pointer for debugfs file
> > * @dpcd: DisplayPort configuration data
> > * @port_cap: port capabilities
> > + * @edid: EDID
> > * @aux: DisplayPort AUX channel
> > *
> > */
> > void drm_dp_downstream_debug(struct seq_file *m,
> > const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > - const u8 port_cap[4], struct drm_dp_aux *aux)
> > + const u8 port_cap[4],
> > + const struct edid *edid,
> > + struct drm_dp_aux *aux)
> > {
> > bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > DP_DETAILED_CAP_INFO_AVAILABLE;
> > @@ -789,7 +802,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
> > seq_printf(m, "\t\tMax TMDS clock: %d kHz\n",
> > clk);
> > }
> >
> > - bpc = drm_dp_downstream_max_bpc(dpcd, port_cap);
> > + bpc = drm_dp_downstream_max_bpc(dpcd, port_cap, edid);
> >
> > if (bpc > 0)
> > seq_printf(m, "\t\tMax bpc: %d\n", bpc);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 53a0a3d9a22d..0bf31f9a8af5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -626,6 +626,7 @@ static void intel_dp_info(struct seq_file *m,
> > {
> > struct intel_encoder *intel_encoder =
> > intel_attached_encoder(intel_connector);
> > struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
> > + const struct drm_property_blob *edid = intel_connector-
> > >base.edid_blob_ptr;
> >
> > seq_printf(m, "\tDPCD rev: %x\n", intel_dp->dpcd[DP_DPCD_REV]);
> > seq_printf(m, "\taudio support: %s\n", yesno(intel_dp->has_audio));
> > @@ -633,7 +634,7 @@ static void intel_dp_info(struct seq_file *m,
> > intel_panel_info(m, &intel_connector->panel);
> >
> > drm_dp_downstream_debug(m, intel_dp->dpcd, intel_dp->downstream_ports,
> > - &intel_dp->aux);
> > + edid ? edid->data : NULL, &intel_dp->aux);
> > }
> >
> > static void intel_dp_mst_info(struct seq_file *m,
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 2c8e82d97a34..c73b3efd84e0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -6071,7 +6071,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
> >
> > intel_dp->dfp.max_bpc =
> > drm_dp_downstream_max_bpc(intel_dp->dpcd,
> > - intel_dp->downstream_ports);
> > + intel_dp->downstream_ports, edid);
> >
> > drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] DFP max bpc %d\n",
> > connector->base.base.id, connector->base.name,
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 4f946826dfce..6218de1294c1 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -1646,10 +1646,14 @@ bool drm_dp_downstream_is_tmds(const u8
> > dpcd[DP_RECEIVER_CAP_SIZE],
> > int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > const u8 port_cap[4]);
> > int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > - const u8 port_cap[4]);
> > + const u8 port_cap[4],
> > + const struct edid *edid);
> > int drm_dp_downstream_id(struct drm_dp_aux *aux, char id[6]);
> > -void drm_dp_downstream_debug(struct seq_file *m, const u8
> > dpcd[DP_RECEIVER_CAP_SIZE],
> > - const u8 port_cap[4], struct drm_dp_aux *aux);
> > +void drm_dp_downstream_debug(struct seq_file *m,
> > + const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > + const u8 port_cap[4],
> > + const struct edid *edid,
> > + struct drm_dp_aux *aux);
> > enum drm_mode_subconnector
> > drm_dp_subconnector_type(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > const u8 port_cap[4]);
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
