From: Andrey Konovalov <andrey.konovalov@linaro.org>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-media@vger.kernel.org, laurent.pinchart@ideasonboard.com,
mchehab@kernel.org, niklas.soderlund@ragnatech.se,
bparrot@ti.com, mickael.guene@st.com
Subject: Re: [RFC PATCH 4/4] staging: media: omap4iss: use v4l2_get_link_freq() to get the external rate
Date: Tue, 9 Mar 2021 14:24:41 +0300 [thread overview]
Message-ID: <da7f0769-6f12-e895-8cf1-adf915bce64b@linaro.org> (raw)
In-Reply-To: <20210305154103.GV3@paasikivi.fi.intel.com>
Hi Sakari,
On 05.03.2021 18:41, Sakari Ailus wrote:
> Hi Andrey,
>
> Thanks for the set.
>
> On Wed, Mar 03, 2021 at 09:08:17PM +0300, Andrey Konovalov wrote:
>> This driver uses V4L2_CID_PIXEL_RATE to calculate the CSI2 link frequency,
>> but this may give incorrect result in some cases. Use v4l2_get_link_freq()
>> instead.
>>
>> Also the driver used the external_rate field in struct iss_pipeline as a
>> flag to prevent excessive v4l2_subdev_call's when processing the frames
>> in single-shot mode. Replace the external_rate with external_lfreq, and
>> use external_bpp and external_lfreq to call v4l2_subdev_call(get_fmt) and
>> v4l2_get_link_freq() respectively only once per iss_video_streamon().
>>
>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
>> ---
>> drivers/staging/media/omap4iss/iss.c | 12 +-----------
>> drivers/staging/media/omap4iss/iss_csiphy.c | 19 ++++++++++++++++---
>> drivers/staging/media/omap4iss/iss_video.c | 2 +-
>> drivers/staging/media/omap4iss/iss_video.h | 2 +-
>> 4 files changed, 19 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
>> index dae9073e7d3c..0eb7b1b5dcc4 100644
>> --- a/drivers/staging/media/omap4iss/iss.c
>> +++ b/drivers/staging/media/omap4iss/iss.c
>> @@ -131,7 +131,7 @@ int omap4iss_get_external_info(struct iss_pipeline *pipe,
>> if (!pipe->external)
>> return 0;
>>
>> - if (pipe->external_rate)
>> + if (pipe->external_bpp)
>> return 0;
>>
>> memset(&fmt, 0, sizeof(fmt));
>> @@ -145,16 +145,6 @@ int omap4iss_get_external_info(struct iss_pipeline *pipe,
>>
>> pipe->external_bpp = omap4iss_video_format_info(fmt.format.code)->bpp;
>>
>> - ctrl = v4l2_ctrl_find(pipe->external->ctrl_handler,
>> - V4L2_CID_PIXEL_RATE);
>> - if (!ctrl) {
>> - dev_warn(iss->dev, "no pixel rate control in subdev %s\n",
>> - pipe->external->name);
>> - return -EPIPE;
>> - }
>> -
>> - pipe->external_rate = v4l2_ctrl_g_ctrl_int64(ctrl);
>> -
>> return 0;
>> }
>>
>> diff --git a/drivers/staging/media/omap4iss/iss_csiphy.c b/drivers/staging/media/omap4iss/iss_csiphy.c
>> index 96f2ce045138..cec0cd21f7e0 100644
>> --- a/drivers/staging/media/omap4iss/iss_csiphy.c
>> +++ b/drivers/staging/media/omap4iss/iss_csiphy.c
>> @@ -119,6 +119,7 @@ int omap4iss_csiphy_config(struct iss_device *iss,
>> struct iss_pipeline *pipe = to_iss_pipeline(&csi2_subdev->entity);
>> struct iss_v4l2_subdevs_group *subdevs = pipe->external->host_priv;
>> struct iss_csiphy_dphy_cfg csi2phy;
>> + s64 link_freq;
>> int csi2_ddrclk_khz;
>> struct iss_csiphy_lanes_cfg *lanes;
>> unsigned int used_lanes = 0;
>> @@ -193,9 +194,21 @@ int omap4iss_csiphy_config(struct iss_device *iss,
>> if (lanes->clk.pos == 0 || used_lanes & (1 << lanes->clk.pos))
>> return -EINVAL;
>>
>> - csi2_ddrclk_khz = pipe->external_rate / 1000
>> - / (2 * csi2->phy->used_data_lanes)
>> - * pipe->external_bpp;
>> + if (!pipe->external_lfreq) {
>> + link_freq = v4l2_get_link_freq(pipe->external->ctrl_handler,
>
> I wonder if you could this unconditionally, and remove external_lfreq field
> altogether. The same could be done for external_bpp but that's out of scope
> for this patch.
Hard to tell.
This might be possible as all this logic to prevent multiple v4l2_subdev_call(get_fmt)
and v4l2_get_link_freq() calls per single iss_video_streamon() seems to be needed
only when ISS operates in memory-to-memory mode. Not sure how link frequency, and
used_lanes are related to memory-to-memory mode... Will try to find out.
Thanks,
Andrey
>> + pipe->external_bpp,
>> + 2 * csi2->phy->used_data_lanes);
>> + if (link_freq < 0) {
>> + dev_warn(iss->dev,
>> + "failed to read the link frequency fromn subdev %s\n",
>> + pipe->external->name);
>> + return -EINVAL;
>> + }
>> +
>> + pipe->external_lfreq = link_freq;
>> + }
>> +
>> + csi2_ddrclk_khz = div_s64(pipe->external_lfreq, 1000);
>>
>> /*
>> * THS_TERM: Programmed value = ceil(12.5 ns/DDRClk period) - 1.
>> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
>> index 66975a37dc85..a654c8d18bbc 100644
>> --- a/drivers/staging/media/omap4iss/iss_video.c
>> +++ b/drivers/staging/media/omap4iss/iss_video.c
>> @@ -872,7 +872,7 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>> pipe = entity->pipe
>> ? to_iss_pipeline(entity) : &video->pipe;
>> pipe->external = NULL;
>> - pipe->external_rate = 0;
>> + pipe->external_lfreq = 0;
>> pipe->external_bpp = 0;
>>
>> ret = media_entity_enum_init(&pipe->ent_enum, entity->graph_obj.mdev);
>> diff --git a/drivers/staging/media/omap4iss/iss_video.h b/drivers/staging/media/omap4iss/iss_video.h
>> index 526281bf0051..2ad5c8483958 100644
>> --- a/drivers/staging/media/omap4iss/iss_video.h
>> +++ b/drivers/staging/media/omap4iss/iss_video.h
>> @@ -86,7 +86,7 @@ struct iss_pipeline {
>> bool error;
>> struct v4l2_fract max_timeperframe;
>> struct v4l2_subdev *external;
>> - unsigned int external_rate;
>> + s64 external_lfreq;
>> int external_bpp;
>> };
>>
>
next prev parent reply other threads:[~2021-03-09 11:25 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-03 18:08 [RFC PATCH 0/4] use v4l2_get_link_freq() in CSI receiver drivers Andrey Konovalov
2021-03-03 18:08 ` [RFC PATCH 1/4] media: rcar-vin: use v4l2_get_link_freq() to calculate phypll frequency Andrey Konovalov
2021-03-08 13:46 ` Niklas Söderlund
2021-03-23 13:10 ` Sakari Ailus
2021-03-23 13:57 ` Niklas Söderlund
2021-03-23 21:24 ` Laurent Pinchart
2021-03-25 9:39 ` Niklas Söderlund
2021-03-30 17:53 ` Andrey Konovalov
2021-03-03 18:08 ` [RFC PATCH 2/4] media: ti-vpe: cal: use v4l2_get_link_freq() for DPHY timing configuration Andrey Konovalov
2021-03-03 18:08 ` [RFC PATCH 3/4] media: st-mipid02: use v4l2_get_link_freq() instead of the custom code Andrey Konovalov
2021-03-03 18:08 ` [RFC PATCH 4/4] staging: media: omap4iss: use v4l2_get_link_freq() to get the external rate Andrey Konovalov
2021-03-05 15:41 ` Sakari Ailus
2021-03-09 11:24 ` Andrey Konovalov [this message]
2021-03-23 12:54 ` Sakari Ailus
2021-03-05 15:42 ` [RFC PATCH 0/4] use v4l2_get_link_freq() in CSI receiver drivers Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=da7f0769-6f12-e895-8cf1-adf915bce64b@linaro.org \
--to=andrey.konovalov@linaro.org \
--cc=bparrot@ti.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=mickael.guene@st.com \
--cc=niklas.soderlund@ragnatech.se \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).