[PATCH v4 10/11] media: vsp1: Support Interlaced display pipelines

[Kieran Bingham]

Hi Laurent,

<snip>

>>> +static void vsp1_rpf_configure_autofld(struct vsp1_rwpf *rpf,
>>> +				       struct vsp1_dl_ext_cmd *cmd)
>>> +{
>>> +	const struct v4l2_pix_format_mplane *format = &rpf->format;
>>> +	struct vsp1_extcmd_auto_fld_body *auto_fld = cmd->data;
>>> +	u32 offset_y, offset_c;
>>> +
>>> +	/* Re-index our auto_fld to match the current RPF */
>>
>> s/RPF/RPF./
> 
> Fixed.
> 
>>
>>> +	auto_fld = &auto_fld[rpf->entity.index];
>>> +
>>> +	auto_fld->top_y0 = rpf->mem.addr[0];
>>> +	auto_fld->top_c0 = rpf->mem.addr[1];
>>> +	auto_fld->top_c1 = rpf->mem.addr[2];
>>> +
>>> +	offset_y = format->plane_fmt[0].bytesperline;
>>> +	offset_c = format->plane_fmt[1].bytesperline;
>>> +
>>> +	auto_fld->bottom_y0 = rpf->mem.addr[0] + offset_y;
>>> +	auto_fld->bottom_c0 = rpf->mem.addr[1] + offset_c;
>>> +	auto_fld->bottom_c1 = rpf->mem.addr[2] + offset_c;
>>> +
>>> +	cmd->flags |= VSP1_DL_EXT_AUTOFLD_INT;
>>> +	cmd->flags |= BIT(16 + rpf->entity.index);
>>
>> Do you expect some flags to already be set ? If not, couldn't we assign the 
>> value to the field instead of OR'ing it ?

> No, I think you are correct. Moved to a single expression setting the
> cmd->flags in one line.

Ahem.... no - of course these flags have to be OR-ed in. Because it
potentially updates a single command object for multiple RPFs.

The flags get reset to 0 when the command object is discarded in
vsp1_dl_ext_cmd_put()


> 
>>
>>> +}

<snip>

--
Kieran