All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: staging/imx: Handle CSI->VDIC->PRPVF pipeline
@ 2018-04-07 13:05 Marek Vasut
  2018-05-07  9:55 ` Hans Verkuil
  0 siblings, 1 reply; 4+ messages in thread
From: Marek Vasut @ 2018-04-07 13:05 UTC (permalink / raw)
  To: linux-media; +Cc: Marek Vasut, Philipp Zabel, Steve Longerbeam

In case the PRPVF is not connected directly to CSI, the PRPVF subdev
driver won't find the CSI subdev and will not configure the CSI input
mux. This is not noticable on the IPU1-CSI0 interface with parallel
camera, since the mux is set "correctly" by default and the parallel
camera will work just fine. This is however noticable on IPU2-CSI1,
where the mux is not set to the correct position by default and the
pipeline will fail.

Add similar code to what is in PRPVF to VDIC driver, so that the VDIC
can locate the CSI subdev and configure the mux correctly if the CSI
is connected to the VDIC. Make the PRPVF driver configure the CSI mux
only in case it's connected directly to CSI and not in case it is
connected to VDIC.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Steve Longerbeam <steve_longerbeam@mentor.com>
---
 drivers/staging/media/imx/imx-ic-prp.c     |  6 ++----
 drivers/staging/media/imx/imx-media-vdic.c | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
index 98923fc844ce..84fa66dae21a 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -72,14 +72,12 @@ static inline struct prp_priv *sd_to_priv(struct v4l2_subdev *sd)
 static int prp_start(struct prp_priv *priv)
 {
 	struct imx_ic_priv *ic_priv = priv->ic_priv;
-	bool src_is_vdic;
 
 	priv->ipu = priv->md->ipu[ic_priv->ipu_id];
 
 	/* set IC to receive from CSI or VDI depending on source */
-	src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC);
-
-	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
+	if (!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC))
+		ipu_set_ic_src_mux(priv->ipu, priv->csi_id, false);
 
 	return 0;
 }
diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
index b538bbebedc5..e660911e7024 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -117,6 +117,9 @@ struct vdic_priv {
 
 	bool csi_direct;  /* using direct CSI->VDIC->IC pipeline */
 
+	/* the CSI id at link validate */
+	int csi_id;
+
 	/* motion select control */
 	struct v4l2_ctrl_handler ctrl_hdlr;
 	enum ipu_motion_sel motion;
@@ -388,6 +391,9 @@ static int vdic_start(struct vdic_priv *priv)
 	if (ret)
 		return ret;
 
+	/* set IC to receive from CSI or VDI depending on source */
+	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, true);
+
 	/*
 	 * init the VDIC.
 	 *
@@ -778,6 +784,7 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_format *sink_fmt)
 {
 	struct vdic_priv *priv = v4l2_get_subdevdata(sd);
+	struct imx_media_subdev *csi;
 	int ret;
 
 	ret = v4l2_subdev_link_validate_default(sd, link,
@@ -785,6 +792,23 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
 	if (ret)
 		return ret;
 
+	csi = imx_media_find_upstream_subdev(priv->md, priv->src,
+					     IMX_MEDIA_GRP_ID_CSI);
+	if (!IS_ERR(csi)) {
+		switch (csi->sd->grp_id) {
+		case IMX_MEDIA_GRP_ID_CSI0:
+			priv->csi_id = 0;
+			break;
+		case IMX_MEDIA_GRP_ID_CSI1:
+			priv->csi_id = 1;
+			break;
+		default:
+			ret = -EINVAL;
+		}
+	} else {
+		priv->csi_id = 0;
+	}
+
 	mutex_lock(&priv->lock);
 
 	if (priv->csi_direct && priv->motion != HIGH_MOTION) {
-- 
2.16.2

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

* Re: [PATCH] media: staging/imx: Handle CSI->VDIC->PRPVF pipeline
  2018-04-07 13:05 [PATCH] media: staging/imx: Handle CSI->VDIC->PRPVF pipeline Marek Vasut
@ 2018-05-07  9:55 ` Hans Verkuil
  2018-09-17 10:25   ` Hans Verkuil
  0 siblings, 1 reply; 4+ messages in thread
From: Hans Verkuil @ 2018-05-07  9:55 UTC (permalink / raw)
  To: Marek Vasut, linux-media; +Cc: Philipp Zabel, Steve Longerbeam

On 07/04/18 15:05, Marek Vasut wrote:
> In case the PRPVF is not connected directly to CSI, the PRPVF subdev
> driver won't find the CSI subdev and will not configure the CSI input
> mux. This is not noticable on the IPU1-CSI0 interface with parallel
> camera, since the mux is set "correctly" by default and the parallel
> camera will work just fine. This is however noticable on IPU2-CSI1,
> where the mux is not set to the correct position by default and the
> pipeline will fail.
> 
> Add similar code to what is in PRPVF to VDIC driver, so that the VDIC
> can locate the CSI subdev and configure the mux correctly if the CSI
> is connected to the VDIC. Make the PRPVF driver configure the CSI mux
> only in case it's connected directly to CSI and not in case it is
> connected to VDIC.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Steve Longerbeam <steve_longerbeam@mentor.com>

Same here, I cannot merge with out Acks since I don't know the details
of the imx hardware.

Regards,

	Hans

> ---
>  drivers/staging/media/imx/imx-ic-prp.c     |  6 ++----
>  drivers/staging/media/imx/imx-media-vdic.c | 24 ++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
> index 98923fc844ce..84fa66dae21a 100644
> --- a/drivers/staging/media/imx/imx-ic-prp.c
> +++ b/drivers/staging/media/imx/imx-ic-prp.c
> @@ -72,14 +72,12 @@ static inline struct prp_priv *sd_to_priv(struct v4l2_subdev *sd)
>  static int prp_start(struct prp_priv *priv)
>  {
>  	struct imx_ic_priv *ic_priv = priv->ic_priv;
> -	bool src_is_vdic;
>  
>  	priv->ipu = priv->md->ipu[ic_priv->ipu_id];
>  
>  	/* set IC to receive from CSI or VDI depending on source */
> -	src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC);
> -
> -	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
> +	if (!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC))
> +		ipu_set_ic_src_mux(priv->ipu, priv->csi_id, false);
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
> index b538bbebedc5..e660911e7024 100644
> --- a/drivers/staging/media/imx/imx-media-vdic.c
> +++ b/drivers/staging/media/imx/imx-media-vdic.c
> @@ -117,6 +117,9 @@ struct vdic_priv {
>  
>  	bool csi_direct;  /* using direct CSI->VDIC->IC pipeline */
>  
> +	/* the CSI id at link validate */
> +	int csi_id;
> +
>  	/* motion select control */
>  	struct v4l2_ctrl_handler ctrl_hdlr;
>  	enum ipu_motion_sel motion;
> @@ -388,6 +391,9 @@ static int vdic_start(struct vdic_priv *priv)
>  	if (ret)
>  		return ret;
>  
> +	/* set IC to receive from CSI or VDI depending on source */
> +	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, true);
> +
>  	/*
>  	 * init the VDIC.
>  	 *
> @@ -778,6 +784,7 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
>  			      struct v4l2_subdev_format *sink_fmt)
>  {
>  	struct vdic_priv *priv = v4l2_get_subdevdata(sd);
> +	struct imx_media_subdev *csi;
>  	int ret;
>  
>  	ret = v4l2_subdev_link_validate_default(sd, link,
> @@ -785,6 +792,23 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
>  	if (ret)
>  		return ret;
>  
> +	csi = imx_media_find_upstream_subdev(priv->md, priv->src,
> +					     IMX_MEDIA_GRP_ID_CSI);
> +	if (!IS_ERR(csi)) {
> +		switch (csi->sd->grp_id) {
> +		case IMX_MEDIA_GRP_ID_CSI0:
> +			priv->csi_id = 0;
> +			break;
> +		case IMX_MEDIA_GRP_ID_CSI1:
> +			priv->csi_id = 1;
> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
> +	} else {
> +		priv->csi_id = 0;
> +	}
> +
>  	mutex_lock(&priv->lock);
>  
>  	if (priv->csi_direct && priv->motion != HIGH_MOTION) {
> 

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

* Re: [PATCH] media: staging/imx: Handle CSI->VDIC->PRPVF pipeline
  2018-05-07  9:55 ` Hans Verkuil
