linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices
@ 2018-12-07 13:55 Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 01/10] phy: Add MIPI D-PHY mode Maxime Ripard
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

Hi,

Here is a set of patches to allow the phy framework consumers to test and
apply runtime configurations.

This is needed to support more phy classes that require tuning based on
parameters depending on the current use case of the device, in addition to
the power state management already provided by the current functions.

A first test bed for that API are the MIPI D-PHY devices. There's a number
of solutions that have been used so far to support these phy, most of the
time being an ad-hoc driver in the consumer.

That approach has a big shortcoming though, which is that this is quite
difficult to deal with consumers integrated with multiple variants of phy,
of multiple consumers integrated with the same phy.

The latter case can be found in the Cadence DSI bridge, and the CSI
transceiver and receivers. All of them are integrated with the same phy, or
can be integrated with different phy, depending on the implementation.

I've looked at all the MIPI DSI drivers I could find, and gathered all the
parameters I could find. The interface should be complete, and most of the
drivers can be converted in the future. The current set converts two of
them: the above mentionned Cadence DSI driver so that the v4l2 drivers can
use them, and the Allwinner MIPI-DSI driver.

Let me know what you think,
Maxime

Changes from v2:
  - Rebased on next
  - Changed the interface to accomodate for the new submodes
  - Changed the timings units from nanoseconds to picoseconds
  - Added minimum and maximum boundaries to the documentation
  - Moved the clock enabling to phy_power_on in the Cadence DPHY driver
  - Exported the phy_configure and phy_validate symbols
  - Rework the phy pll divider computation in the cadence dphy driver

Changes from v1:
  - Rebased on top of 4.20-rc1
  - Removed the bus mode and timings parameters from the MIPI D-PHY
    parameters, since that shouldn't have any impact on the PHY itself.
  - Reworked the Cadence DSI and D-PHY drivers to take this into account.
  - Remove the mode parameter from phy_configure
  - Added phy_configure and phy_validate stubs
  - Return -EOPNOTSUPP in phy_configure and phy_validate when the operation
    is not implemented

Maxime Ripard (10):
  phy: Add MIPI D-PHY mode
  phy: Add configuration interface
  phy: Add MIPI D-PHY configuration options
  phy: dphy: Add configuration helpers
  sun6i: dsi: Convert to generic phy handling
  phy: Move Allwinner A31 D-PHY driver to drivers/phy/
  drm/bridge: cdns: Separate DSI and D-PHY configuration
  dt-bindings: phy: Move the Cadence D-PHY bindings
  phy: Add Cadence D-PHY support
  drm/bridge: cdns: Convert to phy framework

 Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt |  21 +-
 Documentation/devicetree/bindings/phy/cdns,dphy.txt           |  20 +-
 drivers/gpu/drm/bridge/cdns-dsi.c                             | 535 +------
 drivers/gpu/drm/sun4i/Kconfig                                 |   3 +-
 drivers/gpu/drm/sun4i/Makefile                                |   5 +-
 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c                       | 292 +----
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c                        |  31 +-
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h                        |  17 +-
 drivers/phy/Kconfig                                           |   8 +-
 drivers/phy/Makefile                                          |   1 +-
 drivers/phy/allwinner/Kconfig                                 |  12 +-
 drivers/phy/allwinner/Makefile                                |   1 +-
 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c                   | 318 ++++-
 drivers/phy/cadence/Kconfig                                   |  13 +-
 drivers/phy/cadence/Makefile                                  |   1 +-
 drivers/phy/cadence/cdns-dphy.c                               | 389 +++++-
 drivers/phy/phy-core-mipi-dphy.c                              | 166 ++-
 drivers/phy/phy-core.c                                        |  64 +-
 include/linux/phy/phy-mipi-dphy.h                             | 285 ++++-
 include/linux/phy/phy.h                                       |  65 +-
 20 files changed, 1468 insertions(+), 779 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt
 delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
 create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
 create mode 100644 drivers/phy/cadence/cdns-dphy.c
 create mode 100644 drivers/phy/phy-core-mipi-dphy.c
 create mode 100644 include/linux/phy/phy-mipi-dphy.h

base-commit: 74c4a24df7cac1f9213a811d79558ecde23be9a2
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 01/10] phy: Add MIPI D-PHY mode
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 02/10] phy: Add configuration interface Maxime Ripard
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

MIPI D-PHY is a MIPI standard meant mostly for display and cameras in
embedded systems. Add a mode for it.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 include/linux/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 79da05a3e28d..453f21834685 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -39,6 +39,7 @@ enum phy_mode {
 	PHY_MODE_UFS_HS_B,
 	PHY_MODE_PCIE,
 	PHY_MODE_ETHERNET,
+	PHY_MODE_MIPI_DPHY,
 };
 
 /**
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 02/10] phy: Add configuration interface
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 01/10] phy: Add MIPI D-PHY mode Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-13 20:42   ` sakari.ailus
  2018-12-07 13:55 ` [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options Maxime Ripard
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

The phy framework is only allowing to configure the power state of the PHY
using the init and power_on hooks, and their power_off and exit
counterparts.

While it works for most, simple, PHYs supported so far, some more advanced
PHYs need some configuration depending on runtime parameters. These PHYs
have been supported by a number of means already, often by using ad-hoc
drivers in their consumer drivers.

That doesn't work too well however, when a consumer device needs to deal
with multiple PHYs, or when multiple consumers need to deal with the same
PHY (a DSI driver and a CSI driver for example).

So we'll add a new interface, through two funtions, phy_validate and
phy_configure. The first one will allow to check that a current
configuration, for a given mode, is applicable. It will also allow the PHY
driver to tune the settings given as parameters as it sees fit.

phy_configure will actually apply that configuration in the phy itself.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/phy/phy-core.c  | 64 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/phy/phy.h | 58 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 122 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index df3d4ba516ab..19b05e824ee4 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -408,6 +408,70 @@ int phy_calibrate(struct phy *phy)
 EXPORT_SYMBOL_GPL(phy_calibrate);
 
 /**
+ * phy_configure() - Changes the phy parameters
+ * @phy: the phy returned by phy_get()
+ * @opts: New configuration to apply
+ *
+ * Used to change the PHY parameters. phy_init() must have been called
+ * on the phy. The configuration will be applied on the current phy
+ * mode, that can be changed using phy_set_mode().
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
+int phy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+	int ret;
+
+	if (!phy)
+		return -EINVAL;
+
+	if (!phy->ops->configure)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&phy->mutex);
+	ret = phy->ops->configure(phy, opts);
+	mutex_unlock(&phy->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_configure);
+
+/**
+ * phy_validate() - Checks the phy parameters
+ * @phy: the phy returned by phy_get()
+ * @mode: phy_mode the configuration is applicable to.
+ * @submode: PHY submode the configuration is applicable to.
+ * @opts: Configuration to check
+ *
+ * Used to check that the current set of parameters can be handled by
+ * the phy. Implementations are free to tune the parameters passed as
+ * arguments if needed by some implementation detail or
+ * constraints. It will not change any actual configuration of the
+ * PHY, so calling it as many times as deemed fit will have no side
+ * effect.
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
+int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
+		 union phy_configure_opts *opts)
+{
+	int ret;
+
+	if (!phy)
+		return -EINVAL;
+
+	if (!phy->ops->validate)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&phy->mutex);
+	ret = phy->ops->validate(phy, mode, submode, opts);
+	mutex_unlock(&phy->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_validate);
+
+/**
  * _of_phy_get() - lookup and obtain a reference to a phy by phandle
  * @np: device_node for which to get the phy
  * @index: the index of the phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 453f21834685..04476c026b5a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -43,6 +43,12 @@ enum phy_mode {
 };
 
 /**
+ * union phy_configure_opts - Opaque generic phy configuration
+ */
+union phy_configure_opts {
+};
+
+/**
  * struct phy_ops - set of function pointers for performing phy operations
  * @init: operation to be performed for initializing phy
  * @exit: operation to be performed while exiting
@@ -59,6 +65,37 @@ struct phy_ops {
 	int	(*power_on)(struct phy *phy);
 	int	(*power_off)(struct phy *phy);
 	int	(*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
+
+	/**
+	 * @configure:
+	 *
+	 * Optional.
+	 *
+	 * Used to change the PHY parameters. phy_init() must have
+	 * been called on the phy.
+	 *
+	 * Returns: 0 if successful, an negative error code otherwise
+	 */
+	int	(*configure)(struct phy *phy, union phy_configure_opts *opts);
+
+	/**
+	 * @validate:
+	 *
+	 * Optional.
+	 *
+	 * Used to check that the current set of parameters can be
+	 * handled by the phy. Implementations are free to tune the
+	 * parameters passed as arguments if needed by some
+	 * implementation detail or constraints. It must not change
+	 * any actual configuration of the PHY, so calling it as many
+	 * times as deemed fit by the consumer must have no side
+	 * effect.
+	 *
+	 * Returns: 0 if the configuration can be applied, an negative
+	 * error code otherwise
+	 */
+	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
+			    union phy_configure_opts *opts);
 	int	(*reset)(struct phy *phy);
 	int	(*calibrate)(struct phy *phy);
 	struct module *owner;
@@ -165,6 +202,9 @@ int phy_power_off(struct phy *phy);
 int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
 #define phy_set_mode(phy, mode) \
 	phy_set_mode_ext(phy, mode, 0)
+int phy_configure(struct phy *phy, union phy_configure_opts *opts);
+int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
+		 union phy_configure_opts *opts);
 
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
@@ -309,6 +349,24 @@ static inline int phy_calibrate(struct phy *phy)
 	return -ENOSYS;
 }
 
