On 04/05, Sumera Priyadarsini wrote:
> Add two new functions vkms_composer_common() and vkms_crtc_composer().
> The actual plane composition work has been moved to the helper function,
> vkms_composer_common() which is called by both vkms_composer_worker()
> and vkms_crtc_composer(). vkms_crtc_composer() can be used when we
> implement virtual_hw mode.
Hi Sumera,
At this point, vkms_crtc_composer is never called. You should move this
function to the patch where you actually add virtual_hw mode, giving
meaning to create a different function (suitable for composition on
vblank-less mode). keep here only the prep work to add virtual hardware.
Thanks,
Melissa
>
> Signed-off-by: Sumera Priyadarsini <sylphrena...@gmail.com>
> ---
> Changes in V4:
> - Fix warning
> Changes in V3:
> - Refactor patchset (Melissa)
> Change in V2:
> - Add vkms_composer_common() (Daniel)
> ---
> drivers/gpu/drm/vkms/vkms_composer.c | 88 +++++++++++++++++-----------
> drivers/gpu/drm/vkms/vkms_drv.h | 3 +
> 2 files changed, 58 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> b/drivers/gpu/drm/vkms/vkms_composer.c
> index 66c6842d70db..0d2bad3ff849 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -169,6 +169,44 @@ static int compose_planes(void **vaddr_out,
> return 0;
> }
>
> +int vkms_composer_common(struct vkms_crtc_state *crtc_state,
> + struct vkms_output *out, bool wb_pending, uint32_t
> *crc32)
> +{
> + struct vkms_composer *primary_composer = NULL;
> + struct vkms_composer *cursor_composer = NULL;
> + void *vaddr_out = NULL;
> + int ret;
> +
> + if (crtc_state->num_active_planes >= 1)
> + primary_composer = crtc_state->active_planes[0]->composer;
> + if (crtc_state->num_active_planes == 2)
> + cursor_composer = crtc_state->active_planes[1]->composer;
> + if (!primary_composer)
> + return -EINVAL;
> + if (wb_pending)
> + vaddr_out = crtc_state->active_writeback;
> +
> + ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
> + if (ret) {
> + if (ret == -EINVAL && !wb_pending)
> + kfree(vaddr_out);
> + return -EINVAL;
> + }
> +
> + *crc32 = compute_crc(vaddr_out, primary_composer);
> +
> + if (wb_pending) {
> + drm_writeback_signal_completion(&out->wb_connector, 0);
> + spin_lock_irq(&out->composer_lock);
> + crtc_state->wb_pending = false;
> + spin_unlock_irq(&out->composer_lock);
> + } else {
> + kfree(vaddr_out);
> + }
> +
> + return 0;
> +}
> +
> /**
> * vkms_composer_worker - ordered work_struct to compute CRC
> *
> @@ -185,12 +223,9 @@ void vkms_composer_worker(struct work_struct *work)
> composer_work);
> struct drm_crtc *crtc = crtc_state->base.crtc;
> struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> - struct vkms_composer *primary_composer = NULL;
> - struct vkms_composer *cursor_composer = NULL;
> bool crc_pending, wb_pending;
> - void *vaddr_out = NULL;
> - u32 crc32 = 0;
> u64 frame_start, frame_end;
> + u32 crc32 = 0;
> int ret;
>
> spin_lock_irq(&out->composer_lock);
> @@ -210,36 +245,9 @@ void vkms_composer_worker(struct work_struct *work)
> if (!crc_pending)
> return;
>
> - if (crtc_state->num_active_planes >= 1)
> - primary_composer = crtc_state->active_planes[0]->composer;
> -
> - if (crtc_state->num_active_planes == 2)
> - cursor_composer = crtc_state->active_planes[1]->composer;
> -
> - if (!primary_composer)
> - return;
> -
> - if (wb_pending)
> - vaddr_out = crtc_state->active_writeback;
> -
> - ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
> - if (ret) {
> - if (ret == -EINVAL && !wb_pending)
> - kfree(vaddr_out);
> + ret = vkms_composer_common(crtc_state, out, wb_pending, &crc32);
> + if (ret == -EINVAL)
> return;
> - }
> -
> - crc32 = compute_crc(vaddr_out, primary_composer);
> -
> - if (wb_pending) {
> - drm_writeback_signal_completion(&out->wb_connector, 0);
> - spin_lock_irq(&out->composer_lock);
> - crtc_state->wb_pending = false;
> - spin_unlock_irq(&out->composer_lock);
> - } else {
> - kfree(vaddr_out);
> - }
> -
> /*
> * The worker can fall behind the vblank hrtimer, make sure we catch up.
> */
> @@ -247,6 +255,20 @@ void vkms_composer_worker(struct work_struct *work)
> drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);
> }
>
> +void vkms_crtc_composer(struct vkms_crtc_state *crtc_state)
> +{
> + struct drm_crtc *crtc = crtc_state->base.crtc;
> + struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> + u32 crc32 = 0;
> + int ret;
> +
> + ret = vkms_composer_common(crtc_state, out, crtc_state->wb_pending,
> &crc32);
> + if (ret == -EINVAL)
> + return;
> +
> + drm_crtc_add_crc_entry(crtc, true, 0, &crc32);
> +}
> +
> static const char * const pipe_crc_sources[] = {"auto"};
>
> const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 35540c7c4416..538315140585 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -125,8 +125,11 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const
> char *source_name,
> size_t *values_cnt);
>
> /* Composer Support */
> +int vkms_composer_common(struct vkms_crtc_state *crtc_state, struct
> vkms_output *out,
> + bool wb_pending, uint32_t *crcs);
> void vkms_composer_worker(struct work_struct *work);
> void vkms_set_composer(struct vkms_output *out, bool enabled);
> +void vkms_crtc_composer(struct vkms_crtc_state *crtc_state);
>
> /* Writeback */
> int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
> --
> 2.25.1
>