@ 2018-09-17 10:25   ` Hans Verkuil
  2018-09-18 22:49     ` Steve Longerbeam
  0 siblings, 1 reply; 4+ messages in thread
From: Hans Verkuil @ 2018-09-17 10:25 UTC (permalink / raw)
  To: Marek Vasut, linux-media; +Cc: Philipp Zabel, Steve Longerbeam

On 05/07/2018 11:55 AM, Hans Verkuil wrote:
> On 07/04/18 15:05, Marek Vasut wrote:
>> In case the PRPVF is not connected directly to CSI, the PRPVF subdev
>> driver won't find the CSI subdev and will not configure the CSI input
>> mux. This is not noticable on the IPU1-CSI0 interface with parallel
>> camera, since the mux is set "correctly" by default and the parallel
>> camera will work just fine. This is however noticable on IPU2-CSI1,
>> where the mux is not set to the correct position by default and the
>> pipeline will fail.
>>
>> Add similar code to what is in PRPVF to VDIC driver, so that the VDIC
>> can locate the CSI subdev and configure the mux correctly if the CSI
>> is connected to the VDIC. Make the PRPVF driver configure the CSI mux
>> only in case it's connected directly to CSI and not in case it is
>> connected to VDIC.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: Steve Longerbeam <steve_longerbeam@mentor.com>
> 
> Same here, I cannot merge with out Acks since I don't know the details
> of the imx hardware.