+static inline int phy_configure(struct phy *phy,
+				union phy_configure_opts *opts)
+{
+	if (!phy)
+		return 0;
+
+	return -ENOSYS;
+}
+
+static inline int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
+			       union phy_configure_opts *opts)
+{
+	if (!phy)
+		return 0;
+
+	return -ENOSYS;
+}
+
 static inline int phy_get_bus_width(struct phy *phy)
 {
 	return -ENOSYS;
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 01/10] phy: Add MIPI D-PHY mode Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 02/10] phy: Add configuration interface Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-13 20:49   ` sakari.ailus
  2018-12-13 20:54   ` sakari.ailus
  2018-12-07 13:55 ` [PATCH v3 04/10] phy: dphy: Add configuration helpers Maxime Ripard
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

Now that we have some infrastructure for it, allow the MIPI D-PHY phy's to
be configured through the generic functions through a custom structure
added to the generic union.

The parameters added here are the ones defined in the MIPI D-PHY spec, plus
the number of lanes in use. The current set of parameters should cover all
the potential users.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 include/linux/phy/phy-mipi-dphy.h | 279 +++++++++++++++++++++++++++++++-
 include/linux/phy/phy.h           |   6 +-
 2 files changed, 285 insertions(+)
 create mode 100644 include/linux/phy/phy-mipi-dphy.h

diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h
new file mode 100644
index 000000000000..29bf94db88ad
--- /dev/null
+++ b/include/linux/phy/phy-mipi-dphy.h
@@ -0,0 +1,279 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Cadence Design Systems Inc.
+ */
+
+#ifndef __PHY_MIPI_DPHY_H_
+#define __PHY_MIPI_DPHY_H_
+
+#include <video/videomode.h>
+
+/**
+ * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
+ *
+ * This structure is used to represent the configuration state of a
+ * MIPI D-PHY phy.
+ */
+struct phy_configure_opts_mipi_dphy {
+	/**
+	 * @clk_miss:
+	 *
+	 * Timeout, in picoseconds, for receiver to detect absence of
+	 * Clock transitions and disable the Clock Lane HS-RX.
+	 *
+	 * Maximum value: 60000 ps
+	 */
+	unsigned int		clk_miss;
+
+	/**
+	 * @clk_post:
+	 *
+	 * Time, in picoseconds, that the transmitter continues to
+	 * send HS clock after the last associated Data Lane has
+	 * transitioned to LP Mode. Interval is defined as the period
+	 * from the end of @hs_trail to the beginning of @clk_trail.
+	 *
+	 * Minimum value: 60000 ps + 52 * @hs_clk_rate period in ps
+	 */
+	unsigned int		clk_post;
+
+	/**
+	 * @clk_pre:
+	 *
+	 * Time, in UI, that the HS clock shall be driven by
+	 * the transmitter prior to any associated Data Lane beginning
+	 * the transition from LP to HS mode.
+	 *
+	 * Minimum value: 8 UI
+	 */
+	unsigned int		clk_pre;
+
+	/**
+	 * @clk_prepare:
+	 *
+	 * Time, in picoseconds, that the transmitter drives the Clock
+	 * Lane LP-00 Line state immediately before the HS-0 Line
+	 * state starting the HS transmission.
+	 *
+	 * Minimum value: 38000 ps
+	 * Maximum value: 95000 ps
+	 */
+	unsigned int		clk_prepare;
+
+	/**
+	 * @clk_settle:
+	 *
+	 * Time interval, in picoseconds, during which the HS receiver
+	 * should ignore any Clock Lane HS transitions, starting from
+	 * the beginning of @clk_prepare.
+	 *
+	 * Minimum value: 95000 ps
+	 * Maximum value: 300000 ps
+	 */
+	unsigned int		clk_settle;
+
+	/**
+	 * @clk_term_en:
+	 *
+	 * Time, in picoseconds, for the Clock Lane receiver to enable
+	 * the HS line termination.
+	 *
+	 * Maximum value: 38000 ps
+	 */
+	unsigned int		clk_term_en;
+
+	/**
+	 * @clk_trail:
+	 *
+	 * Time, in picoseconds, that the transmitter drives the HS-0
+	 * state after the last payload clock bit of a HS transmission
+	 * burst.
+	 *
+	 * Minimum value: 60000 ps
+	 */
+	unsigned int		clk_trail;
+
+	/**
+	 * @clk_zero:
+	 *
+	 * Time, in picoseconds, that the transmitter drives the HS-0
+	 * state prior to starting the Clock.
+	 */
+	unsigned int		clk_zero;
+
+	/**
+	 * @d_term_en:
+	 *
+	 * Time, in picoseconds, for the Data Lane receiver to enable
+	 * the HS line termination.
+	 *
+	 * Maximum value: 35000 ps + 4 * @hs_clk_rate period in ps
+	 */
+	unsigned int		d_term_en;
+
+	/**
+	 * @eot:
+	 *
+	 * Transmitted time interval, in picoseconds, from the start
+	 * of @hs_trail or @clk_trail, to the start of the LP- 11
+	 * state following a HS burst.
+	 *
+	 * Maximum value: 105000 ps + 12 * @hs_clk_rate period in ps
+	 */
+	unsigned int		eot;
+
+	/**
+	 * @hs_exit:
+	 *
+	 * Time, in picoseconds, that the transmitter drives LP-11
+	 * following a HS burst.
+	 *
+	 * Minimum value: 100000 ps
+	 */
+	unsigned int		hs_exit;
+
+	/**
+	 * @hs_prepare:
+	 *
+	 * Time, in picoseconds, that the transmitter drives the Data
+	 * Lane LP-00 Line state immediately before the HS-0 Line
+	 * state starting the HS transmission.
+	 *
+	 * Minimum value: 40000 ps + 4 * @hs_clk_rate period in ps
+	 * Maximum value: 85000 ps + 6 * @hs_clk_rate period in ps
+	 */
+	unsigned int		hs_prepare;
+
+	/**
+	 * @hs_settle:
+	 *
+	 * Time interval, in picoseconds, during which the HS receiver
+	 * shall ignore any Data Lane HS transitions, starting from
+	 * the beginning of @hs_prepare.
+	 *
+	 * Minimum value: 85000 ps + 6 * @hs_clk_rate period in ps
+	 * Maximum value: 145000 ps + 10 * @hs_clk_rate period in ps
+	 */
+	unsigned int		hs_settle;
+
+	/**
+	 * @hs_skip:
+	 *
+	 * Time interval, in picoseconds, during which the HS-RX
+	 * should ignore any transitions on the Data Lane, following a
+	 * HS burst. The end point of the interval is defined as the
+	 * beginning of the LP-11 state following the HS burst.
+	 *
+	 * Minimum value: 40000 ps
+	 * Maximum value: 55000 ps + 4 * @hs_clk_rate period in ps
+	 */
+	unsigned int		hs_skip;
+
+	/**
+	 * @hs_trail:
+	 *
+	 * Time, in picoseconds, that the transmitter drives the
+	 * flipped differential state after last payload data bit of a
+	 * HS transmission burst
+	 *
+	 * Minimum value: max(8 * @hs_clk_rate period in ps,
+	 *		      60000 ps + 4 * @hs_clk_rate period in ps)
+	 */
+	unsigned int		hs_trail;
+
+	/**
+	 * @hs_zero:
+	 *
+	 * Time, in picoseconds, that the transmitter drives the HS-0
+	 * state prior to transmitting the Sync sequence.
+	 */
+	unsigned int		hs_zero;
+
+	/**
+	 * @init:
+	 *
+	 * Time, in picoseconds for the initialization period to
+	 * complete.
+	 *
+	 * Minimum value: 100000000 ps
+	 */
+	unsigned int		init;
+
+	/**
+	 * @lpx:
+	 *
+	 * Transmitted length, in picoseconds, of any Low-Power state
+	 * period.
+	 *
+	 * Minimum value: 50000 ps
+	 */
+	unsigned int		lpx;
+
+	/**
+	 * @ta_get:
+	 *
+	 * Time, in picoseconds, that the new transmitter drives the
+	 * Bridge state (LP-00) after accepting control during a Link
+	 * Turnaround.
+	 *
+	 * Value: 5 * @lpx
+	 */
+	unsigned int		ta_get;
+
+	/**
+	 * @ta_go:
+	 *
+	 * Time, in picoseconds, that the transmitter drives the
+	 * Bridge state (LP-00) before releasing control during a Link
+	 * Turnaround.
+	 *
+	 * Value: 4 * @lpx
+	 */
+	unsigned int		ta_go;
+
+	/**
+	 * @ta_sure:
+	 *
+	 * Time, in picoseconds, that the new transmitter waits after
+	 * the LP-10 state before transmitting the Bridge state
+	 * (LP-00) during a Link Turnaround.
+	 *
+	 * Minimum value: @lpx
+	 * Maximum value: 2 * @lpx
+	 */
+	unsigned int		ta_sure;
+
+	/**
+	 * @wakeup:
+	 *
+	 * Time, in picoseconds, that a transmitter drives a Mark-1
+	 * state prior to a Stop state in order to initiate an exit
+	 * from ULPS.
+	 *
+	 * Minimum value: 1000000000 ps
+	 */
+	unsigned int		wakeup;
+
+	/**
+	 * @hs_clk_rate:
+	 *
+	 * Clock rate, in Hertz, of the high-speed clock.
+	 */
+	unsigned long		hs_clk_rate;
+
+	/**
+	 * @lp_clk_rate:
+	 *
+	 * Clock rate, in Hertz, of the low-power clock.
+	 */
+	unsigned long		lp_clk_rate;
+
+	/**
+	 * @lanes:
+	 *
+	 * Number of active data lanes used for the transmissions.
+	 */
+	unsigned char		lanes;
+};
+
+#endif /* __PHY_MIPI_DPHY_H_ */
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 04476c026b5a..1fdefadf150a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -20,6 +20,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
+#include <linux/phy/phy-mipi-dphy.h>
+
 struct phy;
 
 enum phy_mode {
@@ -44,8 +46,12 @@ enum phy_mode {
 
 /**
  * union phy_configure_opts - Opaque generic phy configuration
+ *
+ * @mipi_dphy:	Configuration set applicable for phys supporting
+ *		the MIPI_DPHY phy mode.
  */
 union phy_configure_opts {
+	struct phy_configure_opts_mipi_dphy	mipi_dphy;
 };
 
 /**
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 04/10] phy: dphy: Add configuration helpers
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
                   ` (2 preceding siblings ...)
  2018-12-07 13:55 ` [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 05/10] sun6i: dsi: Convert to generic phy handling Maxime Ripard
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

The MIPI D-PHY spec defines default values and boundaries for most of the
parameters it defines. Introduce helpers to help drivers get meaningful
values based on their current parameters, and validate the boundaries of
these parameters if needed.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/phy/Kconfig               |   8 +-
 drivers/phy/Makefile              |   1 +-
 drivers/phy/phy-core-mipi-dphy.c  | 166 +++++++++++++++++++++++++++++++-
 include/linux/phy/phy-mipi-dphy.h |   6 +-
 4 files changed, 181 insertions(+)
 create mode 100644 drivers/phy/phy-core-mipi-dphy.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 60f949e2a684..c87a7d49eaab 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,6 +15,14 @@ config GENERIC_PHY
 	  phy users can obtain reference to the PHY. All the users of this
 	  framework should select this config.
 
+config GENERIC_PHY_MIPI_DPHY
+	bool
+	help
+	  Generic MIPI D-PHY support.
+
+	  Provides a number of helpers a core functions for MIPI D-PHY
+	  drivers to us.
+
 config PHY_LPC18XX_USB_OTG
 	tristate "NXP LPC18xx/43xx SoC USB OTG PHY driver"
 	depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 0301e25d07c1..baec59cebbab 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -4,6 +4,7 @@
 #
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
+obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY)	+= phy-core-mipi-dphy.o
 obj-$(CONFIG_PHY_LPC18XX_USB_OTG)	+= phy-lpc18xx-usb-otg.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
 obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
diff --git a/drivers/phy/phy-core-mipi-dphy.c b/drivers/phy/phy-core-mipi-dphy.c
new file mode 100644
index 000000000000..465fa1b91a5f
--- /dev/null
+++ b/drivers/phy/phy-core-mipi-dphy.c
@@ -0,0 +1,166 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2013 NVIDIA Corporation
+ * Copyright (C) 2018 Cadence Design Systems Inc.
+ */
+
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/time64.h>
+
+#include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.h>
+
+#define PSEC_PER_SEC	1000000000000LL
+
+/*
+ * Minimum D-PHY timings based on MIPI D-PHY specification. Derived
+ * from the valid ranges specified in Section 6.9, Table 14, Page 41
+ * of the D-PHY specification (v2.1).
+ */
+int phy_mipi_dphy_get_default_config(unsigned long pixel_clock,
+				     unsigned int bpp,
+				     unsigned int lanes,
+				     struct phy_configure_opts_mipi_dphy *cfg)
+{
+	unsigned long long hs_clk_rate;
+	unsigned long long ui;
+
+	if (!cfg)
+		return -EINVAL;
+
+	hs_clk_rate = pixel_clock * bpp;
+	do_div(hs_clk_rate, lanes);
+
+	ui = ALIGN(PSEC_PER_SEC, hs_clk_rate);
+	do_div(ui, hs_clk_rate);
+
+	cfg->clk_miss = 0;
+	cfg->clk_post = 60000 + 52 * ui;
+	cfg->clk_pre = 8000;
+	cfg->clk_prepare = 38000;
+	cfg->clk_settle = 95000;
+	cfg->clk_term_en = 0;
+	cfg->clk_trail = 60000;
+	cfg->clk_zero = 262000;
+	cfg->d_term_en = 0;
+	cfg->eot = 0;
+	cfg->hs_exit = 100000;
+	cfg->hs_prepare = 40000 + 4 * ui;
+	cfg->hs_zero = 105000 + 6 * ui;
+	cfg->hs_settle = 85000 + 6 * ui;
+	cfg->hs_skip = 40000;
+
+	/*
+	 * The MIPI D-PHY specification (Section 6.9, v1.2, Table 14, Page 40)
+	 * contains this formula as:
+	 *
+	 *     T_HS-TRAIL = max(n * 8 * ui, 60 + n * 4 * ui)
+	 *
+	 * where n = 1 for forward-direction HS mode and n = 4 for reverse-
+	 * direction HS mode. There's only one setting and this function does
+	 * not parameterize on anything other that ui, so this code will
+	 * assumes that reverse-direction HS mode is supported and uses n = 4.
+	 */
+	cfg->hs_trail = max(4 * 8 * ui, 60000 + 4 * 4 * ui);
+
+	cfg->init = 100000000;
+	cfg->lpx = 60000;
+	cfg->ta_get = 5 * cfg->lpx;
+	cfg->ta_go = 4 * cfg->lpx;
+	cfg->ta_sure = 2 * cfg->lpx;
+	cfg->wakeup = 1000000000;
+
+	cfg->hs_clk_rate = hs_clk_rate;
+	cfg->lanes = lanes;
+
+	return 0;
+}
+EXPORT_SYMBOL(phy_mipi_dphy_get_default_config);
+
+/*
+ * Validate D-PHY configuration according to MIPI D-PHY specification
+ * (v1.2, Section Section 6.9 "Global Operation Timing Parameters").
+ */
+int phy_mipi_dphy_config_validate(struct phy_configure_opts_mipi_dphy *cfg)
+{
+	unsigned long long ui;
+
+	if (!cfg)
+		return -EINVAL;
+
+	ui = ALIGN(PSEC_PER_SEC, cfg->hs_clk_rate);
+	do_div(ui, cfg->hs_clk_rate);
+
+	if (cfg->clk_miss > 60000)
+		return -EINVAL;
+
+	if (cfg->clk_post < (60000 + 52 * ui))
+		return -EINVAL;
+
+	if (cfg->clk_pre < 8000)
+		return -EINVAL;
+
+	if (cfg->clk_prepare < 38000 || cfg->clk_prepare > 95000)
+		return -EINVAL;
+
+	if (cfg->clk_settle < 95000 || cfg->clk_settle > 300000)
+		return -EINVAL;
+
+	if (cfg->clk_term_en > 38000)
+		return -EINVAL;
+
+	if (cfg->clk_trail < 60000)
+		return -EINVAL;
+
+	if ((cfg->clk_prepare + cfg->clk_zero) < 300000)
+		return -EINVAL;
+
+	if (cfg->d_term_en > (35000 + 4 * ui))
+		return -EINVAL;
+
+	if (cfg->eot > (105000 + 12 * ui))
+		return -EINVAL;
+
+	if (cfg->hs_exit < 100000)
+		return -EINVAL;
+
+	if (cfg->hs_prepare < (40000 + 4 * ui) ||
+	    cfg->hs_prepare > (85000 + 6 * ui))
+		return -EINVAL;
+
+	if ((cfg->hs_prepare + cfg->hs_zero) < (145000 + 10 * ui))
+		return -EINVAL;
+
+	if ((cfg->hs_settle < (85000 + 6 * ui)) ||
+	    (cfg->hs_settle > (145000 + 10 * ui)))
+		return -EINVAL;
+
+	if (cfg->hs_skip < 40000 || cfg->hs_skip > (55000 + 4 * ui))
+		return -EINVAL;
+
+	if (cfg->hs_trail < max(8 * ui, 60000 + 4 * ui))
+		return -EINVAL;
+
+	if (cfg->init < 100000000)
+		return -EINVAL;
+
+	if (cfg->lpx < 50000)
+		return -EINVAL;
+
+	if (cfg->ta_get != (5 * cfg->lpx))
+		return -EINVAL;
+
+	if (cfg->ta_go != (4 * cfg->lpx))
+		return -EINVAL;
+
+	if (cfg->ta_sure < cfg->lpx || cfg->ta_sure > (2 * cfg->lpx))
+		return -EINVAL;
+
+	if (cfg->wakeup < 1000000000)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL(phy_mipi_dphy_config_validate);
diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h
index 29bf94db88ad..c08aacc0ac35 100644
--- a/include/linux/phy/phy-mipi-dphy.h
+++ b/include/linux/phy/phy-mipi-dphy.h
@@ -276,4 +276,10 @@ struct phy_configure_opts_mipi_dphy {
 	unsigned char		lanes;
 };
 
+int phy_mipi_dphy_get_default_config(unsigned long pixel_clock,
+				     unsigned int bpp,
+				     unsigned int lanes,
+				     struct phy_configure_opts_mipi_dphy *cfg);
+int phy_mipi_dphy_config_validate(struct phy_configure_opts_mipi_dphy *cfg);
+
 #endif /* __PHY_MIPI_DPHY_H_ */
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 05/10] sun6i: dsi: Convert to generic phy handling
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
                   ` (3 preceding siblings ...)
  2018-12-07 13:55 ` [PATCH v3 04/10] phy: dphy: Add configuration helpers Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 06/10] phy: Move Allwinner A31 D-PHY driver to drivers/phy/ Maxime Ripard
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

Now that we have everything in place in the PHY framework to deal in a
generic way with MIPI D-PHY phys, let's convert our PHY driver and its
associated DSI driver to that new API.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/Kconfig           |  11 +-
 drivers/gpu/drm/sun4i/Makefile          |   6 +-
 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 164 ++++++++++++++-----------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c  |  31 ++---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h  |  17 +---
 5 files changed, 126 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index c2c042287c19..2b8db82c4bab 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -45,10 +45,19 @@ config DRM_SUN6I_DSI
 	default MACH_SUN8I
 	select CRC_CCITT
 	select DRM_MIPI_DSI
+	select DRM_SUN6I_DPHY
 	help
 	  Choose this option if you want have an Allwinner SoC with
 	  MIPI-DSI support. If M is selected the module will be called
-	  sun6i-dsi
+	  sun6i_mipi_dsi.
+
+config DRM_SUN6I_DPHY
+	tristate "Allwinner A31 MIPI D-PHY Support"
+	select GENERIC_PHY_MIPI_DPHY
+	help
+	  Choose this option if you have an Allwinner SoC with
+	  MIPI-DSI support. If M is selected, the module will be
+	  called sun6i_mipi_dphy.
 
 config DRM_SUN8I_DW_HDMI
 	tristate "Support for Allwinner version of DesignWare HDMI"
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 0eb38ac8e86e..1e2320d824b5 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -24,9 +24,6 @@ sun4i-tcon-y			+= sun4i_lvds.o
 sun4i-tcon-y			+= sun4i_tcon.o
 sun4i-tcon-y			+= sun4i_rgb.o
 
-sun6i-dsi-y			+= sun6i_mipi_dphy.o
-sun6i-dsi-y			+= sun6i_mipi_dsi.o
-
 obj-$(CONFIG_DRM_SUN4I)		+= sun4i-drm.o
 obj-$(CONFIG_DRM_SUN4I)		+= sun4i-tcon.o
 obj-$(CONFIG_DRM_SUN4I)		+= sun4i_tv.o
@@ -37,7 +34,8 @@ ifdef CONFIG_DRM_SUN4I_BACKEND
 obj-$(CONFIG_DRM_SUN4I)		+= sun4i-frontend.o
 endif
 obj-$(CONFIG_DRM_SUN4I_HDMI)	+= sun4i-drm-hdmi.o
-obj-$(CONFIG_DRM_SUN6I_DSI)	+= sun6i-dsi.o
+obj-$(CONFIG_DRM_SUN6I_DPHY)	+= sun6i_mipi_dphy.o
+obj-$(CONFIG_DRM_SUN6I_DSI)	+= sun6i_mipi_dsi.o
 obj-$(CONFIG_DRM_SUN8I_DW_HDMI)	+= sun8i-drm-hdmi.o
 obj-$(CONFIG_DRM_SUN8I_MIXER)	+= sun8i-mixer.o
 obj-$(CONFIG_DRM_SUN8I_TCON_TOP) += sun8i_tcon_top.o
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
index e4d19431fa0e..79c8af5c7c1d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
@@ -8,11 +8,14 @@
 
 #include <linux/bitops.h>
 #include <linux/clk.h>
+#include <linux/module.h>
 #include <linux/of_address.h>
+#include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
-#include "sun6i_mipi_dsi.h"
+#include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.h>
 
 #define SUN6I_DPHY_GCTL_REG		0x00
 #define SUN6I_DPHY_GCTL_LANE_NUM(n)		((((n) - 1) & 3) << 4)
@@ -81,12 +84,46 @@
 
 #define SUN6I_DPHY_DBG5_REG		0xf4
 
-int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int lanes)
+struct sun6i_dphy {
+	struct clk				*bus_clk;
+	struct clk				*mod_clk;
+	struct regmap				*regs;
+	struct reset_control			*reset;
+
+	struct phy				*phy;
+	struct phy_configure_opts_mipi_dphy	config;
+};
+
+static int sun6i_dphy_init(struct phy *phy)
 {
+	struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+
 	reset_control_deassert(dphy->reset);
 	clk_prepare_enable(dphy->mod_clk);
 	clk_set_rate_exclusive(dphy->mod_clk, 150000000);
 
+	return 0;
+}
+
+static int sun6i_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+	struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+	int ret;
+
+	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
+	if (ret)
+		return ret;
+
+	memcpy(&dphy->config, opts, sizeof(dphy->config));
+
+	return 0;
+}
+
+static int sun6i_dphy_power_on(struct phy *phy)
+{
+	struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+	u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0);
+
 	regmap_write(dphy->regs, SUN6I_DPHY_TX_CTL_REG,
 		     SUN6I_DPHY_TX_CTL_HS_TX_CLK_CONT);
 
@@ -111,16 +148,9 @@ int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int lanes)
 		     SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(3));
 
 	regmap_write(dphy->regs, SUN6I_DPHY_GCTL_REG,
-		     SUN6I_DPHY_GCTL_LANE_NUM(lanes) |
+		     SUN6I_DPHY_GCTL_LANE_NUM(dphy->config.lanes) |
 		     SUN6I_DPHY_GCTL_EN);
 
-	return 0;
-}
-
-int sun6i_dphy_power_on(struct sun6i_dphy *dphy, unsigned int lanes)
-{
-	u8 lanes_mask = GENMASK(lanes - 1, 0);
-
 	regmap_write(dphy->regs, SUN6I_DPHY_ANA0_REG,
 		     SUN6I_DPHY_ANA0_REG_PWS |
 		     SUN6I_DPHY_ANA0_REG_DMPC |
@@ -181,16 +211,20 @@ int sun6i_dphy_power_on(struct sun6i_dphy *dphy, unsigned int lanes)
 	return 0;
 }
 
-int sun6i_dphy_power_off(struct sun6i_dphy *dphy)
+static int sun6i_dphy_power_off(struct phy *phy)
 {
+	struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+
 	regmap_update_bits(dphy->regs, SUN6I_DPHY_ANA1_REG,
 			   SUN6I_DPHY_ANA1_REG_VTTMODE, 0);
 
 	return 0;
 }
 
-int sun6i_dphy_exit(struct sun6i_dphy *dphy)
+static int sun6i_dphy_exit(struct phy *phy)
 {
+	struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+
 	clk_rate_exclusive_put(dphy->mod_clk);
 	clk_disable_unprepare(dphy->mod_clk);
 	reset_control_assert(dphy->reset);
@@ -198,6 +232,15 @@ int sun6i_dphy_exit(struct sun6i_dphy *dphy)
 	return 0;
 }
 
+
+static struct phy_ops sun6i_dphy_ops = {
+	.configure	= sun6i_dphy_configure,
+	.power_on	= sun6i_dphy_power_on,
+	.power_off	= sun6i_dphy_power_off,
+	.init		= sun6i_dphy_init,
+	.exit		= sun6i_dphy_exit,
+};
+
 static struct regmap_config sun6i_dphy_regmap_config = {
 	.reg_bits	= 32,
 	.val_bits	= 32,
@@ -206,87 +249,70 @@ static struct regmap_config sun6i_dphy_regmap_config = {
 	.name		= "mipi-dphy",
 };
 
-static const struct of_device_id sun6i_dphy_of_table[] = {
-	{ .compatible = "allwinner,sun6i-a31-mipi-dphy" },
-	{ }
-};
-
-int sun6i_dphy_probe(struct sun6i_dsi *dsi, struct device_node *node)
+static int sun6i_dphy_probe(struct platform_device *pdev)
 {
+	struct phy_provider *phy_provider;
 	struct sun6i_dphy *dphy;
-	struct resource res;
+	struct resource *res;
 	void __iomem *regs;
-	int ret;
-
-	if (!of_match_node(sun6i_dphy_of_table, node)) {
-		dev_err(dsi->dev, "Incompatible D-PHY\n");
-		return -EINVAL;
-	}
 
-	dphy = devm_kzalloc(dsi->dev, sizeof(*dphy), GFP_KERNEL);
+	dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
 	if (!dphy)
 		return -ENOMEM;
 
-	ret = of_address_to_resource(node, 0, &res);
-	if (ret) {
-		dev_err(dsi->dev, "phy: Couldn't get our resources\n");
-		return ret;
-	}
-
-	regs = devm_ioremap_resource(dsi->dev, &res);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(regs)) {
-		dev_err(dsi->dev, "Couldn't map the DPHY encoder registers\n");
+		dev_err(&pdev->dev, "Couldn't map the DPHY encoder registers\n");
 		return PTR_ERR(regs);
 	}
 
-	dphy->regs = devm_regmap_init_mmio(dsi->dev, regs,
-					   &sun6i_dphy_regmap_config);
+	dphy->regs = devm_regmap_init_mmio_clk(&pdev->dev, "bus",
+					       regs, &sun6i_dphy_regmap_config);
 	if (IS_ERR(dphy->regs)) {
-		dev_err(dsi->dev, "Couldn't create the DPHY encoder regmap\n");
+		dev_err(&pdev->dev, "Couldn't create the DPHY encoder regmap\n");
 		return PTR_ERR(dphy->regs);
 	}
 
-	dphy->reset = of_reset_control_get_shared(node, NULL);
+	dphy->reset = devm_reset_control_get_shared(&pdev->dev, NULL);
 	if (IS_ERR(dphy->reset)) {
-		dev_err(dsi->dev, "Couldn't get our reset line\n");
+		dev_err(&pdev->dev, "Couldn't get our reset line\n");
 		return PTR_ERR(dphy->reset);
 	}
 
-	dphy->bus_clk = of_clk_get_by_name(node, "bus");
-	if (IS_ERR(dphy->bus_clk)) {
-		dev_err(dsi->dev, "Couldn't get the DPHY bus clock\n");
-		ret = PTR_ERR(dphy->bus_clk);
-		goto err_free_reset;
-	}
-	regmap_mmio_attach_clk(dphy->regs, dphy->bus_clk);
-
-	dphy->mod_clk = of_clk_get_by_name(node, "mod");
+	dphy->mod_clk = devm_clk_get(&pdev->dev, "mod");
 	if (IS_ERR(dphy->mod_clk)) {
-		dev_err(dsi->dev, "Couldn't get the DPHY mod clock\n");
-		ret = PTR_ERR(dphy->mod_clk);
-		goto err_free_bus;
+		dev_err(&pdev->dev, "Couldn't get the DPHY mod clock\n");
+		return PTR_ERR(dphy->mod_clk);
 	}
 
-	dsi->dphy = dphy;
+	dphy->phy = devm_phy_create(&pdev->dev, NULL, &sun6i_dphy_ops);
+	if (IS_ERR(dphy->phy)) {
+		dev_err(&pdev->dev, "failed to create PHY\n");
+		return PTR_ERR(dphy->phy);
+	}
 
-	return 0;
+	phy_set_drvdata(dphy->phy, dphy);
+	phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
 
-err_free_bus:
-	regmap_mmio_detach_clk(dphy->regs);
-	clk_put(dphy->bus_clk);
-err_free_reset:
-	reset_control_put(dphy->reset);
-	return ret;
+	return PTR_ERR_OR_ZERO(phy_provider);
 }
 
-int sun6i_dphy_remove(struct sun6i_dsi *dsi)
-{
-	struct sun6i_dphy *dphy = dsi->dphy;
-
-	regmap_mmio_detach_clk(dphy->regs);
-	clk_put(dphy->mod_clk);
-	clk_put(dphy->bus_clk);
-	reset_control_put(dphy->reset);
+static const struct of_device_id sun6i_dphy_of_table[] = {
+	{ .compatible = "allwinner,sun6i-a31-mipi-dphy" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sun6i_dphy_of_table);
+
+static struct platform_driver sun6i_dphy_platform_driver = {
+	.probe		= sun6i_dphy_probe,
+	.driver		= {
+		.name		= "sun6i-mipi-dphy",
+		.of_match_table	= sun6i_dphy_of_table,
+	},
+};
+module_platform_driver(sun6i_dphy_platform_driver);
 
-	return 0;
-}
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@bootlin>");
+MODULE_DESCRIPTION("Allwinner A31 MIPI D-PHY Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index e3b34a345546..7bbce7708265 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 
 #include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
@@ -616,6 +617,8 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
 	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
 	struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
 	struct mipi_dsi_device *device = dsi->device;
+	union phy_configure_opts opts = { 0 };
+	struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
 	u16 delay;
 
 	DRM_DEBUG_DRIVER("Enabling DSI output\n");
@@ -634,8 +637,15 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
 	sun6i_dsi_setup_format(dsi, mode);
 	sun6i_dsi_setup_timings(dsi, mode);
 
-	sun6i_dphy_init(dsi->dphy, device->lanes);
-	sun6i_dphy_power_on(dsi->dphy, device->lanes);
+	phy_init(dsi->dphy);
+
+	phy_mipi_dphy_get_default_config(mode->clock * 1000,
+					 mipi_dsi_pixel_format_to_bpp(device->format),
+					 device->lanes, cfg);
+
+	phy_set_mode(dsi->dphy, PHY_MODE_MIPI_DPHY);
+	phy_configure(dsi->dphy, &opts);
+	phy_power_on(dsi->dphy);
 
 	if (!IS_ERR(dsi->panel))
 		drm_panel_prepare(dsi->panel);
@@ -673,8 +683,8 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
 		drm_panel_unprepare(dsi->panel);
 	}
 
-	sun6i_dphy_power_off(dsi->dphy);
-	sun6i_dphy_exit(dsi->dphy);
+	phy_power_off(dsi->dphy);
+	phy_exit(dsi->dphy);
 
 	pm_runtime_put(dsi->dev);
 }
@@ -967,7 +977,6 @@ static const struct component_ops sun6i_dsi_ops = {
 static int sun6i_dsi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *dphy_node;
 	struct sun6i_dsi *dsi;
 	struct resource *res;
 	void __iomem *base;
@@ -1013,10 +1022,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	 */
 	clk_set_rate_exclusive(dsi->mod_clk, 297000000);
 
-	dphy_node = of_parse_phandle(dev->of_node, "phys", 0);
-	ret = sun6i_dphy_probe(dsi, dphy_node);
-	of_node_put(dphy_node);
-	if (ret) {
+	dsi->dphy = devm_phy_get(dev, "dphy");
+	if (IS_ERR(dsi->dphy)) {
 		dev_err(dev, "Couldn't get the MIPI D-PHY\n");
 		goto err_unprotect_clk;
 	}
@@ -1026,7 +1033,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	ret = mipi_dsi_host_register(&dsi->host);
 	if (ret) {
 		dev_err(dev, "Couldn't register MIPI-DSI host\n");
-		goto err_remove_phy;
+		goto err_pm_disable;
 	}
 
 	ret = component_add(&pdev->dev, &sun6i_dsi_ops);
@@ -1039,9 +1046,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 
 err_remove_dsi_host:
 	mipi_dsi_host_unregister(&dsi->host);
-err_remove_phy:
+err_pm_disable:
 	pm_runtime_disable(dev);
-	sun6i_dphy_remove(dsi);
 err_unprotect_clk:
 	clk_rate_exclusive_put(dsi->mod_clk);
 	return ret;
@@ -1055,7 +1061,6 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
 	component_del(&pdev->dev, &sun6i_dsi_ops);
 	mipi_dsi_host_unregister(&dsi->host);
 	pm_runtime_disable(dev);
-	sun6i_dphy_remove(dsi);
 	clk_rate_exclusive_put(dsi->mod_clk);
 
 	return 0;
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index dbbc5b3ecbda..a07090579f84 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -13,13 +13,6 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_mipi_dsi.h>
 
-struct sun6i_dphy {
-	struct clk		*bus_clk;
-	struct clk		*mod_clk;
-	struct regmap		*regs;
-	struct reset_control	*reset;
-};
-
 struct sun6i_dsi {
 	struct drm_connector	connector;
 	struct drm_encoder	encoder;
@@ -29,7 +22,7 @@ struct sun6i_dsi {
 	struct clk		*mod_clk;
 	struct regmap		*regs;
 	struct reset_control	*reset;
-	struct sun6i_dphy	*dphy;
+	struct phy		*dphy;
 
 	struct device		*dev;
 	struct sun4i_drv	*drv;
@@ -52,12 +45,4 @@ static inline struct sun6i_dsi *encoder_to_sun6i_dsi(const struct drm_encoder *e
 	return container_of(encoder, struct sun6i_dsi, encoder);
 };
 
-int sun6i_dphy_probe(struct sun6i_dsi *dsi, struct device_node *node);
-int sun6i_dphy_remove(struct sun6i_dsi *dsi);
-
-int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int lanes);
-int sun6i_dphy_power_on(struct sun6i_dphy *dphy, unsigned int lanes);
-int sun6i_dphy_power_off(struct sun6i_dphy *dphy);
-int sun6i_dphy_exit(struct sun6i_dphy *dphy);
-
 #endif /* _SUN6I_MIPI_DSI_H_ */
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 06/10] phy: Move Allwinner A31 D-PHY driver to drivers/phy/
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
                   ` (4 preceding siblings ...)
  2018-12-07 13:55 ` [PATCH v3 05/10] sun6i: dsi: Convert to generic phy handling Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 07/10] drm/bridge: cdns: Separate DSI and D-PHY configuration Maxime Ripard
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

Now that our MIPI D-PHY driver has been converted to the phy framework,
let's move it into the drivers/phy directory.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/Kconfig                        | 10 +---------
 drivers/gpu/drm/sun4i/Makefile                       |  1 -
 drivers/phy/allwinner/Kconfig                        | 12 ++++++++++++
 drivers/phy/allwinner/Makefile                       |  1 +
 .../allwinner/phy-sun6i-mipi-dphy.c}                 |  0
 5 files changed, 14 insertions(+), 10 deletions(-)
 rename drivers/{gpu/drm/sun4i/sun6i_mipi_dphy.c => phy/allwinner/phy-sun6i-mipi-dphy.c} (100%)

diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index 2b8db82c4bab..1dbbc3a1b763 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -45,20 +45,12 @@ config DRM_SUN6I_DSI
 	default MACH_SUN8I
 	select CRC_CCITT
 	select DRM_MIPI_DSI
-	select DRM_SUN6I_DPHY
+	select PHY_SUN6I_MIPI_DPHY
 	help
 	  Choose this option if you want have an Allwinner SoC with
 	  MIPI-DSI support. If M is selected the module will be called
 	  sun6i_mipi_dsi.
 
-config DRM_SUN6I_DPHY
-	tristate "Allwinner A31 MIPI D-PHY Support"
-	select GENERIC_PHY_MIPI_DPHY
-	help
-	  Choose this option if you have an Allwinner SoC with
-	  MIPI-DSI support. If M is selected, the module will be
-	  called sun6i_mipi_dphy.
-
 config DRM_SUN8I_DW_HDMI
 	tristate "Support for Allwinner version of DesignWare HDMI"
 	depends on DRM_SUN4I
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 1e2320d824b5..0d04f2447b01 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -34,7 +34,6 @@ ifdef CONFIG_DRM_SUN4I_BACKEND
 obj-$(CONFIG_DRM_SUN4I)		+= sun4i-frontend.o
 endif
 obj-$(CONFIG_DRM_SUN4I_HDMI)	+= sun4i-drm-hdmi.o
-obj-$(CONFIG_DRM_SUN6I_DPHY)	+= sun6i_mipi_dphy.o
 obj-$(CONFIG_DRM_SUN6I_DSI)	+= sun6i_mipi_dsi.o
 obj-$(CONFIG_DRM_SUN8I_DW_HDMI)	+= sun8i-drm-hdmi.o
 obj-$(CONFIG_DRM_SUN8I_MIXER)	+= sun8i-mixer.o
diff --git a/drivers/phy/allwinner/Kconfig b/drivers/phy/allwinner/Kconfig
index cdc1e745ba47..fb1204bcc454 100644
--- a/drivers/phy/allwinner/Kconfig
+++ b/drivers/phy/allwinner/Kconfig
@@ -17,6 +17,18 @@ config PHY_SUN4I_USB
 	  This driver controls the entire USB PHY block, both the USB OTG
 	  parts, as well as the 2 regular USB 2 host PHYs.
 
+config PHY_SUN6I_MIPI_DPHY
+	tristate "Allwinner A31 MIPI D-PHY Support"
+	depends on ARCH_SUNXI && HAS_IOMEM && OF
+	depends on RESET_CONTROLLER
+	select GENERIC_PHY
+	select GENERIC_PHY_MIPI_DPHY
+	select REGMAP_MMIO
+	help
+	  Choose this option if you have an Allwinner SoC with
+	  MIPI-DSI support. If M is selected, the module will be
+	  called sun6i_mipi_dphy.
+
 config PHY_SUN9I_USB
 	tristate "Allwinner sun9i SoC USB PHY driver"
 	depends on ARCH_SUNXI && HAS_IOMEM && OF
diff --git a/drivers/phy/allwinner/Makefile b/drivers/phy/allwinner/Makefile
index 8605529c01a1..7d0053efbfaa 100644
--- a/drivers/phy/allwinner/Makefile
+++ b/drivers/phy/allwinner/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
+obj-$(CONFIG_PHY_SUN6I_MIPI_DPHY)	+= phy-sun6i-mipi-dphy.o
 obj-$(CONFIG_PHY_SUN9I_USB)		+= phy-sun9i-usb.o
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
similarity index 100%
rename from drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
rename to drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 07/10] drm/bridge: cdns: Separate DSI and D-PHY configuration
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
                   ` (5 preceding siblings ...)
  2018-12-07 13:55 ` [PATCH v3 06/10] phy: Move Allwinner A31 D-PHY driver to drivers/phy/ Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 08/10] dt-bindings: phy: Move the Cadence D-PHY bindings Maxime Ripard
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

The current configuration of the DSI bridge and its associated D-PHY is
intertwined. In order to ease the future conversion to the phy framework
for the D-PHY part, let's split the configuration in two.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/bridge/cdns-dsi.c | 96 ++++++++++++++++++++++----------
 1 file changed, 68 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c
index ce9496d13986..3ac6dd524b6d 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -545,6 +545,11 @@ bridge_to_cdns_dsi_input(struct drm_bridge *bridge)
 	return container_of(bridge, struct cdns_dsi_input, bridge);
 }
 
+static unsigned int mode_to_dpi_hfp(const struct drm_display_mode *mode)
+{
+	return mode->crtc_hsync_start - mode->crtc_hdisplay;
+}
+
 static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
 				     struct cdns_dphy_cfg *cfg,
 				     unsigned int dpi_htotal,
@@ -731,14 +736,12 @@ static unsigned int dpi_to_dsi_timing(unsigned int dpi_timing,
 static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
 			     const struct drm_display_mode *mode,
 			     struct cdns_dsi_cfg *dsi_cfg,
-			     struct cdns_dphy_cfg *dphy_cfg,
 			     bool mode_valid_check)
 {
-	unsigned long dsi_htotal = 0, dsi_hss_hsa_hse_hbp = 0;
 	struct cdns_dsi_output *output = &dsi->output;
-	unsigned int dsi_hfp_ext = 0, dpi_hfp, tmp;
+	unsigned int tmp;
 	bool sync_pulse = false;
-	int bpp, nlanes, ret;
+	int bpp, nlanes;
 
 	memset(dsi_cfg, 0, sizeof(*dsi_cfg));
 
@@ -757,8 +760,6 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
 		       mode->crtc_hsync_end : mode->crtc_hsync_start);
 
 	dsi_cfg->hbp = dpi_to_dsi_timing(tmp, bpp, DSI_HBP_FRAME_OVERHEAD);
-	dsi_htotal += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
-	dsi_hss_hsa_hse_hbp += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
 
 	if (sync_pulse) {
 		if (mode_valid_check)
@@ -768,49 +769,90 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
 
 		dsi_cfg->hsa = dpi_to_dsi_timing(tmp, bpp,
 						 DSI_HSA_FRAME_OVERHEAD);
-		dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
-		dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
 	}
 
 	dsi_cfg->hact = dpi_to_dsi_timing(mode_valid_check ?
 					  mode->hdisplay : mode->crtc_hdisplay,
 					  bpp, 0);
-	dsi_htotal += dsi_cfg->hact;
+	dsi_cfg->hfp = dpi_to_dsi_timing(mode_to_dpi_hfp(mode), bpp,
+					 DSI_HFP_FRAME_OVERHEAD);
 
-	if (mode_valid_check)
-		dpi_hfp = mode->hsync_start - mode->hdisplay;
-	else
-		dpi_hfp = mode->crtc_hsync_start - mode->crtc_hdisplay;
+	return 0;
+}
+
+static int cdns_dphy_validate(struct cdns_dsi *dsi,
+			      struct cdns_dsi_cfg *dsi_cfg,
+			      struct cdns_dphy_cfg *dphy_cfg,
+			      const struct drm_display_mode *mode,
+			      bool mode_valid_check)
+{
+	struct cdns_dsi_output *output = &dsi->output;
+	unsigned long dsi_htotal;
+	unsigned int dsi_hfp_ext = 0;
+
+	int ret;
 
-	dsi_cfg->hfp = dpi_to_dsi_timing(dpi_hfp, bpp, DSI_HFP_FRAME_OVERHEAD);
+	dsi_htotal = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
+	if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
+		dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
+
+	dsi_htotal += dsi_cfg->hact;
 	dsi_htotal += dsi_cfg->hfp + DSI_HFP_FRAME_OVERHEAD;
 
 	if (mode_valid_check)
 		ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
-						mode->htotal, bpp,
+						mode->htotal,
 						mode->clock * 1000,
-						dsi_htotal, nlanes,
+						mipi_dsi_pixel_format_to_bpp(output->dev->format),
+						dsi_htotal,
+						output->dev->lanes,
 						&dsi_hfp_ext);
 	else
 		ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
-						mode->crtc_htotal, bpp,
+						mode->crtc_htotal,
+						mipi_dsi_pixel_format_to_bpp(output->dev->format),
 						mode->crtc_clock * 1000,
-						dsi_htotal, nlanes,
+						dsi_htotal,
+						output->dev->lanes,
 						&dsi_hfp_ext);
-
 	if (ret)
 		return ret;
 
 	dsi_cfg->hfp += dsi_hfp_ext;
-	dsi_htotal += dsi_hfp_ext;
-	dsi_cfg->htotal = dsi_htotal;
+	dsi_cfg->htotal = dsi_htotal + dsi_hfp_ext;
+
+	return 0;
+}
+
+static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
+			       const struct drm_display_mode *mode,
+			       struct cdns_dsi_cfg *dsi_cfg,
+			       struct cdns_dphy_cfg *dphy_cfg,
+			       bool mode_valid_check)
+{
+	struct cdns_dsi_output *output = &dsi->output;
+	unsigned long dsi_hss_hsa_hse_hbp;
+	unsigned int nlanes = output->dev->lanes;
+	int ret;
+
+	ret = cdns_dsi_mode2cfg(dsi, mode, dsi_cfg, mode_valid_check);
+	if (ret)
+		return ret;
+
+	ret = cdns_dphy_validate(dsi, dsi_cfg, dphy_cfg, mode, mode_valid_check);
+	if (ret)
+		return ret;
+
+	dsi_hss_hsa_hse_hbp = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
+	if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
+		dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
 
 	/*
 	 * Make sure DPI(HFP) > DSI(HSS+HSA+HSE+HBP) to guarantee that the FIFO
 	 * is empty before we start a receiving a new line on the DPI
 	 * interface.
 	 */
-	if ((u64)dphy_cfg->lane_bps * dpi_hfp * nlanes <
+	if ((u64)dphy_cfg->lane_bps * mode_to_dpi_hfp(mode) * nlanes <
 	    (u64)dsi_hss_hsa_hse_hbp *
 	    (mode_valid_check ? mode->clock : mode->crtc_clock) * 1000)
 		return -EINVAL;
@@ -842,7 +884,7 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge,
 	struct cdns_dsi_output *output = &dsi->output;
 	struct cdns_dphy_cfg dphy_cfg;
 	struct cdns_dsi_cfg dsi_cfg;
-	int bpp, nlanes, ret;
+	int bpp, ret;
 
 	/*
 	 * VFP_DSI should be less than VFP_DPI and VFP_DSI should be at
@@ -860,11 +902,9 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge,
 	if ((mode->hdisplay * bpp) % 32)
 		return MODE_H_ILLEGAL;
 
-	nlanes = output->dev->lanes;
-
-	ret = cdns_dsi_mode2cfg(dsi, mode, &dsi_cfg, &dphy_cfg, true);
+	ret = cdns_dsi_check_conf(dsi, mode, &dsi_cfg, &dphy_cfg, true);
 	if (ret)
-		return MODE_CLOCK_RANGE;
+		return MODE_BAD;
 
 	return MODE_OK;
 }
@@ -990,7 +1030,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge)
 	bpp = mipi_dsi_pixel_format_to_bpp(output->dev->format);
 	nlanes = output->dev->lanes;
 
-	WARN_ON_ONCE(cdns_dsi_mode2cfg(dsi, mode, &dsi_cfg, &dphy_cfg, false));
+	WARN_ON_ONCE(cdns_dsi_check_conf(dsi, mode, &dsi_cfg, &dphy_cfg, false));
 
 	cdns_dsi_hs_init(dsi, &dphy_cfg);
 	cdns_dsi_init_link(dsi);
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 08/10] dt-bindings: phy: Move the Cadence D-PHY bindings
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
                   ` (6 preceding siblings ...)
  2018-12-07 13:55 ` [PATCH v3 07/10] drm/bridge: cdns: Separate DSI and D-PHY configuration Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 09/10] phy: Add Cadence D-PHY support Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 10/10] drm/bridge: cdns: Convert to phy framework Maxime Ripard
  9 siblings, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

The Cadence D-PHY bindings was defined as part of the DSI block so far.
However, since it's now going to be a separate driver, we need to move the
binding to a file of its own.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +-------
 Documentation/devicetree/bindings/phy/cdns,dphy.txt           | 20 +++++++-
 2 files changed, 20 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
index f5725bb6c61c..525a4bfd8634 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
@@ -31,28 +31,7 @@ Required subnodes:
 - one subnode per DSI device connected on the DSI bus. Each DSI device should
   contain a reg property encoding its virtual channel.
 
-Cadence DPHY
-============
-
-Cadence DPHY block.
-
-Required properties:
-- compatible: should be set to "cdns,dphy".
-- reg: physical base address and length of the DPHY registers.
-- clocks: DPHY reference clocks.
-- clock-names: must contain "psm" and "pll_ref".
-- #phy-cells: must be set to 0.
-
-
 Example:
-	dphy0: dphy@fd0e0000{
-		compatible = "cdns,dphy";
-		reg = <0x0 0xfd0e0000 0x0 0x1000>;
-		clocks = <&psm_clk>, <&pll_ref_clk>;
-		clock-names = "psm", "pll_ref";
-		#phy-cells = <0>;
-	};
-
 	dsi0: dsi@fd0c0000 {
 		compatible = "cdns,dsi";
 		reg = <0x0 0xfd0c0000 0x0 0x1000>;
diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.txt b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
new file mode 100644
index 000000000000..1095bc4e72d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
@@ -0,0 +1,20 @@
+Cadence DPHY
+============
+
+Cadence DPHY block.
+
+Required properties:
+- compatible: should be set to "cdns,dphy".
+- reg: physical base address and length of the DPHY registers.
+- clocks: DPHY reference clocks.
+- clock-names: must contain "psm" and "pll_ref".
+- #phy-cells: must be set to 0.
+
+Example:
+	dphy0: dphy@fd0e0000{
+		compatible = "cdns,dphy";
+		reg = <0x0 0xfd0e0000 0x0 0x1000>;
+		clocks = <&psm_clk>, <&pll_ref_clk>;
+		clock-names = "psm", "pll_ref";
+		#phy-cells = <0>;
+	};
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 09/10] phy: Add Cadence D-PHY support
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
                   ` (7 preceding siblings ...)
  2018-12-07 13:55 ` [PATCH v3 08/10] dt-bindings: phy: Move the Cadence D-PHY bindings Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-07 13:55 ` [PATCH v3 10/10] drm/bridge: cdns: Convert to phy framework Maxime Ripard
  9 siblings, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

