Re: [PATCH 33/57] media: atomisp: ov2680: Add test pattern control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 1/23/23 19:46, Andy Shevchenko wrote:
> On Mon, Jan 23, 2023 at 01:51:41PM +0100, Hans de Goede wrote:
>> Add a test pattern control. This is a 1:1 copy of the test pattern
>> control in the main drivers/media/i2c/ov2680.c driver.
> 
> Hmm... I'm not sure I understand the trend of the changes.
> We have two drivers of the same sensor, correct?
> So, the idea is to move the AtomISP-specific one to be like
> the generic and then kill it eventually?

The goal is to kill one eventually yes. I'm not sure which
one to kill yet though. I have actually found a whole bunch
of bugs in the main drivers/media/i2c/ov2680.c code and
given its buggy-ness I wonder if anyone is actually using it.

I need to start an email thread about this (and a couple of
other open questions which I have), I have a bunch of notes
which I need to turn into emails for this.

> If so, why do we add something here?

Because I suspect that the atomisp version might eventually
be the one we want to keep (and move to drivers/media/i2c).

Regards,

Hans


> Code-wise it's okay change, but see above.
> Reviewed-by: Andy Shevchenko <andy@xxxxxxxxxx>
> 
>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>> ---
>>  .../media/atomisp/i2c/atomisp-ov2680.c        | 33 +++++++++++++++++++
>>  drivers/staging/media/atomisp/i2c/ov2680.h    |  3 ++
>>  2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
>> index 14002a1c22d2..6ca2a5bb0700 100644
>> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
>> @@ -127,6 +127,24 @@ static int ov2680_gain_set(struct ov2680_device *sensor, u32 gain)
>>  	return ovxxxx_write_reg16(sensor->client, OV2680_REG_GAIN_PK, gain);
>>  }
>>  
>> +static int ov2680_test_pattern_set(struct ov2680_device *sensor, int value)
>> +{
>> +	int ret;
>> +
>> +	if (!value)
>> +		return ovxxxx_mod_reg(sensor->client, OV2680_REG_ISP_CTRL00, BIT(7), 0);
>> +
>> +	ret = ovxxxx_mod_reg(sensor->client, OV2680_REG_ISP_CTRL00, 0x03, value - 1);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = ovxxxx_mod_reg(sensor->client, OV2680_REG_ISP_CTRL00, BIT(7), BIT(7));
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>>  static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl)
>>  {
>>  	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
>> @@ -151,6 +169,9 @@ static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl)
>>  	case V4L2_CID_GAIN:
>>  		ret = ov2680_gain_set(sensor, ctrl->val);
>>  		break;
>> +	case V4L2_CID_TEST_PATTERN:
>> +		ret = ov2680_test_pattern_set(sensor, ctrl->val);
>> +		break;
>>  	default:
>>  		ret = -EINVAL;
>>  	}
>> @@ -644,6 +665,13 @@ static const struct v4l2_subdev_ops ov2680_ops = {
>>  
>>  static int ov2680_init_controls(struct ov2680_device *sensor)
>>  {
>> +	static const char * const test_pattern_menu[] = {
>> +		"Disabled",
>> +		"Color Bars",
>> +		"Random Data",
>> +		"Square",
>> +		"Black Image",
>> +	};
>>  	const struct v4l2_ctrl_ops *ops = &ov2680_ctrl_ops;
>>  	struct ov2680_ctrls *ctrls = &sensor->ctrls;
>>  	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
>> @@ -658,6 +686,11 @@ static int ov2680_init_controls(struct ov2680_device *sensor)
>>  	ctrls->exposure = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE,
>>  					    0, exp_max, 1, exp_max);
>>  	ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAIN, 0, 1023, 1, 250);
>> +	ctrls->test_pattern =
>> +		v4l2_ctrl_new_std_menu_items(hdl,
>> +					     &ov2680_ctrl_ops, V4L2_CID_TEST_PATTERN,
>> +					     ARRAY_SIZE(test_pattern_menu) - 1,
>> +					     0, 0, test_pattern_menu);
>>  
>>  	ctrls->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
>>  	ctrls->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
>> diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h b/drivers/staging/media/atomisp/i2c/ov2680.h
>> index e3ad20a7ffd5..45526477b612 100644
>> --- a/drivers/staging/media/atomisp/i2c/ov2680.h
>> +++ b/drivers/staging/media/atomisp/i2c/ov2680.h
>> @@ -120,6 +120,8 @@
>>  #define OV2680_MWB_BLUE_GAIN_H			0x5008/*0x3404*/
>>  #define OV2680_MWB_GAIN_MAX				0x0fff
>>  
>> +#define OV2680_REG_ISP_CTRL00			0x5080
>> +
>>  #define OV2680_START_STREAMING			0x01
>>  #define OV2680_STOP_STREAMING			0x00
>>  
>> @@ -171,6 +173,7 @@ struct ov2680_device {
>>  		struct v4l2_ctrl *vflip;
>>  		struct v4l2_ctrl *exposure;
>>  		struct v4l2_ctrl *gain;
>> +		struct v4l2_ctrl *test_pattern;
>>  	} ctrls;
>>  };
>>  
>> -- 
>> 2.39.0
>>
> 





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux