[PATCH v2] phy: Add DisplayPort configuration options

[Maxime Ripard]

Hi,

On Tue, Dec 24, 2019 at 12:29:40PM +0000, Yuti Suresh Amonkar wrote:
> > -----Original Message-----
> > From: Maxime Ripard <maxime at cerno.tech>
> > Sent: Monday, December 23, 2019 22:49
> > To: Yuti Suresh Amonkar <yamonkar at cadence.com>
> > Cc: linux-kernel at vger.kernel.org; dri-devel at lists.freedesktop.org;
> > praneeth at ti.com; tomi.valkeinen at ti.com; jsarha at ti.com; Milind Parab
> > <mparab at cadence.com>; Swapnil Kashinath Jakhade
> > <sjakhade at cadence.com>
> > Subject: Re: [PATCH v2] phy: Add DisplayPort configuration options
> >
> > EXTERNAL MAIL
> >
> >
> > Hi,
> >
> > Please note that I don't have access to the displayPort spec, so I'll only
> > comment on the content of that patch, not whether it's complete or not.
> >
> > On Mon, Dec 23, 2019 at 02:41:13PM +0100, Yuti Amonkar wrote:
> > > Allow DisplayPort PHYs to be configured through the generic functions
> > > through a custom structure added to the generic union.
> > > The configuration structure is used for reconfiguration of DisplayPort
> > > PHYs during link training operation.
> > >
> > > The parameters added here are the ones defined in the DisplayPort spec
> > > 1.4 which include link rate, number of lanes, voltage swing and
> > > pre-emphasis.
> > >
> > > Signed-off-by: Yuti Amonkar <yamonkar at cadence.com>
> > > ---
> > >
> > > This patch was a part of [1] series earlier but we think that it needs
> > > to have a separate attention of the reviewers. Also as both [1] & [2]
> > > are dependent on this patch, our sincere request to reviewers to have
> > > a faster review of this patch.
> > >
> > > [1]
> > >
> > > https://lkml.org/lkml/2019/12/11/455
> > >
> > > [2]
> > >
> > > https://patchwork.kernel.org/cover/11271191/
> > >
> > >  include/linux/phy/phy-dp.h | 95
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/phy/phy.h    |  4 ++
> > >  2 files changed, 99 insertions(+)
> > >  create mode 100644 include/linux/phy/phy-dp.h
> > >
> > > diff --git a/include/linux/phy/phy-dp.h b/include/linux/phy/phy-dp.h
> > > new file mode 100644 index 0000000..18cad23
> > > --- /dev/null
> > > +++ b/include/linux/phy/phy-dp.h
> > > @@ -0,0 +1,95 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright (C) 2019 Cadence Design Systems Inc.
> > > + */
> > > +
> > > +#ifndef __PHY_DP_H_
> > > +#define __PHY_DP_H_
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +/**
> > > + * struct phy_configure_opts_dp - DisplayPort PHY configuration set
> > > + *
> > > + * This structure is used to represent the configuration state of a
> > > + * DisplayPort phy.
> > > + */
> > > +struct phy_configure_opts_dp {
> > > +	/**
> > > +	 * @link_rate:
> > > +	 *
> > > +	 * Link Rate, in Mb/s, of the main link.
> > > +	 *
> > > +	 * Allowed values: 1620, 2160, 2430, 2700, 3240, 4320, 5400, 8100
> > Mb/s
> > > +	 */
> > > +	unsigned int link_rate;
> > > +
> > > +	/**
> > > +	 * @lanes:
> > > +	 *
> > > +	 * Number of active, consecutive, data lanes, starting from
> > > +	 * lane 0, used for the transmissions on main link.
> > > +	 *
> > > +	 * Allowed values: 1, 2, 4
> > > +	 */
> > > +	unsigned int lanes;
> > > +
> > > +	/**
> > > +	 * @voltage:
> > > +	 *
> > > +	 * Voltage swing levels, as specified by DisplayPort specification,
> > > +	 * to be used by particular lanes. One value per lane.
> > > +	 * voltage[0] is for lane 0, voltage[1] is for lane 1, etc.
> > > +	 *
> > > +	 * Maximum value: 3
> > > +	 */
> > > +	unsigned int voltage[4];
> > > +
> > > +	/**
> > > +	 * @pre:
> > > +	 *
> > > +	 * Pre-emphasis levels, as specified by DisplayPort specification, to be
> > > +	 * used by particular lanes. One value per lane.
> > > +	 *
> > > +	 * Maximum value: 3
> > > +	 */
> > > +	unsigned int pre[4];
> > > +
> > > +	/**
> > > +	 * @ssc:
> > > +	 *
> > > +	 * Flag indicating, whether or not to enable spread-spectrum
> > clocking.
> > > +	 *
> > > +	 */
> > > +	u8 ssc : 1;
> > > +
> > > +	/**
> > > +	 * @set_rate:
> > > +	 *
> > > +	 * Flag indicating, whether or not reconfigure link rate and SSC to
> > > +	 * requested values.
> > > +	 *
> > > +	 */
> > > +	u8 set_rate : 1;
> > > +
> > > +	/**
> > > +	 * @set_lanes:
> > > +	 *
> > > +	 * Flag indicating, whether or not reconfigure lane count to
> > > +	 * requested value.
> > > +	 *
> > > +	 */
> > > +	u8 set_lanes : 1;
> > > +
> > > +	/**
> > > +	 * @set_voltages:
> > > +	 *
> > > +	 * Flag indicating, whether or not reconfigure voltage swing
> > > +	 * and pre-emphasis to requested values. Only lanes specified
> > > +	 * by "lanes" parameter will be affected.
> > > +	 *
> > > +	 */
> > > +	u8 set_voltages : 1;
> >
> > I'm not quite sure what these flags are supposed to be doing, or what use-
> > cases they cover. The current API is using validate to make sure that we can
> > have a handshake between the caller and its PHY and must never apply the
> > configuration, and configure must always apply the configuration. These
> > flags look redundant.
> >
> > Maxime
>
> These flags are used to reconfigure the link during the link
> training procedure as described in DisplayPort spec. In this
> procedure , we may need to configure only subset of parameters (VS,
> Pre-emphasis, link rate and num of lanes) depending on different
> phases. e.g. At one stage, we may need to configure only Voltage
> swing and Pre-emphasis keeping number of lanes and link rate
> intact(set_voltages=true), while at other stage, we may need to
> configure all parameters. We use the flags to determine which
> parameter is updated during link training. Using separate flags for
> this provides control to upper layer.

Ok, it makes sense then :)

> I am not sure how to use validate to achieve this. As per my
> understanding validate is used to verify if set of parameters can be
> handled by PHY.

That's correct :)

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200102/c2542648/attachment-0001.sig>