Cadence has designed a D-PHY that can be used by the, currently in tree,
DSI bridge (DRM), CSI Transceiver and CSI Receiver (v4l2) drivers.

Only the DSI driver has an ad-hoc driver for that phy at the moment, while
the v4l2 drivers are completely missing any phy support. In order to make
that phy support available to all these drivers, without having to
duplicate that code three times, let's create a generic phy framework
driver.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/phy/cadence/Kconfig     |  13 +-
 drivers/phy/cadence/Makefile    |   1 +-
 drivers/phy/cadence/cdns-dphy.c | 389 +++++++++++++++++++++++++++++++++-
 3 files changed, 402 insertions(+), 1 deletion(-)
 create mode 100644 drivers/phy/cadence/cdns-dphy.c

diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
index 2b8c0851ff33..31f18b67dd7c 100644
--- a/drivers/phy/cadence/Kconfig
+++ b/drivers/phy/cadence/Kconfig
@@ -1,6 +1,7 @@
 #
 # Phy drivers for Cadence PHYs
 #
+
 config PHY_CADENCE_DP
 	tristate "Cadence MHDP DisplayPort PHY driver"
 	depends on OF
@@ -9,9 +10,19 @@ config PHY_CADENCE_DP
 	help
 	  Support for Cadence MHDP DisplayPort PHY.
 
