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.

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;
> >     }
> >  
> 

-- 
Regards,
Niklas Söderlund
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to