Hi Kieran,
On Monday, 16 July 2018 21:21:00 EEST Kieran Bingham wrote:
On 24/05/18 13:51, Laurent Pinchart wrote:
On Thursday, 3 May 2018 16:36:21 EEST Kieran Bingham wrote:
Calculate the top and bottom fields for the interlaced frames and utilise the extended display list command feature to implement the auto-field operations. This allows the DU to update the VSP2 registers dynamically based upon the currently processing field.
Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com
v3:
- Pass DL through partition calls to allow autocmd's to be retrieved
- Document interlaced field in struct vsp1_du_atomic_config
v2:
- fix erroneous BIT value which enabled interlaced
- fix field handling at frame_end interrupt
drivers/media/platform/vsp1/vsp1_dl.c | 10 ++++- drivers/media/platform/vsp1/vsp1_drm.c | 11 ++++- drivers/media/platform/vsp1/vsp1_regs.h | 1 +- drivers/media/platform/vsp1/vsp1_rpf.c | 71 ++++++++++++++++++++++++-- drivers/media/platform/vsp1/vsp1_rwpf.h | 1 +- include/media/vsp1.h | 2 +- 6 files changed, 93 insertions(+), 3 deletions(-)
[snip]
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 2c3db8b8adce..cc29c9d96bb7 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -811,6 +811,17 @@ int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index, return -EINVAL;
}
- if (!(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) && cfg->interlaced) {
Nitpicking, writing the condition as
if (cfg->interlaced && !(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)))
Done.
would match the comment better. You can also drop the parentheses around the vsp1_feature() call.
/** Interlaced support requires extended display lists to* provide the auto-fld feature with the DU.*/dev_dbg(vsp1->dev, "Interlaced unsupported on this output\n");Could we catch this in the DU driver to fail atomic test ?
Ugh - I thought moving the configuration to vsp1_du_setup_lif() would give us this, but that return value is not checked in the DU.
How can we interogate the VSP1 to ask it if it supports interlaced from rcar_du_vsp_plane_atomic_check()?
Some dummy call to vsp1_du_setup_lif() to check the return value ? Or should we implement a hook to call through to perform checks in the VSP1 DRM API?
Would it be possible to just infer that from the DU compatible string, without querying the VSP driver ? Of course that's a bit of a layering violation, but as we know what type of VSP instance is present in each SoC, such a small hack wouldn't hurt in my opinion. If the need arises later we can introduce an API to query the information from the VSP driver.
return -EINVAL;- }
- rpf->interlaced = cfg->interlaced;
- rpf->fmtinfo = fmtinfo; rpf->format.num_planes = fmtinfo->planes; rpf->format.plane_fmt[0].bytesperline = cfg->pitch;
[snip]