+config PHY_CADENCE_DPHY
+	tristate "Cadence D-PHY Support"
+	depends on HAS_IOMEM && OF
+	select GENERIC_PHY
+	select GENERIC_PHY_MIPI_DPHY
+	help
+	  Choose this option if you have a Cadence D-PHY in your
+	  system. If M is selected, the module will be called
+	  cdns-dphy.
+
 config PHY_CADENCE_SIERRA
 	tristate "Cadence Sierra PHY Driver"
 	depends on OF && HAS_IOMEM && RESET_CONTROLLER
 	select GENERIC_PHY
 	help
-	  Enable this to support the Cadence Sierra PHY driver
\ No newline at end of file
+	  Enable this to support the Cadence Sierra PHY driver
diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
index 412349af0492..2f9e3457b954 100644
--- a/drivers/phy/cadence/Makefile
+++ b/drivers/phy/cadence/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_PHY_CADENCE_DP)	+= phy-cadence-dp.o
+obj-$(CONFIG_PHY_CADENCE_DPHY)	+= cdns-dphy.o
 obj-$(CONFIG_PHY_CADENCE_SIERRA)	+= phy-cadence-sierra.o
diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
new file mode 100644
index 000000000000..1d0abba03f37
--- /dev/null
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -0,0 +1,389 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright: 2017-2018 Cadence Design Systems, Inc.
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.h>
+
+#define REG_WAKEUP_TIME_NS		800
+#define DPHY_PLL_RATE_HZ		108000000
+
+/* DPHY registers */
+#define DPHY_PMA_CMN(reg)		(reg)
+#define DPHY_PMA_LCLK(reg)		(0x100 + (reg))
+#define DPHY_PMA_LDATA(lane, reg)	(0x200 + ((lane) * 0x100) + (reg))
+#define DPHY_PMA_RCLK(reg)		(0x600 + (reg))
+#define DPHY_PMA_RDATA(lane, reg)	(0x700 + ((lane) * 0x100) + (reg))
+#define DPHY_PCS(reg)			(0xb00 + (reg))
+
+#define DPHY_CMN_SSM			DPHY_PMA_CMN(0x20)
+#define DPHY_CMN_SSM_EN			BIT(0)
+#define DPHY_CMN_TX_MODE_EN		BIT(9)
+
+#define DPHY_CMN_PWM			DPHY_PMA_CMN(0x40)
+#define DPHY_CMN_PWM_DIV(x)		((x) << 20)
+#define DPHY_CMN_PWM_LOW(x)		((x) << 10)
+#define DPHY_CMN_PWM_HIGH(x)		(x)
+
+#define DPHY_CMN_FBDIV			DPHY_PMA_CMN(0x4c)
+#define DPHY_CMN_FBDIV_VAL(low, high)	(((high) << 11) | ((low) << 22))
+#define DPHY_CMN_FBDIV_FROM_REG		(BIT(10) | BIT(21))
+
+#define DPHY_CMN_OPIPDIV		DPHY_PMA_CMN(0x50)
+#define DPHY_CMN_IPDIV_FROM_REG		BIT(0)
+#define DPHY_CMN_IPDIV(x)		((x) << 1)
+#define DPHY_CMN_OPDIV_FROM_REG		BIT(6)
+#define DPHY_CMN_OPDIV(x)		((x) << 7)
+
+#define DPHY_PSM_CFG			DPHY_PCS(0x4)
+#define DPHY_PSM_CFG_FROM_REG		BIT(0)
+#define DPHY_PSM_CLK_DIV(x)		((x) << 1)
+
+#define DSI_HBP_FRAME_OVERHEAD		12
+#define DSI_HSA_FRAME_OVERHEAD		14
+#define DSI_HFP_FRAME_OVERHEAD		6
+#define DSI_HSS_VSS_VSE_FRAME_OVERHEAD	4
+#define DSI_BLANKING_FRAME_OVERHEAD	6
+#define DSI_NULL_FRAME_OVERHEAD		6
+#define DSI_EOT_PKT_SIZE		4
+
+struct cdns_dphy_cfg {
+	u8 pll_ipdiv;
+	u8 pll_opdiv;
+	u16 pll_fbdiv;
+	unsigned int nlanes;
+};
+
+enum cdns_dphy_clk_lane_cfg {
+	DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
+	DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
+	DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
+	DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
+};
+
+struct cdns_dphy;
+struct cdns_dphy_ops {
+	int (*probe)(struct cdns_dphy *dphy);
+	void (*remove)(struct cdns_dphy *dphy);
+	void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
+	void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
+				 enum cdns_dphy_clk_lane_cfg cfg);
+	void (*set_pll_cfg)(struct cdns_dphy *dphy,
+			    const struct cdns_dphy_cfg *cfg);
+	unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
+};
+
+struct cdns_dphy {
+	struct cdns_dphy_cfg cfg;
+	void __iomem *regs;
+	struct clk *psm_clk;
+	struct clk *pll_ref_clk;
+	const struct cdns_dphy_ops *ops;
+	struct phy *phy;
+};
+
+static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
+				     struct cdns_dphy_cfg *cfg,
+				     struct phy_configure_opts_mipi_dphy *opts,
+				     unsigned int *dsi_hfp_ext)
+{
+	unsigned long pll_ref_hz = clk_get_rate(dphy->pll_ref_clk);
+	u64 dlane_bps;
+
+	memset(cfg, 0, sizeof(*cfg));
+
+	if (pll_ref_hz < 9600000 || pll_ref_hz >= 150000000)
+		return -EINVAL;
+	else if (pll_ref_hz < 19200000)
+		cfg->pll_ipdiv = 1;
+	else if (pll_ref_hz < 38400000)
+		cfg->pll_ipdiv = 2;
+	else if (pll_ref_hz < 76800000)
+		cfg->pll_ipdiv = 4;
+	else
+		cfg->pll_ipdiv = 8;
+
+	dlane_bps = opts->hs_clk_rate;
+
+	if (dlane_bps > 2500000000UL || dlane_bps < 160000000UL)
+		return -EINVAL;
+	else if (dlane_bps >= 1250000000)
+		cfg->pll_opdiv = 1;
+	else if (dlane_bps >= 630000000)
+		cfg->pll_opdiv = 2;
+	else if (dlane_bps >= 320000000)
+		cfg->pll_opdiv = 4;
+	else if (dlane_bps >= 160000000)
+		cfg->pll_opdiv = 8;
+
+	cfg->pll_fbdiv = DIV_ROUND_UP_ULL(dlane_bps * 2 * cfg->pll_opdiv *
+					  cfg->pll_ipdiv,
+					  pll_ref_hz);
+
+	return 0;
+}
+
+static int cdns_dphy_setup_psm(struct cdns_dphy *dphy)
+{
+	unsigned long psm_clk_hz = clk_get_rate(dphy->psm_clk);
+	unsigned long psm_div;
+
+	if (!psm_clk_hz || psm_clk_hz > 100000000)
+		return -EINVAL;
+
+	psm_div = DIV_ROUND_CLOSEST(psm_clk_hz, 1000000);
+	if (dphy->ops->set_psm_div)
+		dphy->ops->set_psm_div(dphy, psm_div);
+
+	return 0;
+}
+
+static void cdns_dphy_set_clk_lane_cfg(struct cdns_dphy *dphy,
+				       enum cdns_dphy_clk_lane_cfg cfg)
+{
+	if (dphy->ops->set_clk_lane_cfg)
+		dphy->ops->set_clk_lane_cfg(dphy, cfg);
+}
+
+static void cdns_dphy_set_pll_cfg(struct cdns_dphy *dphy,
+				  const struct cdns_dphy_cfg *cfg)
+{
+	if (dphy->ops->set_pll_cfg)
+		dphy->ops->set_pll_cfg(dphy, cfg);
+}
+
+static unsigned long cdns_dphy_get_wakeup_time_ns(struct cdns_dphy *dphy)
+{
+	return dphy->ops->get_wakeup_time_ns(dphy);
+}
+
+static unsigned long cdns_dphy_ref_get_wakeup_time_ns(struct cdns_dphy *dphy)
+{
+	/* Default wakeup time is 800 ns (in a simulated environment). */
+	return 800;
+}
+
+static void cdns_dphy_ref_set_pll_cfg(struct cdns_dphy *dphy,
+				      const struct cdns_dphy_cfg *cfg)
+{
+	u32 fbdiv_low, fbdiv_high;
+
+	fbdiv_low = (cfg->pll_fbdiv / 4) - 2;
+	fbdiv_high = cfg->pll_fbdiv - fbdiv_low - 2;
+
+	writel(DPHY_CMN_IPDIV_FROM_REG | DPHY_CMN_OPDIV_FROM_REG |
+	       DPHY_CMN_IPDIV(cfg->pll_ipdiv) |
+	       DPHY_CMN_OPDIV(cfg->pll_opdiv),
+	       dphy->regs + DPHY_CMN_OPIPDIV);
+	writel(DPHY_CMN_FBDIV_FROM_REG |
+	       DPHY_CMN_FBDIV_VAL(fbdiv_low, fbdiv_high),
+	       dphy->regs + DPHY_CMN_FBDIV);
+	writel(DPHY_CMN_PWM_HIGH(6) | DPHY_CMN_PWM_LOW(0x101) |
+	       DPHY_CMN_PWM_DIV(0x8),
+	       dphy->regs + DPHY_CMN_PWM);
+}
+
+static void cdns_dphy_ref_set_psm_div(struct cdns_dphy *dphy, u8 div)
+{
+	writel(DPHY_PSM_CFG_FROM_REG | DPHY_PSM_CLK_DIV(div),
+	       dphy->regs + DPHY_PSM_CFG);
+}
+
+/*
+ * This is the reference implementation of DPHY hooks. Specific integration of
+ * this IP may have to re-implement some of them depending on how they decided
+ * to wire things in the SoC.
+ */
+static const struct cdns_dphy_ops ref_dphy_ops = {
+	.get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
+	.set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
+	.set_psm_div = cdns_dphy_ref_set_psm_div,
+};
+
+static int cdns_dphy_config_from_opts(struct phy *phy,
+				      struct phy_configure_opts_mipi_dphy *opts,
+				      struct cdns_dphy_cfg *cfg)
+{
+	struct cdns_dphy *dphy = phy_get_drvdata(phy);
+	unsigned int dsi_hfp_ext = 0;
+	int ret;
+
+	ret = phy_mipi_dphy_config_validate(opts);
+	if (ret)
+		return ret;
+
+	ret = cdns_dsi_get_dphy_pll_cfg(dphy, cfg,
+					opts, &dsi_hfp_ext);
+	if (ret)
+		return ret;
+
+	opts->wakeup = cdns_dphy_get_wakeup_time_ns(dphy) * 1000;
+
+	return 0;
+}
+
+static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
+			      union phy_configure_opts *opts)
+{
+	struct cdns_dphy_cfg cfg = { 0 };
+
+	if (mode != PHY_MODE_MIPI_DPHY)
+		return -EINVAL;
+
+	return cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+}
+
+static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+	struct cdns_dphy *dphy = phy_get_drvdata(phy);
+	struct cdns_dphy_cfg cfg = { 0 };
+	int ret;
+
+	ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+	if (ret)
+		return ret;
+
+	/*
+	 * Configure the internal PSM clk divider so that the DPHY has a
+	 * 1MHz clk (or something close).
+	 */
+	ret = cdns_dphy_setup_psm(dphy);
+	if (ret)
+		return ret;
+
+	/*
+	 * Configure attach clk lanes to data lanes: the DPHY has 2 clk lanes
+	 * and 8 data lanes, each clk lane can be attache different set of
+	 * data lanes. The 2 groups are named 'left' and 'right', so here we
+	 * just say that we want the 'left' clk lane to drive the 'left' data
+	 * lanes.
+	 */
+	cdns_dphy_set_clk_lane_cfg(dphy, DPHY_CLK_CFG_LEFT_DRIVES_LEFT);
+
+	/*
+	 * Configure the DPHY PLL that will be used to generate the TX byte
+	 * clk.
+	 */
+	cdns_dphy_set_pll_cfg(dphy, &cfg);
+
+	return 0;
+}
+
+static int cdns_dphy_power_on(struct phy *phy)
+{
+	struct cdns_dphy *dphy = phy_get_drvdata(phy);
+
+	clk_prepare_enable(dphy->psm_clk);
+	clk_prepare_enable(dphy->pll_ref_clk);
+
+	/* Start TX state machine. */
+	writel(DPHY_CMN_SSM_EN | DPHY_CMN_TX_MODE_EN, dphy->regs + DPHY_CMN_SSM);
+
+	return 0;
+}
+
+static int cdns_dphy_power_off(struct phy *phy)
+{
+	struct cdns_dphy *dphy = phy_get_drvdata(phy);
+
+	clk_disable_unprepare(dphy->pll_ref_clk);
+	clk_disable_unprepare(dphy->psm_clk);
+
+	return 0;
+}
+
+static const struct phy_ops cdns_dphy_ops = {
+	.configure	= cdns_dphy_configure,
+	.validate	= cdns_dphy_validate,
+	.power_on	= cdns_dphy_power_on,
+	.power_off	= cdns_dphy_power_off,
+};
+
+static int cdns_dphy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct cdns_dphy *dphy;
+	struct resource *res;
+	int ret;
+
+	dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
+	if (!dphy)
+		return -ENOMEM;
+	dev_set_drvdata(&pdev->dev, dphy);
+
+	dphy->ops = of_device_get_match_data(&pdev->dev);
+	if (!dphy->ops)
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dphy->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(dphy->regs))
+		return PTR_ERR(dphy->regs);
+
+	dphy->psm_clk = devm_clk_get(&pdev->dev, "psm");
+	if (IS_ERR(dphy->psm_clk))
+		return PTR_ERR(dphy->psm_clk);
+
+	dphy->pll_ref_clk = devm_clk_get(&pdev->dev, "pll_ref");
+	if (IS_ERR(dphy->pll_ref_clk))
+		return PTR_ERR(dphy->pll_ref_clk);
+
+	if (dphy->ops->probe) {
+		ret = dphy->ops->probe(dphy);
+		if (ret)
+			return ret;
+	}
+
+	dphy->phy = devm_phy_create(&pdev->dev, NULL, &cdns_dphy_ops);
+	if (IS_ERR(dphy->phy)) {
+		dev_err(&pdev->dev, "failed to create PHY\n");
+		if (dphy->ops->remove)
+			dphy->ops->remove(dphy);
+		return PTR_ERR(dphy->phy);
+	}
+
+	phy_set_drvdata(dphy->phy, dphy);
+	phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static int cdns_dphy_remove(struct platform_device *pdev)
+{
+	struct cdns_dphy *dphy = dev_get_drvdata(&pdev->dev);
+
+	if (dphy->ops->remove)
+		dphy->ops->remove(dphy);
+
+	return 0;
+}
+
+static const struct of_device_id cdns_dphy_of_match[] = {
+	{ .compatible = "cdns,dphy", .data = &ref_dphy_ops },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, cdns_dphy_of_match);
+
+static struct platform_driver cdns_dphy_platform_driver = {
+	.probe		= cdns_dphy_probe,
+	.remove		= cdns_dphy_remove,
+	.driver		= {
+		.name		= "cdns-mipi-dphy",
+		.of_match_table	= cdns_dphy_of_match,
+	},
+};
+module_platform_driver(cdns_dphy_platform_driver);
+
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@bootlin.com>");
+MODULE_DESCRIPTION("Cadence MIPI D-PHY Driver");
+MODULE_LICENSE("GPL");
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 10/10] drm/bridge: cdns: Convert to phy framework
  2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
                   ` (8 preceding siblings ...)
  2018-12-07 13:55 ` [PATCH v3 09/10] phy: Add Cadence D-PHY support Maxime Ripard
