Re: [PATCHv2 7/7] [PATCHv5] media: adv7180: fix field type

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

 




On 08/03/2016 03:21 PM, Niklas Söderlund wrote:
> On 2016-08-02 17:00:07 +0200, Lars-Peter Clausen wrote:
>> [...]
>>> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
>>> index a8b434b..c6fed71 100644
>>> --- a/drivers/media/i2c/adv7180.c
>>> +++ b/drivers/media/i2c/adv7180.c
>>> @@ -680,10 +680,13 @@ static int adv7180_set_pad_format(struct v4l2_subdev *sd,
>>>  	switch (format->format.field) {
>>>  	case V4L2_FIELD_NONE:
>>>  		if (!(state->chip_info->flags & ADV7180_FLAG_I2P))
>>> -			format->format.field = V4L2_FIELD_INTERLACED;
>>> +			format->format.field = V4L2_FIELD_ALTERNATE;
>>>  		break;
>>>  	default:
>>> -		format->format.field = V4L2_FIELD_INTERLACED;
>>> +		if (state->chip_info->flags & ADV7180_FLAG_I2P)
>>> +			format->format.field = V4L2_FIELD_INTERLACED;
>>
>> I'm not convinced this is correct. As far as I understand it when the I2P
>> feature is enabled the core outputs full progressive frames at the full
>> framerate. If it is bypassed it outputs half-frames. So we have the option
>> of either V4L2_FIELD_NONE or V4L2_FIELD_ALTERNATE, but never interlaced. I
>> think this branch should setup the field format to be ALTERNATE regardless
>> of whether the I2P feature is available.

Actually, that's not true. If the progressive frame is obtained by combining
two fields, then it should return FIELD_INTERLACED. This is how most SDTV
receivers operate.

FIELD_NONE is reserved for pure progressive formats like 720p.

Regards,

	Hans

> 
> I be happy to update the patch in such manner, but I feel this is more 
> for Steven to handle. I have no deep understanding of the adv7180 driver 
> and the only HW I have is the adv7180 and not adv7280, adv7280_m, 
> adv7282 or adv7282_m which is the models which have the ADV7180_FLAG_I2P 
> flag. So I can't really test such a change.
> 
> Steven do you want to update this patch and resend it? I can drop it 
> from this series altogether since the rcar-vin changes do not depend on 
> it. I only kept it since I want the rcar-vin changes to be picked up 
> before the adv7180 changes, otherwise the rcar-vin driver will stop to 
> function on Koelsch. I can also ofc make the change suggested by Lars if 
> you prefer, but then I want your blessing to do so. I have already 
> changed so much of your original patch :-)
> 
>>
>>> +		else
>>> +			format->format.field = V4L2_FIELD_ALTERNATE;
>>>  		break;
>>>  	}
>>>  
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux