Hi Laurent,
On 22/04/18 23:34, Laurent Pinchart wrote:
Add a parameter (in the form of a structure to ease future API extensions) to the VSP atomic flush handler to pass CRC source configuration, and pass the CRC value to the completion callback.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Only a minor 'thought' below. (And Jacopo already caught the enum typos, so with those are fixed...)
Reviewed-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 6 ++++-- drivers/media/platform/vsp1/vsp1_drm.c | 6 ++++-- drivers/media/platform/vsp1/vsp1_drm.h | 2 +- include/media/vsp1.h | 29 +++++++++++++++++++++++++++-- 4 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c260c33840b..bdcec201591f 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -31,7 +31,7 @@ #include "rcar_du_kms.h" #include "rcar_du_vsp.h"
-static void rcar_du_vsp_complete(void *private, bool completed) +static void rcar_du_vsp_complete(void *private, bool completed, u32 crc) { struct rcar_du_crtc *crtc = private;
@@ -102,7 +102,9 @@ void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc) {
- vsp1_du_atomic_flush(crtc->vsp->vsp, crtc->vsp_pipe);
- struct vsp1_du_atomic_pipe_config cfg = { { 0, } }> +
- vsp1_du_atomic_flush(crtc->vsp->vsp, crtc->vsp_pipe, &cfg);
}
/* Keep the two tables in sync. */ diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 2b29a83dceb9..5fc31578f9b0 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -36,7 +36,7 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe, bool complete = completion == VSP1_DL_FRAME_END_COMPLETED;
if (drm_pipe->du_complete)
drm_pipe->du_complete(drm_pipe->du_private, complete);
drm_pipe->du_complete(drm_pipe->du_private, complete, 0);if (completion & VSP1_DL_FRAME_END_INTERNAL) { drm_pipe->force_brx_release = false;
@@ -739,8 +739,10 @@ EXPORT_SYMBOL_GPL(vsp1_du_atomic_update);
- vsp1_du_atomic_flush - Commit an atomic update
- @dev: the VSP device
- @pipe_index: the DRM pipeline index
*/
- @cfg: atomic pipe configuration
-void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index) +void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
const struct vsp1_du_atomic_pipe_config *cfg){ struct vsp1_device *vsp1 = dev_get_drvdata(dev); struct vsp1_drm_pipeline *drm_pipe = &vsp1->drm->pipe[pipe_index]; diff --git a/drivers/media/platform/vsp1/vsp1_drm.h b/drivers/media/platform/vsp1/vsp1_drm.h index f4af1b2b12d6..e5b88b28806c 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.h +++ b/drivers/media/platform/vsp1/vsp1_drm.h @@ -35,7 +35,7 @@ struct vsp1_drm_pipeline { wait_queue_head_t wait_queue;
/* Frame synchronisation */
- void (*du_complete)(void *, bool);
- void (*du_complete)(void *data, bool completed, u32 crc); void *du_private;
};
diff --git a/include/media/vsp1.h b/include/media/vsp1.h index ff7ef894465d..ac63a9928a79 100644 --- a/include/media/vsp1.h +++ b/include/media/vsp1.h @@ -34,7 +34,7 @@ struct vsp1_du_lif_config { unsigned int width; unsigned int height;
- void (*callback)(void *, bool);
- void (*callback)(void *data, bool completed, u32 crc);
Did we have to add parameters to this callback for the dynamic BRx work ?
</me struggles to remember correctly>. If so we might have to consider turning it into a struct perhaps! - but for now or otherwise I think this will be OK.
Ok - so I stopped being lazy and went to take a look. We turned completed into a bitfield - so I think actually that's enough to store any more flags we might come up with, and the CRC doesn't fit in there :D
So lets leave this as adding an extra parameter for now.
void *callback_data; };
@@ -61,11 +61,36 @@ struct vsp1_du_atomic_config { unsigned int zpos; };
+/**
- enum vsp1_du_crc_source - Source used for CRC calculation
- @VSP1_DU_CRC_NONE: CRC calculation disabled
- @VSP_DU_CRC_PLANE: Perform CRC calculation on an input plane
- @VSP_DU_CRC_OUTPUT: Perform CRC calculation on the composed output
I see Jacopo has already caught these ...
- */
+enum vsp1_du_crc_source {
- VSP1_DU_CRC_NONE,
- VSP1_DU_CRC_PLANE,
- VSP1_DU_CRC_OUTPUT,
+};
+/**
- struct vsp1_du_atomic_pipe_config - VSP atomic pipe configuration parameters
- @crc.source: source for CRC calculation
- @crc.index: index of the CRC source plane (when crc.source is set to plane)
- */
+struct vsp1_du_atomic_pipe_config {
- struct {
enum vsp1_du_crc_source source;unsigned int index;- } crc;
+};
void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index); int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index, unsigned int rpf, const struct vsp1_du_atomic_config *cfg); -void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index); +void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
const struct vsp1_du_atomic_pipe_config *cfg);int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt); void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);