@ 2018-12-07 13:55 ` Maxime Ripard
  2018-12-09  4:37   ` kbuild test robot
  2018-12-20 10:52   ` kbuild test robot
  9 siblings, 2 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-07 13:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Boris Brezillon
  Cc: Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

Now that we have everything we need in the phy framework to allow to tune
the phy parameters, let's convert the Cadence DSI bridge to that API
instead of creating a ad-hoc driver for its phy.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/bridge/cdns-dsi.c | 485 +++----------------------------
 1 file changed, 59 insertions(+), 426 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c
index 3ac6dd524b6d..7b432257ffbe 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -21,6 +21,9 @@
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
 
+#include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.h>
+
 #define IP_CONF				0x0
 #define SP_HS_FIFO_DEPTH(x)		(((x) & GENMASK(30, 26)) >> 26)
 #define SP_LP_FIFO_DEPTH(x)		(((x) & GENMASK(25, 21)) >> 21)
@@ -419,44 +422,11 @@
 #define DSI_NULL_FRAME_OVERHEAD		6
 #define DSI_EOT_PKT_SIZE		4
 
-#define REG_WAKEUP_TIME_NS		800
-#define DPHY_PLL_RATE_HZ		108000000
-
-/* DPHY registers */
-#define DPHY_PMA_CMN(reg)		(reg)
-#define DPHY_PMA_LCLK(reg)		(0x100 + (reg))
-#define DPHY_PMA_LDATA(lane, reg)	(0x200 + ((lane) * 0x100) + (reg))
-#define DPHY_PMA_RCLK(reg)		(0x600 + (reg))
-#define DPHY_PMA_RDATA(lane, reg)	(0x700 + ((lane) * 0x100) + (reg))
-#define DPHY_PCS(reg)			(0xb00 + (reg))
-
-#define DPHY_CMN_SSM			DPHY_PMA_CMN(0x20)
-#define DPHY_CMN_SSM_EN			BIT(0)
-#define DPHY_CMN_TX_MODE_EN		BIT(9)
-
-#define DPHY_CMN_PWM			DPHY_PMA_CMN(0x40)
-#define DPHY_CMN_PWM_DIV(x)		((x) << 20)
-#define DPHY_CMN_PWM_LOW(x)		((x) << 10)
-#define DPHY_CMN_PWM_HIGH(x)		(x)
-
-#define DPHY_CMN_FBDIV			DPHY_PMA_CMN(0x4c)
-#define DPHY_CMN_FBDIV_VAL(low, high)	(((high) << 11) | ((low) << 22))
-#define DPHY_CMN_FBDIV_FROM_REG		(BIT(10) | BIT(21))
-
-#define DPHY_CMN_OPIPDIV		DPHY_PMA_CMN(0x50)
-#define DPHY_CMN_IPDIV_FROM_REG		BIT(0)
-#define DPHY_CMN_IPDIV(x)		((x) << 1)
-#define DPHY_CMN_OPDIV_FROM_REG		BIT(6)
-#define DPHY_CMN_OPDIV(x)		((x) << 7)
-
-#define DPHY_PSM_CFG			DPHY_PCS(0x4)
-#define DPHY_PSM_CFG_FROM_REG		BIT(0)
-#define DPHY_PSM_CLK_DIV(x)		((x) << 1)
-
 struct cdns_dsi_output {
 	struct mipi_dsi_device *dev;
 	struct drm_panel *panel;
 	struct drm_bridge *bridge;
+	union phy_configure_opts phy_opts;
 };
 
 enum cdns_dsi_input_id {
@@ -465,14 +435,6 @@ enum cdns_dsi_input_id {
 	CDNS_DSC_INPUT,
 };
 
-struct cdns_dphy_cfg {
-	u8 pll_ipdiv;
-	u8 pll_opdiv;
-	u16 pll_fbdiv;
-	unsigned long lane_bps;
-	unsigned int nlanes;
-};
-
 struct cdns_dsi_cfg {
 	unsigned int hfp;
 	unsigned int hsa;
@@ -481,34 +443,6 @@ struct cdns_dsi_cfg {
 	unsigned int htotal;
 };
 
-struct cdns_dphy;
-
-enum cdns_dphy_clk_lane_cfg {
-	DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
-	DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
-	DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
-	DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
-};
-
-struct cdns_dphy_ops {
-	int (*probe)(struct cdns_dphy *dphy);
-	void (*remove)(struct cdns_dphy *dphy);
-	void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
-	void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
-				 enum cdns_dphy_clk_lane_cfg cfg);
-	void (*set_pll_cfg)(struct cdns_dphy *dphy,
-			    const struct cdns_dphy_cfg *cfg);
-	unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
-};
-
-struct cdns_dphy {
-	struct cdns_dphy_cfg cfg;
-	void __iomem *regs;
-	struct clk *psm_clk;
-	struct clk *pll_ref_clk;
-	const struct cdns_dphy_ops *ops;
-};
-
 struct cdns_dsi_input {
 	enum cdns_dsi_input_id id;
 	struct drm_bridge bridge;
@@ -526,7 +460,7 @@ struct cdns_dsi {
 	struct reset_control *dsi_p_rst;
 	struct clk *dsi_sys_clk;
 	bool link_initialized;
-	struct cdns_dphy *dphy;
+	struct phy *dphy;
 };
 
 static inline struct cdns_dsi *input_to_dsi(struct cdns_dsi_input *input)
@@ -550,175 +484,6 @@ static unsigned int mode_to_dpi_hfp(const struct drm_display_mode *mode)
 	return mode->crtc_hsync_start - mode->crtc_hdisplay;
 }
 
-static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
-				     struct cdns_dphy_cfg *cfg,
-				     unsigned int dpi_htotal,
-				     unsigned int dpi_bpp,
-				     unsigned int dpi_hz,
-				     unsigned int dsi_htotal,
-				     unsigned int dsi_nlanes,
-				     unsigned int *dsi_hfp_ext)
-{
-	u64 dlane_bps, dlane_bps_max, fbdiv, fbdiv_max, adj_dsi_htotal;
-	unsigned long pll_ref_hz = clk_get_rate(dphy->pll_ref_clk);
-
-	memset(cfg, 0, sizeof(*cfg));
-
-	cfg->nlanes = dsi_nlanes;
-
-	if (pll_ref_hz < 9600000 || pll_ref_hz >= 150000000)
-		return -EINVAL;
-	else if (pll_ref_hz < 19200000)
-		cfg->pll_ipdiv = 1;
-	else if (pll_ref_hz < 38400000)
-		cfg->pll_ipdiv = 2;
-	else if (pll_ref_hz < 76800000)
-		cfg->pll_ipdiv = 4;
-	else
-		cfg->pll_ipdiv = 8;
-
-	/*
-	 * Make sure DSI htotal is aligned on a lane boundary when calculating
-	 * the expected data rate. This is done by extending HFP in case of
-	 * misalignment.
-	 */
-	adj_dsi_htotal = dsi_htotal;
-	if (dsi_htotal % dsi_nlanes)
-		adj_dsi_htotal += dsi_nlanes - (dsi_htotal % dsi_nlanes);
-
-	dlane_bps = (u64)dpi_hz * adj_dsi_htotal;
-
-	/* data rate in bytes/sec is not an integer, refuse the mode. */
-	if (do_div(dlane_bps, dsi_nlanes * dpi_htotal))
-		return -EINVAL;
-
-	/* data rate was in bytes/sec, convert to bits/sec. */
-	dlane_bps *= 8;
-
-	if (dlane_bps > 2500000000UL || dlane_bps < 160000000UL)
-		return -EINVAL;
-	else if (dlane_bps >= 1250000000)
-		cfg->pll_opdiv = 1;
-	else if (dlane_bps >= 630000000)
-		cfg->pll_opdiv = 2;
-	else if (dlane_bps >= 320000000)
-		cfg->pll_opdiv = 4;
-	else if (dlane_bps >= 160000000)
-		cfg->pll_opdiv = 8;
-
-	/*
-	 * Allow a deviation of 0.2% on the per-lane data rate to try to
-	 * recover a potential mismatch between DPI and PPI clks.
-	 */
-	dlane_bps_max = dlane_bps + DIV_ROUND_DOWN_ULL(dlane_bps, 500);
-	fbdiv_max = DIV_ROUND_DOWN_ULL(dlane_bps_max * 2 *
-				       cfg->pll_opdiv * cfg->pll_ipdiv,
-				       pll_ref_hz);
-	fbdiv = DIV_ROUND_UP_ULL(dlane_bps * 2 * cfg->pll_opdiv *
-				 cfg->pll_ipdiv,
-				 pll_ref_hz);
-
-	/*
-	 * Iterate over all acceptable fbdiv and try to find an adjusted DSI
-	 * htotal length providing an exact match.
-	 *
-	 * Note that we could do something even trickier by relying on the fact
-	 * that a new line is not necessarily aligned on a lane boundary, so,
-	 * by making adj_dsi_htotal non aligned on a dsi_lanes we can improve a
-	 * bit the precision. With this, the step would be
-	 *
-	 *	pll_ref_hz / (2 * opdiv * ipdiv * nlanes)
-	 *
-	 * instead of
-	 *
-	 *	pll_ref_hz / (2 * opdiv * ipdiv)
-	 *
-	 * The drawback of this approach is that we would need to make sure the
-	 * number or lines is a multiple of the realignment periodicity which is
-	 * a function of the number of lanes and the original misalignment. For
-	 * example, for NLANES = 4 and HTOTAL % NLANES = 3, it takes 4 lines
-	 * to realign on a lane:
-	 * LINE 0: expected number of bytes, starts emitting first byte of
-	 *	   LINE 1 on LANE 3
-	 * LINE 1: expected number of bytes, starts emitting first 2 bytes of
-	 *	   LINE 2 on LANES 2 and 3
-	 * LINE 2: expected number of bytes, starts emitting first 3 bytes of
-	 *	   of LINE 3 on LANES 1, 2 and 3
-	 * LINE 3: one byte less, now things are realigned on LANE 0 for LINE 4
-	 *
-	 * I figured this extra complexity was not worth the benefit, but if
-	 * someone really has unfixable mismatch, that would be something to
-	 * investigate.
-	 */
-	for (; fbdiv <= fbdiv_max; fbdiv++) {
-		u32 rem;
-
-		adj_dsi_htotal = (u64)fbdiv * pll_ref_hz * dsi_nlanes *
-				 dpi_htotal;
-
-		/*
-		 * Do the division in 2 steps to avoid an overflow on the
-		 * divider.
-		 */
-		rem = do_div(adj_dsi_htotal, dpi_hz);
-		if (rem)
-			continue;
-
-		rem = do_div(adj_dsi_htotal,
-			     cfg->pll_opdiv * cfg->pll_ipdiv * 2 * 8);
-		if (rem)
-			continue;
-
-		cfg->pll_fbdiv = fbdiv;
-		*dsi_hfp_ext = adj_dsi_htotal - dsi_htotal;
-		break;
-	}
-
-	/* No match, let's just reject the display mode. */
-	if (!cfg->pll_fbdiv)
-		return -EINVAL;
-
-	dlane_bps = DIV_ROUND_DOWN_ULL((u64)dpi_hz * adj_dsi_htotal * 8,
-				       dsi_nlanes * dpi_htotal);
-	cfg->lane_bps = dlane_bps;
-
-	return 0;
-}
-
-static int cdns_dphy_setup_psm(struct cdns_dphy *dphy)
-{
-	unsigned long psm_clk_hz = clk_get_rate(dphy->psm_clk);
-	unsigned long psm_div;
-
-	if (!psm_clk_hz || psm_clk_hz > 100000000)
-		return -EINVAL;
-
-	psm_div = DIV_ROUND_CLOSEST(psm_clk_hz, 1000000);
-	if (dphy->ops->set_psm_div)
-		dphy->ops->set_psm_div(dphy, psm_div);
-
-	return 0;
-}
-
-static void cdns_dphy_set_clk_lane_cfg(struct cdns_dphy *dphy,
-				       enum cdns_dphy_clk_lane_cfg cfg)
-{
-	if (dphy->ops->set_clk_lane_cfg)
-		dphy->ops->set_clk_lane_cfg(dphy, cfg);
-}
-
-static void cdns_dphy_set_pll_cfg(struct cdns_dphy *dphy,
-				  const struct cdns_dphy_cfg *cfg)
-{
-	if (dphy->ops->set_pll_cfg)
-		dphy->ops->set_pll_cfg(dphy, cfg);
-}
-
-static unsigned long cdns_dphy_get_wakeup_time_ns(struct cdns_dphy *dphy)
-{
-	return dphy->ops->get_wakeup_time_ns(dphy);
-}
-
 static unsigned int dpi_to_dsi_timing(unsigned int dpi_timing,
 				      unsigned int dpi_bpp,
 				      unsigned int dsi_pkt_overhead)
@@ -780,17 +545,20 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
 	return 0;
 }
 
-static int cdns_dphy_validate(struct cdns_dsi *dsi,
+static int cdns_dsi_adjust_phy_config(struct cdns_dsi *dsi,
 			      struct cdns_dsi_cfg *dsi_cfg,
-			      struct cdns_dphy_cfg *dphy_cfg,
+			      struct phy_configure_opts_mipi_dphy *phy_cfg,
 			      const struct drm_display_mode *mode,
 			      bool mode_valid_check)
 {
 	struct cdns_dsi_output *output = &dsi->output;
+	unsigned long long dlane_bps;
+	unsigned long adj_dsi_htotal;
 	unsigned long dsi_htotal;
-	unsigned int dsi_hfp_ext = 0;
-
-	int ret;
+	unsigned long dpi_htotal;
+	unsigned long dpi_hz;
+	unsigned int dsi_hfp_ext;
+	unsigned int lanes = output->dev->lanes;
 
 	dsi_htotal = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
 	if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
@@ -799,25 +567,27 @@ static int cdns_dphy_validate(struct cdns_dsi *dsi,
 	dsi_htotal += dsi_cfg->hact;
 	dsi_htotal += dsi_cfg->hfp + DSI_HFP_FRAME_OVERHEAD;
 
-	if (mode_valid_check)
-		ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
-						mode->htotal,
-						mode->clock * 1000,
-						mipi_dsi_pixel_format_to_bpp(output->dev->format),
-						dsi_htotal,
-						output->dev->lanes,
-						&dsi_hfp_ext);
-	else
-		ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
-						mode->crtc_htotal,
-						mipi_dsi_pixel_format_to_bpp(output->dev->format),
-						mode->crtc_clock * 1000,
-						dsi_htotal,
-						output->dev->lanes,
-						&dsi_hfp_ext);
-	if (ret)
-		return ret;
+	/*
+	 * Make sure DSI htotal is aligned on a lane boundary when calculating
+	 * the expected data rate. This is done by extending HFP in case of
+	 * misalignment.
+	 */
+	adj_dsi_htotal = dsi_htotal;
+	if (dsi_htotal % lanes)
+		adj_dsi_htotal += lanes - (dsi_htotal % lanes);
+
+	dpi_hz = (mode_valid_check ? mode->clock : mode->crtc_clock) * 1000;
+	dlane_bps = (unsigned long long)dpi_hz * adj_dsi_htotal;
+
+	/* data rate in bytes/sec is not an integer, refuse the mode. */
+	dpi_htotal = mode_valid_check ? mode->htotal : mode->crtc_htotal;
+	if (do_div(dlane_bps, lanes * dpi_htotal))
+		return -EINVAL;
 
+	/* data rate was in bytes/sec, convert to bits/sec. */
+	phy_cfg->hs_clk_rate = dlane_bps * 8;
+
+	dsi_hfp_ext = adj_dsi_htotal - dsi_htotal;
 	dsi_cfg->hfp += dsi_hfp_ext;
 	dsi_cfg->htotal = dsi_htotal + dsi_hfp_ext;
 
@@ -827,10 +597,10 @@ static int cdns_dphy_validate(struct cdns_dsi *dsi,
 static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
 			       const struct drm_display_mode *mode,
 			       struct cdns_dsi_cfg *dsi_cfg,
-			       struct cdns_dphy_cfg *dphy_cfg,
 			       bool mode_valid_check)
 {
 	struct cdns_dsi_output *output = &dsi->output;
+	struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy;
 	unsigned long dsi_hss_hsa_hse_hbp;
 	unsigned int nlanes = output->dev->lanes;
 	int ret;
@@ -839,7 +609,15 @@ static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
 	if (ret)
 		return ret;
 
-	ret = cdns_dphy_validate(dsi, dsi_cfg, dphy_cfg, mode, mode_valid_check);
+	phy_mipi_dphy_get_default_config(mode->crtc_clock * 1000,
+					 mipi_dsi_pixel_format_to_bpp(output->dev->format),
+					 nlanes, phy_cfg);
+
+	ret = cdns_dsi_adjust_phy_config(dsi, dsi_cfg, phy_cfg, mode, mode_valid_check);
+	if (ret)
+		return ret;
+
+	ret = phy_validate(dsi->dphy, PHY_MODE_MIPI_DPHY, 0, &output->phy_opts);
 	if (ret)
 		return ret;
 
@@ -852,7 +630,7 @@ static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
 	 * is empty before we start a receiving a new line on the DPI
 	 * interface.
 	 */
-	if ((u64)dphy_cfg->lane_bps * mode_to_dpi_hfp(mode) * nlanes <
+	if ((u64)phy_cfg->hs_clk_rate * mode_to_dpi_hfp(mode) * nlanes <
 	    (u64)dsi_hss_hsa_hse_hbp *
 	    (mode_valid_check ? mode->clock : mode->crtc_clock) * 1000)
 		return -EINVAL;
@@ -882,7 +660,6 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge,
 	struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
 	struct cdns_dsi *dsi = input_to_dsi(input);
 	struct cdns_dsi_output *output = &dsi->output;
-	struct cdns_dphy_cfg dphy_cfg;
 	struct cdns_dsi_cfg dsi_cfg;
 	int bpp, ret;
 
@@ -902,7 +679,7 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge,
 	if ((mode->hdisplay * bpp) % 32)
 		return MODE_H_ILLEGAL;
 
-	ret = cdns_dsi_check_conf(dsi, mode, &dsi_cfg, &dphy_cfg, true);
+	ret = cdns_dsi_check_conf(dsi, mode, &dsi_cfg, true);
 	if (ret)
 		return MODE_BAD;
 
@@ -925,9 +702,9 @@ static void cdns_dsi_bridge_disable(struct drm_bridge *bridge)
 	pm_runtime_put(dsi->base.dev);
 }
 
-static void cdns_dsi_hs_init(struct cdns_dsi *dsi,
-			     const struct cdns_dphy_cfg *dphy_cfg)
+static void cdns_dsi_hs_init(struct cdns_dsi *dsi)
 {
+	struct cdns_dsi_output *output = &dsi->output;
 	u32 status;
 
 	/*
@@ -938,30 +715,10 @@ static void cdns_dsi_hs_init(struct cdns_dsi *dsi,
 	       DPHY_CMN_PDN | DPHY_PLL_PDN,
 	       dsi->regs + MCTL_DPHY_CFG0);
 
-	/*
-	 * Configure the internal PSM clk divider so that the DPHY has a
-	 * 1MHz clk (or something close).
-	 */
-	WARN_ON_ONCE(cdns_dphy_setup_psm(dsi->dphy));
-
-	/*
-	 * Configure attach clk lanes to data lanes: the DPHY has 2 clk lanes
-	 * and 8 data lanes, each clk lane can be attache different set of
-	 * data lanes. The 2 groups are named 'left' and 'right', so here we
-	 * just say that we want the 'left' clk lane to drive the 'left' data
-	 * lanes.
-	 */
-	cdns_dphy_set_clk_lane_cfg(dsi->dphy, DPHY_CLK_CFG_LEFT_DRIVES_LEFT);
-
-	/*
-	 * Configure the DPHY PLL that will be used to generate the TX byte
-	 * clk.
-	 */
-	cdns_dphy_set_pll_cfg(dsi->dphy, dphy_cfg);
-
-	/* Start TX state machine. */
-	writel(DPHY_CMN_SSM_EN | DPHY_CMN_TX_MODE_EN,
-	       dsi->dphy->regs + DPHY_CMN_SSM);
+	phy_init(dsi->dphy);
+	phy_set_mode(dsi->dphy, PHY_MODE_MIPI_DPHY);
+	phy_configure(dsi->dphy, &output->phy_opts);
+	phy_power_on(dsi->dphy);
 
 	/* Activate the PLL and wait until it's locked. */
 	writel(PLL_LOCKED, dsi->regs + MCTL_MAIN_STS_CLR);
@@ -971,7 +728,7 @@ static void cdns_dsi_hs_init(struct cdns_dsi *dsi,
 					status & PLL_LOCKED, 100, 100));
 	/* De-assert data and clock reset lines. */
 	writel(DPHY_CMN_PSO | DPHY_ALL_D_PDN | DPHY_C_PDN | DPHY_CMN_PDN |
-	       DPHY_D_RSTB(dphy_cfg->nlanes) | DPHY_C_RSTB,
+	       DPHY_D_RSTB(output->dev->lanes) | DPHY_C_RSTB,
 	       dsi->regs + MCTL_DPHY_CFG0);
 }
 
@@ -1017,7 +774,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge)
 	struct cdns_dsi *dsi = input_to_dsi(input);
 	struct cdns_dsi_output *output = &dsi->output;
 	struct drm_display_mode *mode;
-	struct cdns_dphy_cfg dphy_cfg;
+	struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy;
 	unsigned long tx_byte_period;
 	struct cdns_dsi_cfg dsi_cfg;
 	u32 tmp, reg_wakeup, div;
@@ -1030,9 +787,9 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge)
 	bpp = mipi_dsi_pixel_format_to_bpp(output->dev->format);
 	nlanes = output->dev->lanes;
 
-	WARN_ON_ONCE(cdns_dsi_check_conf(dsi, mode, &dsi_cfg, &dphy_cfg, false));
+	WARN_ON_ONCE(cdns_dsi_check_conf(dsi, mode, &dsi_cfg, false));
 
-	cdns_dsi_hs_init(dsi, &dphy_cfg);
+	cdns_dsi_hs_init(dsi);
 	cdns_dsi_init_link(dsi);
 
 	writel(HBP_LEN(dsi_cfg.hbp) | HSA_LEN(dsi_cfg.hsa),
@@ -1068,9 +825,8 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge)
 		tmp -= DIV_ROUND_UP(DSI_EOT_PKT_SIZE, nlanes);
 
 	tx_byte_period = DIV_ROUND_DOWN_ULL((u64)NSEC_PER_SEC * 8,
-					    dphy_cfg.lane_bps);
-	reg_wakeup = cdns_dphy_get_wakeup_time_ns(dsi->dphy) /
-		     tx_byte_period;
+					    phy_cfg->hs_clk_rate);
+	reg_wakeup = (phy_cfg->hs_prepare + phy_cfg->hs_zero) / tx_byte_period;
 	writel(REG_WAKEUP_TIME(reg_wakeup) | REG_LINE_DURATION(tmp),
 	       dsi->regs + VID_DPHY_TIME);
 
@@ -1384,8 +1140,6 @@ static int __maybe_unused cdns_dsi_resume(struct device *dev)
 	reset_control_deassert(dsi->dsi_p_rst);
 	clk_prepare_enable(dsi->dsi_p_clk);
 	clk_prepare_enable(dsi->dsi_sys_clk);
-	clk_prepare_enable(dsi->dphy->psm_clk);
-	clk_prepare_enable(dsi->dphy->pll_ref_clk);
 
 	return 0;
 }
@@ -1394,8 +1148,6 @@ static int __maybe_unused cdns_dsi_suspend(struct device *dev)
 {
 	struct cdns_dsi *dsi = dev_get_drvdata(dev);
 
-	clk_disable_unprepare(dsi->dphy->pll_ref_clk);
-	clk_disable_unprepare(dsi->dphy->psm_clk);
 	clk_disable_unprepare(dsi->dsi_sys_clk);
 	clk_disable_unprepare(dsi->dsi_p_clk);
 	reset_control_assert(dsi->dsi_p_rst);
@@ -1406,121 +1158,6 @@ static int __maybe_unused cdns_dsi_suspend(struct device *dev)
 static UNIVERSAL_DEV_PM_OPS(cdns_dsi_pm_ops, cdns_dsi_suspend, cdns_dsi_resume,
 			    NULL);
 
-static unsigned long cdns_dphy_ref_get_wakeup_time_ns(struct cdns_dphy *dphy)
-{
-	/* Default wakeup time is 800 ns (in a simulated environment). */
-	return 800;
-}
-
-static void cdns_dphy_ref_set_pll_cfg(struct cdns_dphy *dphy,
-				      const struct cdns_dphy_cfg *cfg)
-{
-	u32 fbdiv_low, fbdiv_high;
-
-	fbdiv_low = (cfg->pll_fbdiv / 4) - 2;
-	fbdiv_high = cfg->pll_fbdiv - fbdiv_low - 2;
-
-	writel(DPHY_CMN_IPDIV_FROM_REG | DPHY_CMN_OPDIV_FROM_REG |
-	       DPHY_CMN_IPDIV(cfg->pll_ipdiv) |
-	       DPHY_CMN_OPDIV(cfg->pll_opdiv),
-	       dphy->regs + DPHY_CMN_OPIPDIV);
-	writel(DPHY_CMN_FBDIV_FROM_REG |
-	       DPHY_CMN_FBDIV_VAL(fbdiv_low, fbdiv_high),
-	       dphy->regs + DPHY_CMN_FBDIV);
-	writel(DPHY_CMN_PWM_HIGH(6) | DPHY_CMN_PWM_LOW(0x101) |
-	       DPHY_CMN_PWM_DIV(0x8),
-	       dphy->regs + DPHY_CMN_PWM);
-}
-
-static void cdns_dphy_ref_set_psm_div(struct cdns_dphy *dphy, u8 div)
-{
-	writel(DPHY_PSM_CFG_FROM_REG | DPHY_PSM_CLK_DIV(div),
-	       dphy->regs + DPHY_PSM_CFG);
-}
-
-/*
- * This is the reference implementation of DPHY hooks. Specific integration of
- * this IP may have to re-implement some of them depending on how they decided
- * to wire things in the SoC.
- */
-static const struct cdns_dphy_ops ref_dphy_ops = {
-	.get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
-	.set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
-	.set_psm_div = cdns_dphy_ref_set_psm_div,
-};
-
-static const struct of_device_id cdns_dphy_of_match[] = {
-	{ .compatible = "cdns,dphy", .data = &ref_dphy_ops },
-	{ /* sentinel */ },
-};
-
-static struct cdns_dphy *cdns_dphy_probe(struct platform_device *pdev)
-{
-	const struct of_device_id *match;
-	struct cdns_dphy *dphy;
-	struct of_phandle_args args;
-	struct resource res;
-	int ret;
-
-	ret = of_parse_phandle_with_args(pdev->dev.of_node, "phys",
-					 "#phy-cells", 0, &args);
-	if (ret)
-		return ERR_PTR(-ENOENT);
-
-	match = of_match_node(cdns_dphy_of_match, args.np);
-	if (!match || !match->data)
-		return ERR_PTR(-EINVAL);
-
-	dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
-	if (!dphy)
-		return ERR_PTR(-ENOMEM);
-
-	dphy->ops = match->data;
-
-	ret = of_address_to_resource(args.np, 0, &res);
-	if (ret)
-		return ERR_PTR(ret);
-
-	dphy->regs = devm_ioremap_resource(&pdev->dev, &res);
-	if (IS_ERR(dphy->regs))
-		return ERR_CAST(dphy->regs);
-
-	dphy->psm_clk = of_clk_get_by_name(args.np, "psm");
-	if (IS_ERR(dphy->psm_clk))
-		return ERR_CAST(dphy->psm_clk);
-
-	dphy->pll_ref_clk = of_clk_get_by_name(args.np, "pll_ref");
-	if (IS_ERR(dphy->pll_ref_clk)) {
-		ret = PTR_ERR(dphy->pll_ref_clk);
-		goto err_put_psm_clk;
-	}
-
-	if (dphy->ops->probe) {
-		ret = dphy->ops->probe(dphy);
-		if (ret)
-			goto err_put_pll_ref_clk;
-	}
-
-	return dphy;
-
-err_put_pll_ref_clk:
-	clk_put(dphy->pll_ref_clk);
-
-err_put_psm_clk:
-	clk_put(dphy->psm_clk);
-
-	return ERR_PTR(ret);
-}
-
-static void cdns_dphy_remove(struct cdns_dphy *dphy)
-{
-	if (dphy->ops->remove)
-		dphy->ops->remove(dphy);
-
-	clk_put(dphy->pll_ref_clk);
-	clk_put(dphy->psm_clk);
-}
-
 static int cdns_dsi_drm_probe(struct platform_device *pdev)
 {
 	struct cdns_dsi *dsi;
@@ -1559,13 +1196,13 @@ static int cdns_dsi_drm_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	dsi->dphy = cdns_dphy_probe(pdev);
+	dsi->dphy = devm_phy_get(&pdev->dev, "dphy");
 	if (IS_ERR(dsi->dphy))
 		return PTR_ERR(dsi->dphy);
 
 	ret = clk_prepare_enable(dsi->dsi_p_clk);
 	if (ret)
-		goto err_remove_dphy;
+		return ret;
 
 	val = readl(dsi->regs + ID_REG);
 	if (REV_VENDOR_ID(val) != 0xcad) {
@@ -1623,9 +1260,6 @@ static int cdns_dsi_drm_probe(struct platform_device *pdev)
 err_disable_pclk:
 	clk_disable_unprepare(dsi->dsi_p_clk);
 
-err_remove_dphy:
-	cdns_dphy_remove(dsi->dphy);
-
 	return ret;
 }
 
@@ -1635,7 +1269,6 @@ static int cdns_dsi_drm_remove(struct platform_device *pdev)
 
 	mipi_dsi_host_unregister(&dsi->base);
 	pm_runtime_disable(&pdev->dev);
-	cdns_dphy_remove(dsi->dphy);
 
 	return 0;
 }
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 10/10] drm/bridge: cdns: Convert to phy framework
  2018-12-07 13:55 ` [PATCH v3 10/10] drm/bridge: cdns: Convert to phy framework Maxime Ripard
