| Message ID | 20231018131729.1022521-1-umang.jain@ideasonboard.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | media: css: Write LINE_LENGTH_PCK correctly | expand |
Hi Umang, On Wed, Oct 18, 2023 at 06:47:29PM +0530, Umang Jain wrote: > According to MIPI CCS v1.1 specification, the LINE_LENGTH_PCK > units is in VT pixel clocks (Section 8.2.6). > > To compute how many pixel clocks it takes, simply divide the > VT pixel clock frequency by the number of pixels in a single line. Have you hard of dimensional analysis (https://en.wikipedia.org/wiki/Dimensional_analysis) ? It's a very good and simple way of checking the validity of mathematical formulas. > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > Testing: > > The patch is tested using IMX519 with CCS. > This patch makes the frame buffer being filled to the fullest > (instead of getting 1/3rd only previously) - without any workarounds. > --- > drivers/media/i2c/ccs/ccs-core.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c > index 16de66a37fad..12c75a843dfe 100644 > --- a/drivers/media/i2c/ccs/ccs-core.c > +++ b/drivers/media/i2c/ccs/ccs-core.c > @@ -734,9 +734,11 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl) > > break; > case V4L2_CID_HBLANK: > - rval = ccs_write(sensor, LINE_LENGTH_PCK, > - sensor->pixel_array->crop[CCS_PA_PAD_SRC].width > - + ctrl->val); > + /* LINE_LENGTH_PCK units are in VT pixel clocks. */ > + u16 line_length_pck = > + sensor->pll.vt_bk.pix_clk_freq_hz / > + (sensor->pixel_array->crop[CCS_PA_PAD_SRC].width + ctrl->val); > + rval = ccs_write(sensor, LINE_LENGTH_PCK, line_length_pck); > > break; > case V4L2_CID_TEST_PATTERN:
hi Umang, On Wed, Oct 18, 2023 at 06:47:29PM +0530, Umang Jain wrote: > According to MIPI CCS v1.1 specification, the LINE_LENGTH_PCK > units is in VT pixel clocks (Section 8.2.6). > > To compute how many pixel clocks it takes, simply divide the > VT pixel clock frequency by the number of pixels in a single line. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > Testing: > > The patch is tested using IMX519 with CCS. > This patch makes the frame buffer being filled to the fullest > (instead of getting 1/3rd only previously) - without any workarounds. > --- > drivers/media/i2c/ccs/ccs-core.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c > index 16de66a37fad..12c75a843dfe 100644 > --- a/drivers/media/i2c/ccs/ccs-core.c > +++ b/drivers/media/i2c/ccs/ccs-core.c > @@ -734,9 +734,11 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl) > > break; > case V4L2_CID_HBLANK: > - rval = ccs_write(sensor, LINE_LENGTH_PCK, > - sensor->pixel_array->crop[CCS_PA_PAD_SRC].width > - + ctrl->val); > + /* LINE_LENGTH_PCK units are in VT pixel clocks. */ > + u16 line_length_pck = > + sensor->pll.vt_bk.pix_clk_freq_hz / > + (sensor->pixel_array->crop[CCS_PA_PAD_SRC].width + ctrl->val); The time of processing one pixel in the pixel array takes a single VT pixel clock. This register configures how many you have per line, i.e. analogue crop width + horizontal blanking --- which is what the driver does without this patch. I'm still not suggesting the default value for LINE_LENGTH_PCK CCS driver programs is correct value for this device. The issue is still almost certainly in the related limit register values in the sensor's register address space, and that should be addressed by using CCS static data. > + rval = ccs_write(sensor, LINE_LENGTH_PCK, line_length_pck); > > break; > case V4L2_CID_TEST_PATTERN:
Hi Sakari, On 10/18/23 7:18 PM, Sakari Ailus wrote: > hi Umang, > > On Wed, Oct 18, 2023 at 06:47:29PM +0530, Umang Jain wrote: >> According to MIPI CCS v1.1 specification, the LINE_LENGTH_PCK >> units is in VT pixel clocks (Section 8.2.6). >> >> To compute how many pixel clocks it takes, simply divide the >> VT pixel clock frequency by the number of pixels in a single line. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> Testing: >> >> The patch is tested using IMX519 with CCS. >> This patch makes the frame buffer being filled to the fullest >> (instead of getting 1/3rd only previously) - without any workarounds. >> --- >> drivers/media/i2c/ccs/ccs-core.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c >> index 16de66a37fad..12c75a843dfe 100644 >> --- a/drivers/media/i2c/ccs/ccs-core.c >> +++ b/drivers/media/i2c/ccs/ccs-core.c >> @@ -734,9 +734,11 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl) >> >> break; >> case V4L2_CID_HBLANK: >> - rval = ccs_write(sensor, LINE_LENGTH_PCK, >> - sensor->pixel_array->crop[CCS_PA_PAD_SRC].width >> - + ctrl->val); >> + /* LINE_LENGTH_PCK units are in VT pixel clocks. */ >> + u16 line_length_pck = >> + sensor->pll.vt_bk.pix_clk_freq_hz / >> + (sensor->pixel_array->crop[CCS_PA_PAD_SRC].width + ctrl->val); > The time of processing one pixel in the pixel array takes a single VT pixel I mis-interpreted this, where it got all wrong. I assumed single VT pixel clock = no. of pixels processed in one second which is not correct. I have been pointed out that pixel clock and pixel clock rate (frequency I suppose) are different. I thought in terms of "Hz" :-S So yes if 1 VT pixel clock is duration of one pixel processing from pixel_array, the line_length_pck is correct without this patch. > clock. This register configures how many you have per line, i.e. analogue > crop width + horizontal blanking --- which is what the driver does without > this patch. > > I'm still not suggesting the default value for LINE_LENGTH_PCK CCS driver > programs is correct value for this device. The issue is still almost > certainly in the related limit register values in the sensor's register > address space, and that should be addressed by using CCS static data. > >> + rval = ccs_write(sensor, LINE_LENGTH_PCK, line_length_pck); >> >> break; >> case V4L2_CID_TEST_PATTERN:
diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c index 16de66a37fad..12c75a843dfe 100644 --- a/drivers/media/i2c/ccs/ccs-core.c +++ b/drivers/media/i2c/ccs/ccs-core.c @@ -734,9 +734,11 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl) break; case V4L2_CID_HBLANK: - rval = ccs_write(sensor, LINE_LENGTH_PCK, - sensor->pixel_array->crop[CCS_PA_PAD_SRC].width - + ctrl->val); + /* LINE_LENGTH_PCK units are in VT pixel clocks. */ + u16 line_length_pck = + sensor->pll.vt_bk.pix_clk_freq_hz / + (sensor->pixel_array->crop[CCS_PA_PAD_SRC].width + ctrl->val); + rval = ccs_write(sensor, LINE_LENGTH_PCK, line_length_pck); break; case V4L2_CID_TEST_PATTERN:
According to MIPI CCS v1.1 specification, the LINE_LENGTH_PCK units is in VT pixel clocks (Section 8.2.6). To compute how many pixel clocks it takes, simply divide the VT pixel clock frequency by the number of pixels in a single line. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- Testing: The patch is tested using IMX519 with CCS. This patch makes the frame buffer being filled to the fullest (instead of getting 1/3rd only previously) - without any workarounds. --- drivers/media/i2c/ccs/ccs-core.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)