Re: [PATCH V4 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode

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 sylphrenadin@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