@ 2018-12-09  4:37   ` kbuild test robot
  2018-12-20 10:52   ` kbuild test robot
  1 sibling, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2018-12-09  4:37 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: kbuild-all, Kishon Vijay Abraham I, Boris Brezillon,
	Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

[-- Attachment #1: Type: text/plain, Size: 2711 bytes --]

Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on phy/next]
[cannot apply to v4.20-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/phy-Add-MIPI-D-PHY-mode/20181208-034527
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
config: i386-randconfig-s0-12051035 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/bridge/cdns-dsi.o: In function `cdns_dsi_check_conf':
>> drivers/gpu/drm/bridge/cdns-dsi.c:612: undefined reference to `phy_mipi_dphy_get_default_config'

vim +612 drivers/gpu/drm/bridge/cdns-dsi.c

   596	
   597	static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
   598				       const struct drm_display_mode *mode,
   599				       struct cdns_dsi_cfg *dsi_cfg,
   600				       bool mode_valid_check)
   601	{
   602		struct cdns_dsi_output *output = &dsi->output;
   603		struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy;
   604		unsigned long dsi_hss_hsa_hse_hbp;
   605		unsigned int nlanes = output->dev->lanes;
   606		int ret;
   607	
   608		ret = cdns_dsi_mode2cfg(dsi, mode, dsi_cfg, mode_valid_check);
   609		if (ret)
   610			return ret;
   611	
 > 612		phy_mipi_dphy_get_default_config(mode->crtc_clock * 1000,
   613						 mipi_dsi_pixel_format_to_bpp(output->dev->format),
   614						 nlanes, phy_cfg);
   615	
   616		ret = cdns_dsi_adjust_phy_config(dsi, dsi_cfg, phy_cfg, mode, mode_valid_check);
   617		if (ret)
   618			return ret;
   619	
   620		ret = phy_validate(dsi->dphy, PHY_MODE_MIPI_DPHY, 0, &output->phy_opts);
   621		if (ret)
   622			return ret;
   623	
   624		dsi_hss_hsa_hse_hbp = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
   625		if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
   626			dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
   627	
   628		/*
   629		 * Make sure DPI(HFP) > DSI(HSS+HSA+HSE+HBP) to guarantee that the FIFO
   630		 * is empty before we start a receiving a new line on the DPI
   631		 * interface.
   632		 */
   633		if ((u64)phy_cfg->hs_clk_rate * mode_to_dpi_hfp(mode) * nlanes <
   634		    (u64)dsi_hss_hsa_hse_hbp *
   635		    (mode_valid_check ? mode->clock : mode->crtc_clock) * 1000)
   636			return -EINVAL;
   637	
   638		return 0;
   639	}
   640	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30580 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 02/10] phy: Add configuration interface
  2018-12-07 13:55 ` [PATCH v3 02/10] phy: Add configuration interface Maxime Ripard
@ 2018-12-13 20:42   ` sakari.ailus
  0 siblings, 0 replies; 20+ messages in thread
