| Message ID | 0fa6a92c613087705bd8c6f7131cdaf3b1d0c8c2.1530133610.git.hamohammed.sa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 28, 2018 at 10:07 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Jun 28, 2018 at 12:24:35AM +0300, Haneen Mohammed wrote: >> Implement the .set_crc_source() callback. >> Compute CRC using crc32 on the visible part of the framebuffer. >> Use work_struct to compute and add CRC at the end of a vblank. >> >> Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com> > > Ok locking review. I still don't have a clear idea how to best fix this, > but oh well. > >> --- >> drivers/gpu/drm/vkms/vkms_crtc.c | 76 ++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/vkms/vkms_drv.h | 16 +++++++ >> 2 files changed, 92 insertions(+) >> >> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c >> index 73aae129c37d..d78934b76e33 100644 >> --- a/drivers/gpu/drm/vkms/vkms_crtc.c >> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c >> @@ -7,8 +7,78 @@ >> */ >> >> #include "vkms_drv.h" >> +#include <linux/crc32.h> >> #include <drm/drm_atomic_helper.h> >> #include <drm/drm_crtc_helper.h> >> +#include <drm/drm_gem_framebuffer_helper.h> >> + >> +uint32_t _vkms_get_crc(struct drm_framebuffer *fb) >> +{ >> + struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); >> + struct vkms_gem_object *vkms_obj = container_of(gem_obj, >> + struct vkms_gem_object, >> + gem); >> + u32 crc = 0; >> + int i = 0; >> + struct drm_plane_state *state = vkms_obj->plane_state; > > vkms_obj->plane_state is the first locking issue: You store the pointer > without any locks or synchronization, which means the crc worker here > could access is while it's being changed, resulting in an inconsistent > crc. Note that the compiler/cpu is allowed to read a pointer multiple > times if it's not protected by locking/barriers, so all kinds of funny > stuff can happen here. > > Second issue is that the memory you're pointing to isn't owned by the crc > subsystem, but by the atomic commit worker. That could release the memory > (and it might get reused for something else meanwhile) before the crc > worker here is done with it. > >> + unsigned int x = state->src.x1 >> 16; >> + unsigned int y = state->src.y1 >> 16; >> + unsigned int height = drm_rect_height(&state->src) >> 16; >> + unsigned int width = drm_rect_width(&state->src) >> 16; >> + unsigned int cpp = fb->format->cpp[0]; >> + unsigned int src_offset; >> + unsigned int size_byte = width * cpp; >> + void *vaddr = vkms_obj->vaddr; >> + >> + if (WARN_ON(!vaddr)) >> + return crc; >> + >> + for (i = y; i < y + height; i++) { >> + src_offset = fb->offsets[0] + (i * fb->pitches[0]) + (x * cpp); >> + crc = crc32_le(crc, vaddr + src_offset, size_byte); >> + } >> + >> + return crc; >> +} >> + >> +void vkms_crc_work_handle(struct work_struct *work) >> +{ >> + struct vkms_crtc_state *crtc_state = container_of(work, >> + struct vkms_crtc_state, >> + crc_workq); >> + struct drm_crtc *crtc = crtc_state->crtc; >> + struct drm_framebuffer *fb = (crtc->primary ? crtc->primary->fb : NULL); > > The drm_plane->fb pointer has similar issues: No locking (the only thing > that's really allowed to change this is the atomic commit worker for > atomic drivers, nothing else), so same issues with inconsistent data and > using possibly freed memory. > > On top Ville has a patch series to set drm_plane->fb == NULL for all > atomic drivers - it's really a leftover from the pre-atomic age and > doesn't fit for atomic. Ville just said that he pushed all that. Are you sure that you're on latest drm-tip? On latest drm-tip primary->fb should be always NULL for an atomic driver like vkms ... If you're not on latest drm-tip probably good idea to rebase everything. -Daniel > > On top of these issues for ->plane_state and ->fb we have a third issue: > The crc computation must be atomic wrt the generated vblank event for a > given atomic update. That means we must make sure that the crc we're > generating is exactly for the plane_state/fb that also issued the vblank > event. There's some atomic kms tests in igt that check this. More > concretely we need to make sure that: > - if the vblank code sees the new vblank event, then the crc _must_ > compute the crc for the new configuration. > - if the vblank code does not yet see the new vblank event, then the crc > _must_ compute the crc for the old configuration. > > One simple solution, but quite a bit of code to type would be. > > 1. add a spinlock to the vkms_crtc structure (probably need that first) to > protect all the crc/vblank event stuff. Needs to be a spinlock since we > must look at this from the hrtimer. > > 2. Create a new structure where you make a copy (not just pointers) of all > the data the hrtimer needs. So src offset, vblank event, all that stuff. > For the fb pointer you need to make sure it's properly reference counted > (drm_framebuffer_get/put). > > 3. For every atomic update, allocate a new such structure and store it in > the vkms_crtc structure (holding the spinlock while updating the pointer). > > 4. In the hrtimer grab that pointer and set it to NULL (again holding the > spinlock), and then explicitly send out the vblank event (using > drm_crtc_send_vblank_event). And then pass that structure with everything > the crc worker needs to the crc worker. > > I think this should work, mostly. > -Daniel > >> + >> + if (crtc_state->crc_enabled && fb) { >> + u32 crc32 = _vkms_get_crc(fb); >> + unsigned int n_frame = drm_crtc_accurate_vblank_count(crtc); >> + >> + drm_crtc_add_crc_entry(crtc, true, n_frame, &crc32); >> + } >> +} >> + >> +static int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name, >> + size_t *values_cnt) >> +{ >> + struct drm_device *dev = crtc->dev; >> + struct vkms_device *vkms_dev = container_of(dev, >> + struct vkms_device, >> + drm); >> + struct vkms_crtc_state *crtc_state = &vkms_dev->output.crtc_state; >> + >> + if (!src_name) { >> + crtc_state->crc_enabled = false; >> + } else if (strcmp(src_name, "auto") == 0) { >> + crtc_state->crc_enabled = true; >> + } else { >> + crtc_state->crc_enabled = false; >> + return -EINVAL; >> + } >> + >> + *values_cnt = 1; >> + >> + return 0; >> +} >> >> static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) >> { >> @@ -23,6 +93,8 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) >> if (!ret) >> DRM_ERROR("vkms failure on handling vblank"); >> >> + vkms_crc_work_handle(&output->crtc_state.crc_workq); >> + >> spin_lock_irqsave(&crtc->dev->event_lock, flags); >> if (output->event) { >> drm_crtc_send_vblank_event(crtc, output->event); >> @@ -46,6 +118,9 @@ static int vkms_enable_vblank(struct drm_crtc *crtc) >> out->period_ns = ktime_set(0, DEFAULT_VBLANK_NS); >> hrtimer_start(&out->vblank_hrtimer, out->period_ns, HRTIMER_MODE_ABS); >> >> + out->crtc_state.crtc = crtc; >> + INIT_WORK(&out->crtc_state.crc_workq, vkms_crc_work_handle); >> + >> return 0; >> } >> >> @@ -65,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = { >> .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, >> .enable_vblank = vkms_enable_vblank, >> .disable_vblank = vkms_disable_vblank, >> + .set_crc_source = vkms_set_crc_source, >> }; >> >> static int vkms_crtc_atomic_check(struct drm_crtc *crtc, >> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h >> index 300e6a05d473..4a1458731dd7 100644 >> --- a/drivers/gpu/drm/vkms/vkms_drv.h >> +++ b/drivers/gpu/drm/vkms/vkms_drv.h >> @@ -22,6 +22,18 @@ static const u32 vkms_formats[] = { >> DRM_FORMAT_XRGB8888, >> }; >> >> +/** >> + * struct vkms_crtc_state >> + * @crtc: backpointer to the CRTC >> + * @crc_workq: worker that captures CRCs for each frame >> + * @crc_enabled: flag to test if crc is enabled >> + */ >> +struct vkms_crtc_state { >> + struct drm_crtc *crtc; >> + struct work_struct crc_workq; >> + bool crc_enabled; >> +}; >> + >> struct vkms_output { >> struct drm_crtc crtc; >> struct drm_encoder encoder; >> @@ -29,6 +41,7 @@ struct vkms_output { >> struct hrtimer vblank_hrtimer; >> ktime_t period_ns; >> struct drm_pending_vblank_event *event; >> + struct vkms_crtc_state crtc_state; >> }; >> >> struct vkms_device { >> @@ -55,6 +68,9 @@ int vkms_output_init(struct vkms_device *vkmsdev); >> >> struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); >> >> +/* CRC Support */ >> +void vkms_crc_work_handle(struct work_struct *work); >> + >> /* Gem stuff */ >> struct drm_gem_object *vkms_gem_create(struct drm_device *dev, >> struct drm_file *file, >> -- >> 2.17.1 >> > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Thu, Jun 28, 2018 at 01:40:08PM +0200, Daniel Vetter wrote: > On Thu, Jun 28, 2018 at 10:07 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Thu, Jun 28, 2018 at 12:24:35AM +0300, Haneen Mohammed wrote: > >> Implement the .set_crc_source() callback. > >> Compute CRC using crc32 on the visible part of the framebuffer. > >> Use work_struct to compute and add CRC at the end of a vblank. > >> > >> Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com> > > > > Ok locking review. I still don't have a clear idea how to best fix this, > > but oh well. > > > >> --- > >> drivers/gpu/drm/vkms/vkms_crtc.c | 76 ++++++++++++++++++++++++++++++++ > >> drivers/gpu/drm/vkms/vkms_drv.h | 16 +++++++ > >> 2 files changed, 92 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > >> index 73aae129c37d..d78934b76e33 100644 > >> --- a/drivers/gpu/drm/vkms/vkms_crtc.c > >> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > >> @@ -7,8 +7,78 @@ > >> */ > >> > >> #include "vkms_drv.h" > >> +#include <linux/crc32.h> > >> #include <drm/drm_atomic_helper.h> > >> #include <drm/drm_crtc_helper.h> > >> +#include <drm/drm_gem_framebuffer_helper.h> > >> + > >> +uint32_t _vkms_get_crc(struct drm_framebuffer *fb) > >> +{ > >> + struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); > >> + struct vkms_gem_object *vkms_obj = container_of(gem_obj, > >> + struct vkms_gem_object, > >> + gem); > >> + u32 crc = 0; > >> + int i = 0; > >> + struct drm_plane_state *state = vkms_obj->plane_state; > > > > vkms_obj->plane_state is the first locking issue: You store the pointer > > without any locks or synchronization, which means the crc worker here > > could access is while it's being changed, resulting in an inconsistent > > crc. Note that the compiler/cpu is allowed to read a pointer multiple > > times if it's not protected by locking/barriers, so all kinds of funny > > stuff can happen here. > > > > Second issue is that the memory you're pointing to isn't owned by the crc > > subsystem, but by the atomic commit worker. That could release the memory > > (and it might get reused for something else meanwhile) before the crc > > worker here is done with it. > > > >> + unsigned int x = state->src.x1 >> 16; > >> + unsigned int y = state->src.y1 >> 16; > >> + unsigned int height = drm_rect_height(&state->src) >> 16; > >> + unsigned int width = drm_rect_width(&state->src) >> 16; > >> + unsigned int cpp = fb->format->cpp[0]; > >> + unsigned int src_offset; > >> + unsigned int size_byte = width * cpp; > >> + void *vaddr = vkms_obj->vaddr; > >> + > >> + if (WARN_ON(!vaddr)) > >> + return crc; > >> + > >> + for (i = y; i < y + height; i++) { > >> + src_offset = fb->offsets[0] + (i * fb->pitches[0]) + (x * cpp); > >> + crc = crc32_le(crc, vaddr + src_offset, size_byte); > >> + } > >> + > >> + return crc; > >> +} > >> + > >> +void vkms_crc_work_handle(struct work_struct *work) > >> +{ > >> + struct vkms_crtc_state *crtc_state = container_of(work, > >> + struct vkms_crtc_state, > >> + crc_workq); > >> + struct drm_crtc *crtc = crtc_state->crtc; > >> + struct drm_framebuffer *fb = (crtc->primary ? crtc->primary->fb : NULL); > > > > The drm_plane->fb pointer has similar issues: No locking (the only thing > > that's really allowed to change this is the atomic commit worker for > > atomic drivers, nothing else), so same issues with inconsistent data and > > using possibly freed memory. > > > > On top Ville has a patch series to set drm_plane->fb == NULL for all > > atomic drivers - it's really a leftover from the pre-atomic age and > > doesn't fit for atomic. > > Ville just said that he pushed all that. Are you sure that you're on > latest drm-tip? On latest drm-tip primary->fb should be always NULL > for an atomic driver like vkms ... If you're not on latest drm-tip > probably good idea to rebase everything. > -Daniel > Ok I will work on that, thank you so much! I did update my branch recently but then that resulted in networking problems so I reverted the update. I will fix that issue first and then work on the locking issues. Thank you so much again! Haneen > > > > On top of these issues for ->plane_state and ->fb we have a third issue: > > The crc computation must be atomic wrt the generated vblank event for a > > given atomic update. That means we must make sure that the crc we're > > generating is exactly for the plane_state/fb that also issued the vblank > > event. There's some atomic kms tests in igt that check this. More > > concretely we need to make sure that: > > - if the vblank code sees the new vblank event, then the crc _must_ > > compute the crc for the new configuration. > > - if the vblank code does not yet see the new vblank event, then the crc > > _must_ compute the crc for the old configuration. > > > > One simple solution, but quite a bit of code to type would be. > > > > 1. add a spinlock to the vkms_crtc structure (probably need that first) to > > protect all the crc/vblank event stuff. Needs to be a spinlock since we > > must look at this from the hrtimer. > > > > 2. Create a new structure where you make a copy (not just pointers) of all > > the data the hrtimer needs. So src offset, vblank event, all that stuff. > > For the fb pointer you need to make sure it's properly reference counted > > (drm_framebuffer_get/put). > > > > 3. For every atomic update, allocate a new such structure and store it in > > the vkms_crtc structure (holding the spinlock while updating the pointer). > > > > 4. In the hrtimer grab that pointer and set it to NULL (again holding the > > spinlock), and then explicitly send out the vblank event (using > > drm_crtc_send_vblank_event). And then pass that structure with everything > > the crc worker needs to the crc worker. > > > > I think this should work, mostly. > > -Daniel > > > >> + > >> + if (crtc_state->crc_enabled && fb) { > >> + u32 crc32 = _vkms_get_crc(fb); > >> + unsigned int n_frame = drm_crtc_accurate_vblank_count(crtc); > >> + > >> + drm_crtc_add_crc_entry(crtc, true, n_frame, &crc32); > >> + } > >> +} > >> + > >> +static int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name, > >> + size_t *values_cnt) > >> +{ > >> + struct drm_device *dev = crtc->dev; > >> + struct vkms_device *vkms_dev = container_of(dev, > >> + struct vkms_device, > >> + drm); > >> + struct vkms_crtc_state *crtc_state = &vkms_dev->output.crtc_state; > >> + > >> + if (!src_name) { > >> + crtc_state->crc_enabled = false; > >> + } else if (strcmp(src_name, "auto") == 0) { > >> + crtc_state->crc_enabled = true; > >> + } else { > >> + crtc_state->crc_enabled = false; > >> + return -EINVAL; > >> + } > >> + > >> + *values_cnt = 1; > >> + > >> + return 0; > >> +} > >> > >> static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) > >> { > >> @@ -23,6 +93,8 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) > >> if (!ret) > >> DRM_ERROR("vkms failure on handling vblank"); > >> > >> + vkms_crc_work_handle(&output->crtc_state.crc_workq); > >> + > >> spin_lock_irqsave(&crtc->dev->event_lock, flags); > >> if (output->event) { > >> drm_crtc_send_vblank_event(crtc, output->event); > >> @@ -46,6 +118,9 @@ static int vkms_enable_vblank(struct drm_crtc *crtc) > >> out->period_ns = ktime_set(0, DEFAULT_VBLANK_NS); > >> hrtimer_start(&out->vblank_hrtimer, out->period_ns, HRTIMER_MODE_ABS); > >> > >> + out->crtc_state.crtc = crtc; > >> + INIT_WORK(&out->crtc_state.crc_workq, vkms_crc_work_handle); > >> + > >> return 0; > >> } > >> > >> @@ -65,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = { > >> .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > >> .enable_vblank = vkms_enable_vblank, > >> .disable_vblank = vkms_disable_vblank, > >> + .set_crc_source = vkms_set_crc_source, > >> }; > >> > >> static int vkms_crtc_atomic_check(struct drm_crtc *crtc, > >> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > >> index 300e6a05d473..4a1458731dd7 100644 > >> --- a/drivers/gpu/drm/vkms/vkms_drv.h > >> +++ b/drivers/gpu/drm/vkms/vkms_drv.h > >> @@ -22,6 +22,18 @@ static const u32 vkms_formats[] = { > >> DRM_FORMAT_XRGB8888, > >> }; > >> > >> +/** > >> + * struct vkms_crtc_state > >> + * @crtc: backpointer to the CRTC > >> + * @crc_workq: worker that captures CRCs for each frame > >> + * @crc_enabled: flag to test if crc is enabled > >> + */ > >> +struct vkms_crtc_state { > >> + struct drm_crtc *crtc; > >> + struct work_struct crc_workq; > >> + bool crc_enabled; > >> +}; > >> + > >> struct vkms_output { > >> struct drm_crtc crtc; > >> struct drm_encoder encoder; > >> @@ -29,6 +41,7 @@ struct vkms_output { > >> struct hrtimer vblank_hrtimer; > >> ktime_t period_ns; > >> struct drm_pending_vblank_event *event; > >> + struct vkms_crtc_state crtc_state; > >> }; > >> > >> struct vkms_device { > >> @@ -55,6 +68,9 @@ int vkms_output_init(struct vkms_device *vkmsdev); > >> > >> struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); > >> > >> +/* CRC Support */ > >> +void vkms_crc_work_handle(struct work_struct *work); > >> + > >> /* Gem stuff */ > >> struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > >> struct drm_file *file, > >> -- > >> 2.17.1 > >> > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 73aae129c37d..d78934b76e33 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -7,8 +7,78 @@ */ #include "vkms_drv.h" +#include <linux/crc32.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_gem_framebuffer_helper.h> + +uint32_t _vkms_get_crc(struct drm_framebuffer *fb) +{ + struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); + struct vkms_gem_object *vkms_obj = container_of(gem_obj, + struct vkms_gem_object, + gem); + u32 crc = 0; + int i = 0; + struct drm_plane_state *state = vkms_obj->plane_state; + unsigned int x = state->src.x1 >> 16; + unsigned int y = state->src.y1 >> 16; + unsigned int height = drm_rect_height(&state->src) >> 16; + unsigned int width = drm_rect_width(&state->src) >> 16; + unsigned int cpp = fb->format->cpp[0]; + unsigned int src_offset; + unsigned int size_byte = width * cpp; + void *vaddr = vkms_obj->vaddr; + + if (WARN_ON(!vaddr)) + return crc; + + for (i = y; i < y + height; i++) { + src_offset = fb->offsets[0] + (i * fb->pitches[0]) + (x * cpp); + crc = crc32_le(crc, vaddr + src_offset, size_byte); + } + + return crc; +} + +void vkms_crc_work_handle(struct work_struct *work) +{ + struct vkms_crtc_state *crtc_state = container_of(work, + struct vkms_crtc_state, + crc_workq); + struct drm_crtc *crtc = crtc_state->crtc; + struct drm_framebuffer *fb = (crtc->primary ? crtc->primary->fb : NULL); + + if (crtc_state->crc_enabled && fb) { + u32 crc32 = _vkms_get_crc(fb); + unsigned int n_frame = drm_crtc_accurate_vblank_count(crtc); + + drm_crtc_add_crc_entry(crtc, true, n_frame, &crc32); + } +} + +static int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name, + size_t *values_cnt) +{ + struct drm_device *dev = crtc->dev; + struct vkms_device *vkms_dev = container_of(dev, + struct vkms_device, + drm); + struct vkms_crtc_state *crtc_state = &vkms_dev->output.crtc_state; + + if (!src_name) { + crtc_state->crc_enabled = false; + } else if (strcmp(src_name, "auto") == 0) { + crtc_state->crc_enabled = true; + } else { + crtc_state->crc_enabled = false; + return -EINVAL; + } + + *values_cnt = 1; + + return 0; +} static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) { @@ -23,6 +93,8 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) if (!ret) DRM_ERROR("vkms failure on handling vblank"); + vkms_crc_work_handle(&output->crtc_state.crc_workq); + spin_lock_irqsave(&crtc->dev->event_lock, flags); if (output->event) { drm_crtc_send_vblank_event(crtc, output->event); @@ -46,6 +118,9 @@ static int vkms_enable_vblank(struct drm_crtc *crtc) out->period_ns = ktime_set(0, DEFAULT_VBLANK_NS); hrtimer_start(&out->vblank_hrtimer, out->period_ns, HRTIMER_MODE_ABS); + out->crtc_state.crtc = crtc; + INIT_WORK(&out->crtc_state.crc_workq, vkms_crc_work_handle); + return 0; } @@ -65,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .enable_vblank = vkms_enable_vblank, .disable_vblank = vkms_disable_vblank, + .set_crc_source = vkms_set_crc_source, }; static int vkms_crtc_atomic_check(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 300e6a05d473..4a1458731dd7 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -22,6 +22,18 @@ static const u32 vkms_formats[] = { DRM_FORMAT_XRGB8888, }; +/** + * struct vkms_crtc_state + * @crtc: backpointer to the CRTC + * @crc_workq: worker that captures CRCs for each frame + * @crc_enabled: flag to test if crc is enabled + */ +struct vkms_crtc_state { + struct drm_crtc *crtc; + struct work_struct crc_workq; + bool crc_enabled; +}; + struct vkms_output { struct drm_crtc crtc; struct drm_encoder encoder; @@ -29,6 +41,7 @@ struct vkms_output { struct hrtimer vblank_hrtimer; ktime_t period_ns; struct drm_pending_vblank_event *event; + struct vkms_crtc_state crtc_state; }; struct vkms_device { @@ -55,6 +68,9 @@ int vkms_output_init(struct vkms_device *vkmsdev); struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); +/* CRC Support */ +void vkms_crc_work_handle(struct work_struct *work); + /* Gem stuff */ struct drm_gem_object *vkms_gem_create(struct drm_device *dev, struct drm_file *file,
Implement the .set_crc_source() callback. Compute CRC using crc32 on the visible part of the framebuffer. Use work_struct to compute and add CRC at the end of a vblank. Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com> --- drivers/gpu/drm/vkms/vkms_crtc.c | 76 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/vkms/vkms_drv.h | 16 +++++++ 2 files changed, 92 insertions(+)