* RE: [PATCH v2] phy: Add DisplayPort configuration options
2019-12-23 13:41 [PATCH v2] phy: Add DisplayPort configuration options Yuti Amonkar
@ 2019-12-23 13:46 ` Yuti Suresh Amonkar
2019-12-23 17:18 ` Maxime Ripard
2020-01-02 11:28 ` Jyri Sarha
2 siblings, 0 replies; 6+ messages in thread
From: Yuti Suresh Amonkar @ 2019-12-23 13:46 UTC (permalink / raw)
To: linux-kernel, dri-devel, maxime, kishon
Cc: praneeth, tomi.valkeinen, jsarha, Milind Parab,
Swapnil Kashinath Jakhade
+Kishon
Thanks & Regards,
Yuti Amonkar
> -----Original Message-----
> From: Yuti Amonkar <yamonkar@cadence.com>
> Sent: Monday, December 23, 2019 19:11
> To: linux-kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
> maxime@cerno.tech
> Cc: praneeth@ti.com; tomi.valkeinen@ti.com; jsarha@ti.com; Milind Parab
> <mparab@cadence.com>; Swapnil Kashinath Jakhade
> <sjakhade@cadence.com>; Yuti Suresh Amonkar <yamonkar@cadence.com>
> Subject: [PATCH v2] phy: Add DisplayPort configuration options
>
> 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@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;
> +};
> +
> +#endif /* __PHY_DP_H_ */
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index
> 15032f14..ba0aab5 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -16,6 +16,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
>
> +#include <linux/phy/phy-dp.h>
> #include <linux/phy/phy-mipi-dphy.h>
>
> struct phy;
> @@ -46,9 +47,12 @@ enum phy_mode {
> *
> * @mipi_dphy: Configuration set applicable for phys supporting
> * the MIPI_DPHY phy mode.
> + * @dp: Configuration set applicable for phys supporting
> + * the DisplayPort protocol.
> */
> union phy_configure_opts {
> struct phy_configure_opts_mipi_dphy mipi_dphy;
> + struct phy_configure_opts_dp dp;
> };
>
> /**
> --
> 2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] phy: Add DisplayPort configuration options
2019-12-23 13:41 [PATCH v2] phy: Add DisplayPort configuration options Yuti Amonkar
2019-12-23 13:46 ` Yuti Suresh Amonkar
@ 2019-12-23 17:18 ` Maxime Ripard
2019-12-24 12:29 ` Yuti Suresh Amonkar
2020-01-02 11:28 ` Jyri Sarha
2 siblings, 1 reply; 6+ messages in thread
From: Maxime Ripard @ 2019-12-23 17:18 UTC (permalink / raw)
To: Yuti Amonkar
Cc: linux-kernel, dri-devel, praneeth, tomi.valkeinen, jsarha,
mparab, sjakhade
[-- Attachment #1: Type: text/plain, Size: 3847 bytes --]
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@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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v2] phy: Add DisplayPort configuration options
2019-12-23 17:18 ` Maxime Ripard
@ 2019-12-24 12:29 ` Yuti Suresh Amonkar
2020-01-02 10:36 ` Maxime Ripard
0 siblings, 1 reply; 6+ messages in thread
From: Yuti Suresh Amonkar @ 2019-12-24 12:29 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-kernel, dri-devel, praneeth, tomi.valkeinen, jsarha,
Milind Parab, Swapnil Kashinath Jakhade, kishon
Hi,
> -----Original Message-----
> From: Maxime Ripard <maxime@cerno.tech>
> Sent: Monday, December 23, 2019 22:49
> To: Yuti Suresh Amonkar <yamonkar@cadence.com>
> Cc: linux-kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
> praneeth@ti.com; tomi.valkeinen@ti.com; jsarha@ti.com; Milind Parab
> <mparab@cadence.com>; Swapnil Kashinath Jakhade
> <sjakhade@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@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.
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.
Thanks & Regards,
Yuti Amonkar
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] phy: Add DisplayPort configuration options
2019-12-24 12:29 ` Yuti Suresh Amonkar
@ 2020-01-02 10:36 ` Maxime Ripard
0 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2020-01-02 10:36 UTC (permalink / raw)
To: Yuti Suresh Amonkar
Cc: linux-kernel, dri-devel, praneeth, tomi.valkeinen, jsarha,
Milind Parab, Swapnil Kashinath Jakhade, kishon
[-- Attachment #1: Type: text/plain, Size: 5826 bytes --]
Hi,
On Tue, Dec 24, 2019 at 12:29:40PM +0000, Yuti Suresh Amonkar wrote:
> > -----Original Message-----
> > From: Maxime Ripard <maxime@cerno.tech>
> > Sent: Monday, December 23, 2019 22:49
> > To: Yuti Suresh Amonkar <yamonkar@cadence.com>
> > Cc: linux-kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
> > praneeth@ti.com; tomi.valkeinen@ti.com; jsarha@ti.com; Milind Parab
> > <mparab@cadence.com>; Swapnil Kashinath Jakhade
> > <sjakhade@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@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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] phy: Add DisplayPort configuration options
2019-12-23 13:41 [PATCH v2] phy: Add DisplayPort configuration options Yuti Amonkar
2019-12-23 13:46 ` Yuti Suresh Amonkar
2019-12-23 17:18 ` Maxime Ripard
@ 2020-01-02 11:28 ` Jyri Sarha
2 siblings, 0 replies; 6+ messages in thread
From: Jyri Sarha @ 2020-01-02 11:28 UTC (permalink / raw)
To: Yuti Amonkar, linux-kernel, dri-devel, maxime, ABRAHAM, KISHON VIJAY
Cc: praneeth, tomi.valkeinen, mparab, sjakhade
On 23/12/2019 15:41, 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@cadence.com>
I think we should also add phy mode for DisplayPort in this patch (or
series). e.g. something like this:
------------------------------------------------------------------
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index ba0aab59804f..79f25c8801f7 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -39,7 +39,8 @@ enum phy_mode {
PHY_MODE_PCIE,
PHY_MODE_ETHERNET,
PHY_MODE_MIPI_DPHY,
- PHY_MODE_SATA
+ PHY_MODE_SATA,
+ PHY_MODE_DISPLAYPORT
};
/**
------------------------------------------------------------------
Without it (and an associated phy_ops set_mode() call by the phy client)
the phy driver has no way to know how to interpret the "union
phy_configure_opts" in phy_ops configure() call.
Best regards,
Jyri
> ---
>
> 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;
> +};
> +
> +#endif /* __PHY_DP_H_ */
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 15032f14..ba0aab5 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -16,6 +16,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
>
> +#include <linux/phy/phy-dp.h>
> #include <linux/phy/phy-mipi-dphy.h>
>
> struct phy;
> @@ -46,9 +47,12 @@ enum phy_mode {
> *
> * @mipi_dphy: Configuration set applicable for phys supporting
> * the MIPI_DPHY phy mode.
> + * @dp: Configuration set applicable for phys supporting
> + * the DisplayPort protocol.
> */
> union phy_configure_opts {
> struct phy_configure_opts_mipi_dphy mipi_dphy;
> + struct phy_configure_opts_dp dp;
> };
>
> /**
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 6+ messages in thread