I'm marking this patch as Obsoleted since there has been no activity for a long
time.

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>> ---
>>  drivers/staging/media/imx/imx-ic-prp.c     |  6 ++----
>>  drivers/staging/media/imx/imx-media-vdic.c | 24 ++++++++++++++++++++++++
>>  2 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
>> index 98923fc844ce..84fa66dae21a 100644
>> --- a/drivers/staging/media/imx/imx-ic-prp.c
>> +++ b/drivers/staging/media/imx/imx-ic-prp.c
>> @@ -72,14 +72,12 @@ static inline struct prp_priv *sd_to_priv(struct v4l2_subdev *sd)
>>  static int prp_start(struct prp_priv *priv)
>>  {
>>  	struct imx_ic_priv *ic_priv = priv->ic_priv;
>> -	bool src_is_vdic;
>>  
>>  	priv->ipu = priv->md->ipu[ic_priv->ipu_id];
>>  
>>  	/* set IC to receive from CSI or VDI depending on source */
>> -	src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC);
>> -
>> -	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
>> +	if (!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC))
>> +		ipu_set_ic_src_mux(priv->ipu, priv->csi_id, false);
>>  
>>  	return 0;
>>  }
>> diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
>> index b538bbebedc5..e660911e7024 100644
>> --- a/drivers/staging/media/imx/imx-media-vdic.c
>> +++ b/drivers/staging/media/imx/imx-media-vdic.c
>> @@ -117,6 +117,9 @@ struct vdic_priv {
>>  
>>  	bool csi_direct;  /* using direct CSI->VDIC->IC pipeline */
>>  
>> +	/* the CSI id at link validate */
>> +	int csi_id;
>> +
>>  	/* motion select control */
>>  	struct v4l2_ctrl_handler ctrl_hdlr;
>>  	enum ipu_motion_sel motion;
>> @@ -388,6 +391,9 @@ static int vdic_start(struct vdic_priv *priv)
>>  	if (ret)
>>  		return ret;
>>  
>> +	/* set IC to receive from CSI or VDI depending on source */
>> +	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, true);
>> +
>>  	/*
>>  	 * init the VDIC.
>>  	 *
>> @@ -778,6 +784,7 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
>>  			      struct v4l2_subdev_format *sink_fmt)
>>  {
>>  	struct vdic_priv *priv = v4l2_get_subdevdata(sd);
>> +	struct imx_media_subdev *csi;
>>  	int ret;
>>  
>>  	ret = v4l2_subdev_link_validate_default(sd, link,
>> @@ -785,6 +792,23 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
>>  	if (ret)
>>  		return ret;
>>  
>> +	csi = imx_media_find_upstream_subdev(priv->md, priv->src,
>> +					     IMX_MEDIA_GRP_ID_CSI);
>> +	if (!IS_ERR(csi)) {
>> +		switch (csi->sd->grp_id) {
>> +		case IMX_MEDIA_GRP_ID_CSI0:
>> +			priv->csi_id = 0;
>> +			break;
>> +		case IMX_MEDIA_GRP_ID_CSI1:
>> +			priv->csi_id = 1;
>> +			break;
>> +		default:
>> +			ret = -EINVAL;
>> +		}
>> +	} else {
>> +		priv->csi_id = 0;
>> +	}
>> +
>>  	mutex_lock(&priv->lock);
>>  
>>  	if (priv->csi_direct && priv->motion != HIGH_MOTION) {
>>
> 
> 

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

* Re: [PATCH] media: staging/imx: Handle CSI->VDIC->PRPVF pipeline
  2018-09-17 10:25   ` Hans Verkuil
@ 2018-09-18 22:49     ` Steve Longerbeam
  0 siblings, 0 replies; 4+ messages in thread
From: Steve Longerbeam @ 2018-09-18 22:49 UTC (permalink / raw)
  To: Hans Verkuil, Marek Vasut, linux-media; +Cc: Philipp Zabel



On 09/17/2018 03:25 AM, Hans Verkuil wrote:
> On 05/07/2018 11:55 AM, Hans Verkuil wrote:
>> On 07/04/18 15:05, Marek Vasut wrote:
>>> In case the PRPVF is not connected directly to CSI, the PRPVF subdev
>>> driver won't find the CSI subdev and will not configure the CSI input
>>> mux. This is not noticable on the IPU1-CSI0 interface with parallel
>>> camera, since the mux is set "correctly" by default and the parallel
>>> camera will work just fine. This is however noticable on IPU2-CSI1,
>>> where the mux is not set to the correct position by default and the
>>> pipeline will fail.
>>>
>>> Add similar code to what is in PRPVF to VDIC driver, so that the VDIC
>>> can locate the CSI subdev and configure the mux correctly if the CSI
>>> is connected to the VDIC. Make the PRPVF driver configure the CSI mux
>>> only in case it's connected directly to CSI and not in case it is
>>> connected to VDIC.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>>> Cc: Steve Longerbeam <steve_longerbeam@mentor.com>
>> Same here, I cannot merge with out Acks since I don't know the details
>> of the imx hardware.
> I'm marking this patch as Obsoleted since there has been no activity for a long
> time.

Hi Hans, yes that's fine. IIRC this issue has been fixed a different way.

Steve

>
>
>>> ---
>>>   drivers/staging/media/imx/imx-ic-prp.c     |  6 ++----
>>>   drivers/staging/media/imx/imx-media-vdic.c | 24 ++++++++++++++++++++++++
>>>   2 files changed, 26 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
>>> index 98923fc844ce..84fa66dae21a 100644
>>> --- a/drivers/staging/media/imx/imx-ic-prp.c
>>> +++ b/drivers/staging/media/imx/imx-ic-prp.c
>>> @@ -72,14 +72,12 @@ static inline struct prp_priv *sd_to_priv(struct v4l2_subdev *sd)
>>>   static int prp_start(struct prp_priv *priv)
>>>   {
>>>   	struct imx_ic_priv *ic_priv = priv->ic_priv;
>>> -	bool src_is_vdic;
>>>   
>>>   	priv->ipu = priv->md->ipu[ic_priv->ipu_id];
>>>   
>>>   	/* set IC to receive from CSI or VDI depending on source */
>>> -	src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC);
>>> -
>>> -	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
>>> +	if (!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC))
>>> +		ipu_set_ic_src_mux(priv->ipu, priv->csi_id, false);
>>>   
>>>   	return 0;
>>>   }
>>> diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
>>> index b538bbebedc5..e660911e7024 100644
>>> --- a/drivers/staging/media/imx/imx-media-vdic.c
>>> +++ b/drivers/staging/media/imx/imx-media-vdic.c
>>> @@ -117,6 +117,9 @@ struct vdic_priv {
>>>   
>>>   	bool csi_direct;  /* using direct CSI->VDIC->IC pipeline */
>>>   
>>> +	/* the CSI id at link validate */
>>> +	int csi_id;
>>> +
>>>   	/* motion select control */
>>>   	struct v4l2_ctrl_handler ctrl_hdlr;
>>>   	enum ipu_motion_sel motion;
>>> @@ -388,6 +391,9 @@ static int vdic_start(struct vdic_priv *priv)
>>>   	if (ret)
>>>   		return ret;
>>>   
>>> +	/* set IC to receive from CSI or VDI depending on source */
>>> +	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, true);
>>> +
>>>   	/*
>>>   	 * init the VDIC.
>>>   	 *
>>> @@ -778,6 +784,7 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
>>>   			      struct v4l2_subdev_format *sink_fmt)
>>>   {
>>>   	struct vdic_priv *priv = v4l2_get_subdevdata(sd);
>>> +	struct imx_media_subdev *csi;
>>>   	int ret;
>>>   
>>>   	ret = v4l2_subdev_link_validate_default(sd, link,
>>> @@ -785,6 +792,23 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
>>>   	if (ret)
>>>   		return ret;
>>>   
>>> +	csi = imx_media_find_upstream_subdev(priv->md, priv->src,
>>> +					     IMX_MEDIA_GRP_ID_CSI);
>>> +	if (!IS_ERR(csi)) {
>>> +		switch (csi->sd->grp_id) {
>>> +		case IMX_MEDIA_GRP_ID_CSI0:
>>> +			priv->csi_id = 0;
>>> +			break;
>>> +		case IMX_MEDIA_GRP_ID_CSI1:
>>> +			priv->csi_id = 1;
>>> +			break;
>>> +		default:
>>> +			ret = -EINVAL;
>>> +		}
>>> +	} else {
>>> +		priv->csi_id = 0;
>>> +	}
>>> +
>>>   	mutex_lock(&priv->lock);
>>>   
>>>   	if (priv->csi_direct && priv->motion != HIGH_MOTION) {
>>>
>>

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

end of thread, other threads:[~2018-09-19  4:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-07 13:05 [PATCH] media: staging/imx: Handle CSI->VDIC->PRPVF pipeline Marek Vasut
2018-05-07  9:55 ` Hans Verkuil
2018-09-17 10:25   ` Hans Verkuil
2018-09-18 22:49     ` Steve Longerbeam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.