* [PATCH] media: staging: max96712: Add support for 3-lane C-PHY
@ 2023-02-11 14:46 Niklas Söderlund
2023-03-30 14:21 ` Niklas Söderlund
2023-03-31 12:21 ` Sakari Ailus
0 siblings, 2 replies; 6+ messages in thread
From: Niklas Söderlund @ 2023-02-11 14:46 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging
Cc: linux-renesas-soc, Niklas Söderlund
Add basic support for outputting the test patterns on a 3-lane CSI-2
C-PHY bus. As the driver only can output frames form its internal test
pattern generator, enabling C-PHY output is as simple as setting the
output mode to C-PHY instead of D-PHY.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/staging/media/max96712/max96712.c | 36 +++++++++++++++++++----
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 99b333b68198..d93dd985fb27 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -30,6 +30,7 @@ struct max96712_priv {
struct regmap *regmap;
struct gpio_desc *gpiod_pwdn;
+ bool cphy;
struct v4l2_mbus_config_mipi_csi2 mipi;
struct v4l2_subdev sd;
@@ -127,10 +128,18 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
/* Select 2x4 mode. */
max96712_write(priv, 0x8a0, 0x04);
- /* Configure a 4-lane DPHY using PHY0 and PHY1. */
/* TODO: Add support for 2-lane and 1-lane configurations. */
- /* TODO: Add support CPHY mode. */
- max96712_write(priv, 0x94a, 0xc0);
+ if (priv->cphy) {
+ /* Configure a 3-lane C-PHY using PHY0 and PHY1. */
+ max96712_write(priv, 0x94a, 0xa0);
+
+ /* Configure C-PHY timings. */
+ max96712_write(priv, 0x8ad, 0x3f);
+ max96712_write(priv, 0x8ae, 0x7d);
+ } else {
+ /* Configure a 4-lane D-PHY using PHY0 and PHY1. */
+ max96712_write(priv, 0x94a, 0xc0);
+ }
/* Configure lane mapping for PHY0 and PHY1. */
/* TODO: Add support for lane swapping. */
@@ -332,8 +341,9 @@ static int max96712_parse_dt(struct max96712_priv *priv)
{
struct fwnode_handle *ep;
struct v4l2_fwnode_endpoint v4l2_ep = {
- .bus_type = V4L2_MBUS_CSI2_DPHY
+ .bus_type = V4L2_MBUS_UNKNOWN,
};
+ unsigned int supported_lanes;
int ret;
ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(&priv->client->dev), 4,
@@ -350,8 +360,22 @@ static int max96712_parse_dt(struct max96712_priv *priv)
return -EINVAL;
}
- if (v4l2_ep.bus.mipi_csi2.num_data_lanes != 4) {
- dev_err(&priv->client->dev, "Only 4 data lanes supported\n");
+ switch (v4l2_ep.bus_type) {
+ case V4L2_MBUS_CSI2_DPHY:
+ supported_lanes = 4;
+ priv->cphy = false;
+ break;
+ case V4L2_MBUS_CSI2_CPHY:
+ supported_lanes = 3;
+ priv->cphy = true;
+ break;
+ default:
+ dev_err(&priv->client->dev, "Unsupported bus-type %u\n", v4l2_ep.bus_type);
+ return -EINVAL;
+ }
+
+ if (v4l2_ep.bus.mipi_csi2.num_data_lanes != supported_lanes) {
+ dev_err(&priv->client->dev, "Only %u data lanes supported\n", supported_lanes);
return -EINVAL;
}
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] media: staging: max96712: Add support for 3-lane C-PHY
2023-02-11 14:46 [PATCH] media: staging: max96712: Add support for 3-lane C-PHY Niklas Söderlund
@ 2023-03-30 14:21 ` Niklas Söderlund
2023-03-31 12:21 ` Sakari Ailus
1 sibling, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2023-03-30 14:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging
Cc: linux-renesas-soc
Hi Mauro,
Gentle ping on this patch.
On 2023-02-11 15:46:14 +0100, Niklas Söderlund wrote:
> Add basic support for outputting the test patterns on a 3-lane CSI-2
> C-PHY bus. As the driver only can output frames form its internal test
> pattern generator, enabling C-PHY output is as simple as setting the
> output mode to C-PHY instead of D-PHY.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/staging/media/max96712/max96712.c | 36 +++++++++++++++++++----
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> index 99b333b68198..d93dd985fb27 100644
> --- a/drivers/staging/media/max96712/max96712.c
> +++ b/drivers/staging/media/max96712/max96712.c
> @@ -30,6 +30,7 @@ struct max96712_priv {
> struct regmap *regmap;
> struct gpio_desc *gpiod_pwdn;
>
> + bool cphy;
> struct v4l2_mbus_config_mipi_csi2 mipi;
>
> struct v4l2_subdev sd;
> @@ -127,10 +128,18 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
> /* Select 2x4 mode. */
> max96712_write(priv, 0x8a0, 0x04);
>
> - /* Configure a 4-lane DPHY using PHY0 and PHY1. */
> /* TODO: Add support for 2-lane and 1-lane configurations. */
> - /* TODO: Add support CPHY mode. */
> - max96712_write(priv, 0x94a, 0xc0);
> + if (priv->cphy) {
> + /* Configure a 3-lane C-PHY using PHY0 and PHY1. */
> + max96712_write(priv, 0x94a, 0xa0);
> +
> + /* Configure C-PHY timings. */
> + max96712_write(priv, 0x8ad, 0x3f);
> + max96712_write(priv, 0x8ae, 0x7d);
> + } else {
> + /* Configure a 4-lane D-PHY using PHY0 and PHY1. */
> + max96712_write(priv, 0x94a, 0xc0);
> + }
>
> /* Configure lane mapping for PHY0 and PHY1. */
> /* TODO: Add support for lane swapping. */
> @@ -332,8 +341,9 @@ static int max96712_parse_dt(struct max96712_priv *priv)
> {
> struct fwnode_handle *ep;
> struct v4l2_fwnode_endpoint v4l2_ep = {
> - .bus_type = V4L2_MBUS_CSI2_DPHY
> + .bus_type = V4L2_MBUS_UNKNOWN,
> };
> + unsigned int supported_lanes;
> int ret;
>
> ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(&priv->client->dev), 4,
> @@ -350,8 +360,22 @@ static int max96712_parse_dt(struct max96712_priv *priv)
> return -EINVAL;
> }
>
> - if (v4l2_ep.bus.mipi_csi2.num_data_lanes != 4) {
> - dev_err(&priv->client->dev, "Only 4 data lanes supported\n");
> + switch (v4l2_ep.bus_type) {
> + case V4L2_MBUS_CSI2_DPHY:
> + supported_lanes = 4;
> + priv->cphy = false;
> + break;
> + case V4L2_MBUS_CSI2_CPHY:
> + supported_lanes = 3;
> + priv->cphy = true;
> + break;
> + default:
> + dev_err(&priv->client->dev, "Unsupported bus-type %u\n", v4l2_ep.bus_type);
> + return -EINVAL;
> + }
> +
> + if (v4l2_ep.bus.mipi_csi2.num_data_lanes != supported_lanes) {
> + dev_err(&priv->client->dev, "Only %u data lanes supported\n", supported_lanes);
> return -EINVAL;
> }
>
> --
> 2.39.1
>
--
Kind Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: staging: max96712: Add support for 3-lane C-PHY
2023-02-11 14:46 [PATCH] media: staging: max96712: Add support for 3-lane C-PHY Niklas Söderlund
2023-03-30 14:21 ` Niklas Söderlund
@ 2023-03-31 12:21 ` Sakari Ailus
2023-03-31 13:24 ` Niklas Söderlund
1 sibling, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2023-03-31 12:21 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media,
linux-staging, linux-renesas-soc
Hejssan,
On Sat, Feb 11, 2023 at 03:46:14PM +0100, Niklas Söderlund wrote:
> Add basic support for outputting the test patterns on a 3-lane CSI-2
> C-PHY bus. As the driver only can output frames form its internal test
> pattern generator, enabling C-PHY output is as simple as setting the
> output mode to C-PHY instead of D-PHY.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/staging/media/max96712/max96712.c | 36 +++++++++++++++++++----
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> index 99b333b68198..d93dd985fb27 100644
> --- a/drivers/staging/media/max96712/max96712.c
> +++ b/drivers/staging/media/max96712/max96712.c
> @@ -30,6 +30,7 @@ struct max96712_priv {
> struct regmap *regmap;
> struct gpio_desc *gpiod_pwdn;
>
> + bool cphy;
> struct v4l2_mbus_config_mipi_csi2 mipi;
>
> struct v4l2_subdev sd;
> @@ -127,10 +128,18 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
> /* Select 2x4 mode. */
> max96712_write(priv, 0x8a0, 0x04);
>
> - /* Configure a 4-lane DPHY using PHY0 and PHY1. */
> /* TODO: Add support for 2-lane and 1-lane configurations. */
> - /* TODO: Add support CPHY mode. */
> - max96712_write(priv, 0x94a, 0xc0);
> + if (priv->cphy) {
> + /* Configure a 3-lane C-PHY using PHY0 and PHY1. */
> + max96712_write(priv, 0x94a, 0xa0);
> +
> + /* Configure C-PHY timings. */
> + max96712_write(priv, 0x8ad, 0x3f);
> + max96712_write(priv, 0x8ae, 0x7d);
> + } else {
> + /* Configure a 4-lane D-PHY using PHY0 and PHY1. */
> + max96712_write(priv, 0x94a, 0xc0);
> + }
>
> /* Configure lane mapping for PHY0 and PHY1. */
> /* TODO: Add support for lane swapping. */
> @@ -332,8 +341,9 @@ static int max96712_parse_dt(struct max96712_priv *priv)
> {
> struct fwnode_handle *ep;
> struct v4l2_fwnode_endpoint v4l2_ep = {
> - .bus_type = V4L2_MBUS_CSI2_DPHY
> + .bus_type = V4L2_MBUS_UNKNOWN,
The bindings don't require setting bus-type. Please change the bindings as
well. And assume D-PHY in the driver if bus-type isn't set.
> };
> + unsigned int supported_lanes;
> int ret;
>
> ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(&priv->client->dev), 4,
> @@ -350,8 +360,22 @@ static int max96712_parse_dt(struct max96712_priv *priv)
> return -EINVAL;
> }
>
> - if (v4l2_ep.bus.mipi_csi2.num_data_lanes != 4) {
> - dev_err(&priv->client->dev, "Only 4 data lanes supported\n");
> + switch (v4l2_ep.bus_type) {
> + case V4L2_MBUS_CSI2_DPHY:
> + supported_lanes = 4;
> + priv->cphy = false;
> + break;
> + case V4L2_MBUS_CSI2_CPHY:
> + supported_lanes = 3;
> + priv->cphy = true;
> + break;
> + default:
> + dev_err(&priv->client->dev, "Unsupported bus-type %u\n", v4l2_ep.bus_type);
> + return -EINVAL;
> + }
> +
> + if (v4l2_ep.bus.mipi_csi2.num_data_lanes != supported_lanes) {
> + dev_err(&priv->client->dev, "Only %u data lanes supported\n", supported_lanes);
> return -EINVAL;
> }
>
--
Hälsningar,
Sakari Ailus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: staging: max96712: Add support for 3-lane C-PHY
2023-03-31 12:21 ` Sakari Ailus
@ 2023-03-31 13:24 ` Niklas Söderlund
2023-03-31 14:08 ` Sakari Ailus
0 siblings, 1 reply; 6+ messages in thread
From: Niklas Söderlund @ 2023-03-31 13:24 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media,
linux-staging, linux-renesas-soc
Hej Sakari,
Tack för din feedback.
On 2023-03-31 15:21:34 +0300, Sakari Ailus wrote:
...
> > @@ -332,8 +341,9 @@ static int max96712_parse_dt(struct
> > max96712_priv *priv)
> > {
> > struct fwnode_handle *ep;
> > struct v4l2_fwnode_endpoint v4l2_ep = {
> > - .bus_type = V4L2_MBUS_CSI2_DPHY
> > + .bus_type = V4L2_MBUS_UNKNOWN,
>
> The bindings don't require setting bus-type. Please change the bindings as
> well. And assume D-PHY in the driver if bus-type isn't set.
Thanks for spotting this, I will send out an update to update the
binding to require setting bus-type.
About updating the driver to assume D-PHY if bus-type is not set. I
wonder if we can avoid that and keep the driver change as is? The only
in-tree user of this binding is in r8a779a0-falcon-csi-dsi.dtsi, and I
intend to send out a patch to set the bus-type for that together with
the updated bindings.
I have tested this driver change on the Falcon board and if bus-type is
not explicitly set in the DTS it is reported as D-PHY and everything
works as expected. So if I
- Send out a patch to update the bindings to require bus-type being set.
- Send out patch to update the only in-tree use of the driver to set
bus-type.
Can't we avoid issues in the future by not assuming no bus-type is D-PHY
in the driver while still being backward compatible with the old DTS?
--
Kind Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: staging: max96712: Add support for 3-lane C-PHY
2023-03-31 13:24 ` Niklas Söderlund
@ 2023-03-31 14:08 ` Sakari Ailus
2023-03-31 14:19 ` Niklas Söderlund
0 siblings, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2023-03-31 14:08 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media,
linux-staging, linux-renesas-soc
Hejssan, Niklas!
On Fri, Mar 31, 2023 at 03:24:04PM +0200, Niklas Söderlund wrote:
> Hej Sakari,
>
> Tack för din feedback.
>
> On 2023-03-31 15:21:34 +0300, Sakari Ailus wrote:
>
> ...
>
> > > @@ -332,8 +341,9 @@ static int max96712_parse_dt(struct
> > > max96712_priv *priv)
> > > {
> > > struct fwnode_handle *ep;
> > > struct v4l2_fwnode_endpoint v4l2_ep = {
> > > - .bus_type = V4L2_MBUS_CSI2_DPHY
> > > + .bus_type = V4L2_MBUS_UNKNOWN,
> >
> > The bindings don't require setting bus-type. Please change the bindings as
> > well. And assume D-PHY in the driver if bus-type isn't set.
>
> Thanks for spotting this, I will send out an update to update the
> binding to require setting bus-type.
>
> About updating the driver to assume D-PHY if bus-type is not set. I
> wonder if we can avoid that and keep the driver change as is? The only
> in-tree user of this binding is in r8a779a0-falcon-csi-dsi.dtsi, and I
> intend to send out a patch to set the bus-type for that together with
> the updated bindings.
>
> I have tested this driver change on the Falcon board and if bus-type is
> not explicitly set in the DTS it is reported as D-PHY and everything
> works as expected. So if I
>
> - Send out a patch to update the bindings to require bus-type being set.
> - Send out patch to update the only in-tree use of the driver to set
> bus-type.
>
> Can't we avoid issues in the future by not assuming no bus-type is D-PHY
> in the driver while still being backward compatible with the old DTS?
If you want to keep supporting D-PHY as a default, you should instead try
parsing with V4L2_MBUS_CSI2_DPHY as bus_type. Then CPHY if that fails.
Although if bus_type is mandatory, then this patch is fine.
--
Trevliga hälsningar,
Sakari Ailus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: staging: max96712: Add support for 3-lane C-PHY
2023-03-31 14:08 ` Sakari Ailus
@ 2023-03-31 14:19 ` Niklas Söderlund
0 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2023-03-31 14:19 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media,
linux-staging, linux-renesas-soc
Hej Sakari,
On 2023-03-31 17:08:53 +0300, Sakari Ailus wrote:
> Hejssan, Niklas!
>
> On Fri, Mar 31, 2023 at 03:24:04PM +0200, Niklas Söderlund wrote:
> > Hej Sakari,
> >
> > Tack för din feedback.
> >
> > On 2023-03-31 15:21:34 +0300, Sakari Ailus wrote:
> >
> > ...
> >
> > > > @@ -332,8 +341,9 @@ static int max96712_parse_dt(struct
> > > > max96712_priv *priv)
> > > > {
> > > > struct fwnode_handle *ep;
> > > > struct v4l2_fwnode_endpoint v4l2_ep = {
> > > > - .bus_type = V4L2_MBUS_CSI2_DPHY
> > > > + .bus_type = V4L2_MBUS_UNKNOWN,
> > >
> > > The bindings don't require setting bus-type. Please change the bindings as
> > > well. And assume D-PHY in the driver if bus-type isn't set.
> >
> > Thanks for spotting this, I will send out an update to update the
> > binding to require setting bus-type.
> >
> > About updating the driver to assume D-PHY if bus-type is not set. I
> > wonder if we can avoid that and keep the driver change as is? The only
> > in-tree user of this binding is in r8a779a0-falcon-csi-dsi.dtsi, and I
> > intend to send out a patch to set the bus-type for that together with
> > the updated bindings.
> >
> > I have tested this driver change on the Falcon board and if bus-type is
> > not explicitly set in the DTS it is reported as D-PHY and everything
> > works as expected. So if I
> >
> > - Send out a patch to update the bindings to require bus-type being set.
> > - Send out patch to update the only in-tree use of the driver to set
> > bus-type.
> >
> > Can't we avoid issues in the future by not assuming no bus-type is D-PHY
> > in the driver while still being backward compatible with the old DTS?
>
> If you want to keep supporting D-PHY as a default, you should instead try
> parsing with V4L2_MBUS_CSI2_DPHY as bus_type. Then CPHY if that fails.
>
> Although if bus_type is mandatory, then this patch is fine.
Thanks. I prefers to make bus-type a mandatory property, it feels like
being explicit in the bindings is the best way going forward. I have
posted the patches to make this change and to update the only user.
There is only one user in-tree, and making the change now would still be
backward compatible with it. So better bite the bullet now before the
binding spreads.
Trevlig helg!
>
> --
> Trevliga hälsningar,
>
> Sakari Ailus
--
Kind Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-31 14:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-11 14:46 [PATCH] media: staging: max96712: Add support for 3-lane C-PHY Niklas Söderlund
2023-03-30 14:21 ` Niklas Söderlund
2023-03-31 12:21 ` Sakari Ailus
2023-03-31 13:24 ` Niklas Söderlund
2023-03-31 14:08 ` Sakari Ailus
2023-03-31 14:19 ` Niklas Söderlund
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).