From: sakari.ailus @ 2018-12-13 20:42 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Kishon Vijay Abraham I, Boris Brezillon, Thomas Petazzoni,
	Laurent Pinchart, linux-media, Archit Taneja, Andrzej Hajda,
	Chen-Yu Tsai, linux-kernel, dri-devel, linux-arm-kernel,
	Krzysztof Witos, Rafal Ciepiela

Hi Maxime,

On Fri, Dec 07, 2018 at 02:55:29PM +0100, Maxime Ripard wrote:
> The phy framework is only allowing to configure the power state of the PHY
> using the init and power_on hooks, and their power_off and exit
> counterparts.
> 
> While it works for most, simple, PHYs supported so far, some more advanced
> PHYs need some configuration depending on runtime parameters. These PHYs
> have been supported by a number of means already, often by using ad-hoc
> drivers in their consumer drivers.
> 
> That doesn't work too well however, when a consumer device needs to deal
> with multiple PHYs, or when multiple consumers need to deal with the same
> PHY (a DSI driver and a CSI driver for example).
> 
> So we'll add a new interface, through two funtions, phy_validate and
> phy_configure. The first one will allow to check that a current
> configuration, for a given mode, is applicable. It will also allow the PHY
> driver to tune the settings given as parameters as it sees fit.
> 
> phy_configure will actually apply that configuration in the phy itself.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/phy/phy-core.c  | 64 ++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/phy/phy.h | 58 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 122 insertions(+)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index df3d4ba516ab..19b05e824ee4 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -408,6 +408,70 @@ int phy_calibrate(struct phy *phy)
>  EXPORT_SYMBOL_GPL(phy_calibrate);
>  
>  /**
> + * phy_configure() - Changes the phy parameters
> + * @phy: the phy returned by phy_get()
> + * @opts: New configuration to apply
> + *
> + * Used to change the PHY parameters. phy_init() must have been called
> + * on the phy. The configuration will be applied on the current phy
> + * mode, that can be changed using phy_set_mode().
> + *
> + * Returns: 0 if successful, an negative error code otherwise
> + */
> +int phy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> +	int ret;
> +
> +	if (!phy)
> +		return -EINVAL;
> +
> +	if (!phy->ops->configure)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&phy->mutex);
> +	ret = phy->ops->configure(phy, opts);
> +	mutex_unlock(&phy->mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_configure);
> +
> +/**
> + * phy_validate() - Checks the phy parameters
> + * @phy: the phy returned by phy_get()
> + * @mode: phy_mode the configuration is applicable to.
> + * @submode: PHY submode the configuration is applicable to.
> + * @opts: Configuration to check
> + *
> + * Used to check that the current set of parameters can be handled by
> + * the phy. Implementations are free to tune the parameters passed as
> + * arguments if needed by some implementation detail or
> + * constraints. It will not change any actual configuration of the
> + * PHY, so calling it as many times as deemed fit will have no side
> + * effect.
> + *
> + * Returns: 0 if successful, an negative error code otherwise
> + */
> +int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
> +		 union phy_configure_opts *opts)
> +{
> +	int ret;
> +
> +	if (!phy)
> +		return -EINVAL;
> +
> +	if (!phy->ops->validate)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&phy->mutex);
> +	ret = phy->ops->validate(phy, mode, submode, opts);
> +	mutex_unlock(&phy->mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_validate);
> +
> +/**
>   * _of_phy_get() - lookup and obtain a reference to a phy by phandle
>   * @np: device_node for which to get the phy
>   * @index: the index of the phy
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 453f21834685..04476c026b5a 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -43,6 +43,12 @@ enum phy_mode {
>  };
>  
>  /**
> + * union phy_configure_opts - Opaque generic phy configuration
> + */
> +union phy_configure_opts {
> +};
> +
> +/**
>   * struct phy_ops - set of function pointers for performing phy operations
>   * @init: operation to be performed for initializing phy
>   * @exit: operation to be performed while exiting
> @@ -59,6 +65,37 @@ struct phy_ops {
>  	int	(*power_on)(struct phy *phy);
>  	int	(*power_off)(struct phy *phy);
>  	int	(*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
> +
> +	/**
> +	 * @configure:
> +	 *
> +	 * Optional.
> +	 *
> +	 * Used to change the PHY parameters. phy_init() must have
> +	 * been called on the phy.
> +	 *
> +	 * Returns: 0 if successful, an negative error code otherwise
> +	 */
> +	int	(*configure)(struct phy *phy, union phy_configure_opts *opts);
> +
> +	/**
> +	 * @validate:
> +	 *
> +	 * Optional.
> +	 *
> +	 * Used to check that the current set of parameters can be
> +	 * handled by the phy. Implementations are free to tune the
> +	 * parameters passed as arguments if needed by some
> +	 * implementation detail or constraints. It must not change
> +	 * any actual configuration of the PHY, so calling it as many
> +	 * times as deemed fit by the consumer must have no side
> +	 * effect.
> +	 *
> +	 * Returns: 0 if the configuration can be applied, an negative
> +	 * error code otherwise

I think it'd be good to document which error codes can be returned and what
do they mean. I'm no expert in the PHY framework, but it seems the caller
would need to be able to cope with -EOPNOTSUPP for instance, which may not
necessarily signal an error at all.

Would it be enough to document the PHY framework interface functions rather
than the ops at this level? There seems to be a non-zero chances of the two
descriptions of diverging. Just an idea.

> +	 */
> +	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
> +			    union phy_configure_opts *opts);
>  	int	(*reset)(struct phy *phy);
>  	int	(*calibrate)(struct phy *phy);
>  	struct module *owner;
> @@ -165,6 +202,9 @@ int phy_power_off(struct phy *phy);
>  int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
>  #define phy_set_mode(phy, mode) \
>  	phy_set_mode_ext(phy, mode, 0)
> +int phy_configure(struct phy *phy, union phy_configure_opts *opts);
> +int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
> +		 union phy_configure_opts *opts);
>  
>  static inline enum phy_mode phy_get_mode(struct phy *phy)
>  {
> @@ -309,6 +349,24 @@ static inline int phy_calibrate(struct phy *phy)
>  	return -ENOSYS;
>  }
>  
> +static inline int phy_configure(struct phy *phy,
> +				union phy_configure_opts *opts)
> +{
> +	if (!phy)
> +		return 0;
> +
> +	return -ENOSYS;
> +}
> +
> +static inline int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
> +			       union phy_configure_opts *opts)
> +{
> +	if (!phy)
> +		return 0;
> +
> +	return -ENOSYS;
> +}
> +
>  static inline int phy_get_bus_width(struct phy *phy)
>  {
>  	return -ENOSYS;

-- 
Kind regar,

Sakari Ailus

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
  2018-12-07 13:55 ` [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options Maxime Ripard
@ 2018-12-13 20:49   ` sakari.ailus
  2018-12-17 15:49     ` Maxime Ripard
  2018-12-13 20:54   ` sakari.ailus
  1 sibling, 1 reply; 20+ messages in thread
From: sakari.ailus @ 2018-12-13 20:49 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Kishon Vijay Abraham I, Boris Brezillon, Thomas Petazzoni,
	Laurent Pinchart, linux-media, Archit Taneja, Andrzej Hajda,
	Chen-Yu Tsai, linux-kernel, dri-devel, linux-arm-kernel,
	Krzysztof Witos, Rafal Ciepiela

Hi Maxime,

On Fri, Dec 07, 2018 at 02:55:30PM +0100, Maxime Ripard wrote:
> Now that we have some infrastructure for it, allow the MIPI D-PHY phy's to
> be configured through the generic functions through a custom structure
> added to the generic union.
> 
> The parameters added here are the ones defined in the MIPI D-PHY spec, plus
> the number of lanes in use. The current set of parameters should cover all
> the potential users.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  include/linux/phy/phy-mipi-dphy.h | 279 +++++++++++++++++++++++++++++++-
>  include/linux/phy/phy.h           |   6 +-
>  2 files changed, 285 insertions(+)
>  create mode 100644 include/linux/phy/phy-mipi-dphy.h
> 
> diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h
> new file mode 100644
> index 000000000000..29bf94db88ad
> --- /dev/null
> +++ b/include/linux/phy/phy-mipi-dphy.h
> @@ -0,0 +1,279 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Cadence Design Systems Inc.
> + */
> +
> +#ifndef __PHY_MIPI_DPHY_H_
> +#define __PHY_MIPI_DPHY_H_
> +
> +#include <video/videomode.h>

Where do you need this? Please remove.

> +
> +/**
> + * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
> + *
> + * This structure is used to represent the configuration state of a
> + * MIPI D-PHY phy.
> + */
> +struct phy_configure_opts_mipi_dphy {
> +	/**
> +	 * @clk_miss:
> +	 *
> +	 * Timeout, in picoseconds, for receiver to detect absence of
> +	 * Clock transitions and disable the Clock Lane HS-RX.
> +	 *
> +	 * Maximum value: 60000 ps
> +	 */
> +	unsigned int		clk_miss;
> +
> +	/**
> +	 * @clk_post:
> +	 *
> +	 * Time, in picoseconds, that the transmitter continues to
> +	 * send HS clock after the last associated Data Lane has
> +	 * transitioned to LP Mode. Interval is defined as the period
> +	 * from the end of @hs_trail to the beginning of @clk_trail.
> +	 *
> +	 * Minimum value: 60000 ps + 52 * @hs_clk_rate period in ps
> +	 */
> +	unsigned int		clk_post;
> +
> +	/**
> +	 * @clk_pre:
> +	 *
> +	 * Time, in UI, that the HS clock shall be driven by
> +	 * the transmitter prior to any associated Data Lane beginning
> +	 * the transition from LP to HS mode.
> +	 *
> +	 * Minimum value: 8 UI
> +	 */
> +	unsigned int		clk_pre;
> +
> +	/**
> +	 * @clk_prepare:
> +	 *
> +	 * Time, in picoseconds, that the transmitter drives the Clock
> +	 * Lane LP-00 Line state immediately before the HS-0 Line
> +	 * state starting the HS transmission.
> +	 *
> +	 * Minimum value: 38000 ps
> +	 * Maximum value: 95000 ps
> +	 */
> +	unsigned int		clk_prepare;
> +
> +	/**
> +	 * @clk_settle:
> +	 *
> +	 * Time interval, in picoseconds, during which the HS receiver
> +	 * should ignore any Clock Lane HS transitions, starting from
> +	 * the beginning of @clk_prepare.
> +	 *
> +	 * Minimum value: 95000 ps
> +	 * Maximum value: 300000 ps
> +	 */
> +	unsigned int		clk_settle;
> +
> +	/**
> +	 * @clk_term_en:
> +	 *
> +	 * Time, in picoseconds, for the Clock Lane receiver to enable
> +	 * the HS line termination.
> +	 *
> +	 * Maximum value: 38000 ps
> +	 */
> +	unsigned int		clk_term_en;
> +
> +	/**
> +	 * @clk_trail:
> +	 *
> +	 * Time, in picoseconds, that the transmitter drives the HS-0
> +	 * state after the last payload clock bit of a HS transmission
> +	 * burst.
> +	 *
> +	 * Minimum value: 60000 ps
> +	 */
> +	unsigned int		clk_trail;
> +
> +	/**
> +	 * @clk_zero:
> +	 *
> +	 * Time, in picoseconds, that the transmitter drives the HS-0
> +	 * state prior to starting the Clock.
> +	 */
> +	unsigned int		clk_zero;
> +
> +	/**
> +	 * @d_term_en:
> +	 *
> +	 * Time, in picoseconds, for the Data Lane receiver to enable
> +	 * the HS line termination.
> +	 *
> +	 * Maximum value: 35000 ps + 4 * @hs_clk_rate period in ps
> +	 */
> +	unsigned int		d_term_en;
> +
> +	/**
> +	 * @eot:
> +	 *
> +	 * Transmitted time interval, in picoseconds, from the start
> +	 * of @hs_trail or @clk_trail, to the start of the LP- 11
> +	 * state following a HS burst.
> +	 *
> +	 * Maximum value: 105000 ps + 12 * @hs_clk_rate period in ps
> +	 */
> +	unsigned int		eot;
> +
> +	/**
> +	 * @hs_exit:
> +	 *
> +	 * Time, in picoseconds, that the transmitter drives LP-11
> +	 * following a HS burst.
> +	 *
> +	 * Minimum value: 100000 ps
> +	 */
> +	unsigned int		hs_exit;
> +
> +	/**
> +	 * @hs_prepare:
> +	 *
> +	 * Time, in picoseconds, that the transmitter drives the Data
> +	 * Lane LP-00 Line state immediately before the HS-0 Line
> +	 * state starting the HS transmission.
> +	 *
> +	 * Minimum value: 40000 ps + 4 * @hs_clk_rate period in ps
> +	 * Maximum value: 85000 ps + 6 * @hs_clk_rate period in ps
> +	 */
> +	unsigned int		hs_prepare;
> +
> +	/**
> +	 * @hs_settle:
> +	 *
> +	 * Time interval, in picoseconds, during which the HS receiver
> +	 * shall ignore any Data Lane HS transitions, starting from
> +	 * the beginning of @hs_prepare.
> +	 *
> +	 * Minimum value: 85000 ps + 6 * @hs_clk_rate period in ps
> +	 * Maximum value: 145000 ps + 10 * @hs_clk_rate period in ps
> +	 */
> +	unsigned int		hs_settle;
> +
> +	/**
> +	 * @hs_skip:
> +	 *
> +	 * Time interval, in picoseconds, during which the HS-RX
> +	 * should ignore any transitions on the Data Lane, following a
> +	 * HS burst. The end point of the interval is defined as the
> +	 * beginning of the LP-11 state following the HS burst.
> +	 *
> +	 * Minimum value: 40000 ps
> +	 * Maximum value: 55000 ps + 4 * @hs_clk_rate period in ps
> +	 */
> +	unsigned int		hs_skip;
> +
> +	/**
> +	 * @hs_trail:
> +	 *
> +	 * Time, in picoseconds, that the transmitter drives the
> +	 * flipped differential state after last payload data bit of a
> +	 * HS transmission burst
> +	 *
> +	 * Minimum value: max(8 * @hs_clk_rate period in ps,
> +	 *		      60000 ps + 4 * @hs_clk_rate period in ps)
> +	 */
> +	unsigned int		hs_trail;
> +
> +	/**
> +	 * @hs_zero:
> +	 *
> +	 * Time, in picoseconds, that the transmitter drives the HS-0
> +	 * state prior to transmitting the Sync sequence.
> +	 */
> +	unsigned int		hs_zero;
> +
> +	/**
> +	 * @init:
> +	 *
> +	 * Time, in picoseconds for the initialization period to
> +	 * complete.
> +	 *
> +	 * Minimum value: 100000000 ps
> +	 */
> +	unsigned int		init;
> +
> +	/**
> +	 * @lpx:
> +	 *
> +	 * Transmitted length, in picoseconds, of any Low-Power state
> +	 * period.
> +	 *
> +	 * Minimum value: 50000 ps
> +	 */
> +	unsigned int		lpx;
> +
> +	/**
> +	 * @ta_get:
> +	 *
> +	 * Time, in picoseconds, that the new transmitter drives the
> +	 * Bridge state (LP-00) after accepting control during a Link
> +	 * Turnaround.
> +	 *
> +	 * Value: 5 * @lpx
> +	 */
> +	unsigned int		ta_get;
> +
> +	/**
> +	 * @ta_go:
> +	 *
> +	 * Time, in picoseconds, that the transmitter drives the
> +	 * Bridge state (LP-00) before releasing control during a Link
> +	 * Turnaround.
> +	 *
> +	 * Value: 4 * @lpx
> +	 */
> +	unsigned int		ta_go;
> +
> +	/**
> +	 * @ta_sure:
> +	 *
> +	 * Time, in picoseconds, that the new transmitter waits after
> +	 * the LP-10 state before transmitting the Bridge state
> +	 * (LP-00) during a Link Turnaround.
> +	 *
> +	 * Minimum value: @lpx
> +	 * Maximum value: 2 * @lpx
> +	 */
> +	unsigned int		ta_sure;
> +
> +	/**
> +	 * @wakeup:
> +	 *
> +	 * Time, in picoseconds, that a transmitter drives a Mark-1
> +	 * state prior to a Stop state in order to initiate an exit
> +	 * from ULPS.
> +	 *
> +	 * Minimum value: 1000000000 ps
> +	 */
> +	unsigned int		wakeup;
> +
> +	/**
> +	 * @hs_clk_rate:
> +	 *
> +	 * Clock rate, in Hertz, of the high-speed clock.
> +	 */
> +	unsigned long		hs_clk_rate;
> +
> +	/**
> +	 * @lp_clk_rate:
> +	 *
> +	 * Clock rate, in Hertz, of the low-power clock.
> +	 */
> +	unsigned long		lp_clk_rate;
> +
> +	/**
> +	 * @lanes:
> +	 *
> +	 * Number of active data lanes used for the transmissions.

Could you add that these are the first "lanes" number of lanes from what
are available?

With these addressed,

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> +	 */
> +	unsigned char		lanes;
> +};
> +
> +#endif /* __PHY_MIPI_DPHY_H_ */
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 04476c026b5a..1fdefadf150a 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -20,6 +20,8 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
>  
> +#include <linux/phy/phy-mipi-dphy.h>
> +
>  struct phy;
>  
>  enum phy_mode {
> @@ -44,8 +46,12 @@ enum phy_mode {
>  
>  /**
>   * union phy_configure_opts - Opaque generic phy configuration
> + *
> + * @mipi_dphy:	Configuration set applicable for phys supporting
> + *		the MIPI_DPHY phy mode.
>   */
>  union phy_configure_opts {
> +	struct phy_configure_opts_mipi_dphy	mipi_dphy;
>  };
>  
>  /**
> -- 
> git-series 0.9.1

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
  2018-12-07 13:55 ` [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options Maxime Ripard
  2018-12-13 20:49   ` sakari.ailus
@ 2018-12-13 20:54   ` sakari.ailus
  2018-12-13 20:55     ` sakari.ailus
  1 sibling, 1 reply; 20+ messages in thread
From: sakari.ailus @ 2018-12-13 20:54 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Kishon Vijay Abraham I, Boris Brezillon, Thomas Petazzoni,
	Laurent Pinchart, linux-media, Archit Taneja, Andrzej Hajda,
	Chen-Yu Tsai, linux-kernel, dri-devel, linux-arm-kernel,
	Krzysztof Witos, Rafal Ciepiela

Hi Maxime,

One more small note.

On Fri, Dec 07, 2018 at 02:55:30PM +0100, Maxime Ripard wrote:

...

> +	/**
> +	 * @wakeup:
> +	 *
> +	 * Time, in picoseconds, that a transmitter drives a Mark-1
> +	 * state prior to a Stop state in order to initiate an exit
> +	 * from ULPS.
> +	 *
> +	 * Minimum value: 1000000000 ps
> +	 */
> +	unsigned int		wakeup;

This is very close to the higher limit of the type's value range. How about
using ns or µs for this one?

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
  2018-12-13 20:54   ` sakari.ailus
@ 2018-12-13 20:55     ` sakari.ailus
  0 siblings, 0 replies; 20+ messages in thread
From: sakari.ailus @ 2018-12-13 20:55 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Kishon Vijay Abraham I, Boris Brezillon, Thomas Petazzoni,
	Laurent Pinchart, linux-media, Archit Taneja, Andrzej Hajda,
	Chen-Yu Tsai, linux-kernel, dri-devel, linux-arm-kernel,
	Krzysztof Witos, Rafal Ciepiela

On Thu, Dec 13, 2018 at 10:54:28PM +0200, sakari.ailus@iki.fi wrote:
> Hi Maxime,
> 
> One more small note.
> 
> On Fri, Dec 07, 2018 at 02:55:30PM +0100, Maxime Ripard wrote:
> 
> ...
> 
> > +	/**
> > +	 * @wakeup:
> > +	 *
> > +	 * Time, in picoseconds, that a transmitter drives a Mark-1
> > +	 * state prior to a Stop state in order to initiate an exit
> > +	 * from ULPS.
> > +	 *
> > +	 * Minimum value: 1000000000 ps
> > +	 */
> > +	unsigned int		wakeup;
> 
> This is very close to the higher limit of the type's value range. How about
> using ns or µs for this one?

Same for init actually.

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
  2018-12-13 20:49   ` sakari.ailus
@ 2018-12-17 15:49     ` Maxime Ripard
  2018-12-17 20:20       ` sakari.ailus
  0 siblings, 1 reply; 20+ messages in thread
From: Maxime Ripard @ 2018-12-17 15:49 UTC (permalink / raw)
  To: sakari.ailus
  Cc: Kishon Vijay Abraham I, Boris Brezillon, Thomas Petazzoni,
	Laurent Pinchart, linux-media, Archit Taneja, Andrzej Hajda,
	Chen-Yu Tsai, linux-kernel, dri-devel, linux-arm-kernel,
	Krzysztof Witos, Rafal Ciepiela

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

Hi Sakari,

Thanks for your feedback.

On Thu, Dec 13, 2018 at 10:49:28PM +0200, sakari.ailus@iki.fi wrote:
> > +	/**
> > +	 * @lanes:
> > +	 *
> > +	 * Number of active data lanes used for the transmissions.
> 
> Could you add that these are the first "lanes" number of lanes from what
> are available?

I'm not quite sure I understood this part though, what did you mean?

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
  2018-12-17 15:49     ` Maxime Ripard
@ 2018-12-17 20:20       ` sakari.ailus
  2018-12-19 15:40         ` Maxime Ripard
  0 siblings, 1 reply; 20+ messages in thread
From: sakari.ailus @ 2018-12-17 20:20 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Kishon Vijay Abraham I, Boris Brezillon, Thomas Petazzoni,
	Laurent Pinchart, linux-media, Archit Taneja, Andrzej Hajda,
	Chen-Yu Tsai, linux-kernel, dri-devel, linux-arm-kernel,
	Krzysztof Witos, Rafal Ciepiela

Hi Maxime,

On Mon, Dec 17, 2018 at 04:49:21PM +0100, Maxime Ripard wrote:
> Hi Sakari,
> 
> Thanks for your feedback.
> 
> On Thu, Dec 13, 2018 at 10:49:28PM +0200, sakari.ailus@iki.fi wrote:
> > > +	/**
> > > +	 * @lanes:
> > > +	 *
> > > +	 * Number of active data lanes used for the transmissions.
> > 
> > Could you add that these are the first "lanes" number of lanes from what
> > are available?
> 
> I'm not quite sure I understood this part though, what did you mean?

A number of lanes are routed between the two devices on hardware, and this
field is specifying how many of them are in use. In order for the bus to
function, both ends need to be in agreement on which of these lanes are
actually being used. The current practice I've seen without exceptions is
that these are the first n lanes.

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
  2018-12-17 20:20       ` sakari.ailus
@ 2018-12-19 15:40         ` Maxime Ripard
  0 siblings, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2018-12-19 15:40 UTC (permalink / raw)
  To: sakari.ailus
  Cc: Kishon Vijay Abraham I, Boris Brezillon, Thomas Petazzoni,
	Laurent Pinchart, linux-media, Archit Taneja, Andrzej Hajda,
	Chen-Yu Tsai, linux-kernel, dri-devel, linux-arm-kernel,
	Krzysztof Witos, Rafal Ciepiela

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

On Mon, Dec 17, 2018 at 10:20:39PM +0200, sakari.ailus@iki.fi wrote:
> Hi Maxime,
> 
> On Mon, Dec 17, 2018 at 04:49:21PM +0100, Maxime Ripard wrote:
> > Hi Sakari,
> > 
> > Thanks for your feedback.
> > 
> > On Thu, Dec 13, 2018 at 10:49:28PM +0200, sakari.ailus@iki.fi wrote:
> > > > +	/**
> > > > +	 * @lanes:
> > > > +	 *
> > > > +	 * Number of active data lanes used for the transmissions.
> > > 
> > > Could you add that these are the first "lanes" number of lanes from what
> > > are available?
> > 
> > I'm not quite sure I understood this part though, what did you mean?
> 
> A number of lanes are routed between the two devices on hardware, and this
> field is specifying how many of them are in use. In order for the bus to
> function, both ends need to be in agreement on which of these lanes are
> actually being used. The current practice I've seen without exceptions is
> that these are the first n lanes.

Ah, right, I get it now, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 10/10] drm/bridge: cdns: Convert to phy framework
  2018-12-07 13:55 ` [PATCH v3 10/10] drm/bridge: cdns: Convert to phy framework Maxime Ripard
  2018-12-09  4:37   ` kbuild test robot
@ 2018-12-20 10:52   ` kbuild test robot
  1 sibling, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2018-12-20 10:52 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: kbuild-all, Kishon Vijay Abraham I, Boris Brezillon,
	Thomas Petazzoni, Laurent Pinchart, linux-media, Archit Taneja,
	Andrzej Hajda, Chen-Yu Tsai, linux-kernel, dri-devel,
	linux-arm-kernel, Krzysztof Witos, Rafal Ciepiela, Maxime Ripard

[-- Attachment #1: Type: text/plain, Size: 905 bytes --]

Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on phy/next]
[cannot apply to v4.20-rc7 next-20181220]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/phy-Add-MIPI-D-PHY-mode/20181208-034527
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
config: i386-randconfig-a2-12191846 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "phy_mipi_dphy_get_default_config" [drivers/gpu/drm/bridge/cdns-dsi.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31161 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-12-20 10:54 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-07 13:55 [PATCH v3 00/10] phy: Add configuration interface for MIPI D-PHY devices Maxime Ripard
2018-12-07 13:55 ` [PATCH v3 01/10] phy: Add MIPI D-PHY mode Maxime Ripard
2018-12-07 13:55 ` [PATCH v3 02/10] phy: Add configuration interface Maxime Ripard
2018-12-13 20:42   ` sakari.ailus
2018-12-07 13:55 ` [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options Maxime Ripard
2018-12-13 20:49   ` sakari.ailus
2018-12-17 15:49     ` Maxime Ripard
2018-12-17 20:20       ` sakari.ailus
2018-12-19 15:40         ` Maxime Ripard
2018-12-13 20:54   ` sakari.ailus
2018-12-13 20:55     ` sakari.ailus
2018-12-07 13:55 ` [PATCH v3 04/10] phy: dphy: Add configuration helpers Maxime Ripard
2018-12-07 13:55 ` [PATCH v3 05/10] sun6i: dsi: Convert to generic phy handling Maxime Ripard
2018-12-07 13:55 ` [PATCH v3 06/10] phy: Move Allwinner A31 D-PHY driver to drivers/phy/ Maxime Ripard
2018-12-07 13:55 ` [PATCH v3 07/10] drm/bridge: cdns: Separate DSI and D-PHY configuration Maxime Ripard
2018-12-07 13:55 ` [PATCH v3 08/10] dt-bindings: phy: Move the Cadence D-PHY bindings Maxime Ripard
2018-12-07 13:55 ` [PATCH v3 09/10] phy: Add Cadence D-PHY support Maxime Ripard
2018-12-07 13:55 ` [PATCH v3 10/10] drm/bridge: cdns: Convert to phy framework Maxime Ripard
2018-12-09  4:37   ` kbuild test robot
2018-12-20 10:52   ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).