| Message ID | 1470393905-6640-4-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomeu, IMHO it would be better to split out the refactoring into preparatory patch. It brings a minor change which (not 100% sure on that) should not cause issues but is worth pointing out. On 5 August 2016 at 11:45, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > +static int do_set_crc_source(struct drm_device *dev, enum pipe pipe, > + enum intel_pipe_crc_source source) > +{ > + if (source == INTEL_PIPE_CRC_SOURCE_NONE) { Nit: use !source here or sourse != INTEL_PIPE_CRC_SOURCE_NONE elsewhere in the code ? > @@ -693,10 +718,11 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, > spin_unlock_irq(&pipe_crc->lock); > } > > - pipe_crc->source = source; > + ret = do_set_crc_source(dev, pipe, source); > + if (ret) > + goto out; > We seem to have modified pipe_crc even if the new function fails. Haven't check if it matters, but definatelly not ideal. > @@ -720,15 +746,6 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, > spin_unlock_irq(&pipe_crc->lock); > > kfree(entries); > - > - if (IS_G4X(dev)) > - g4x_undo_pipe_scramble_reset(dev, pipe); > - else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) > - vlv_undo_pipe_scramble_reset(dev, pipe); > - else if (IS_HASWELL(dev) && pipe == PIPE_A) > - hsw_trans_edp_pipe_A_crc_wa(dev, false); > - > - hsw_enable_ips(crtc); The above is the piece I have in mind: With the introduction of do_set_crc_source() the above are executed prior to the intel_wait_for_vblank() call. Afaics this will not cause any functional change, then again I'm not that familiar with the i915 vblank code. > +int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, > + size_t *values_cnt) > +{ > + ret = do_set_crc_source(crtc->dev, crtc->index, source); > + > + intel_display_power_put(dev_priv, power_domain); > + > + *values_cnt = 5; > + Please don't overwrite values_cnt if the function fails. Regards, Emil
On 2 September 2016 at 17:18, Emil Velikov <emil.l.velikov@gmail.com> wrote: > Hi Tomeu, > > IMHO it would be better to split out the refactoring into preparatory > patch. It brings a minor change which (not 100% sure on that) should > not cause issues but is worth pointing out. I think at this point it would make sense to change the series structure only if there was a strong reason, as a few people have already looked at the patches already. > On 5 August 2016 at 11:45, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > >> +static int do_set_crc_source(struct drm_device *dev, enum pipe pipe, >> + enum intel_pipe_crc_source source) >> +{ > >> + if (source == INTEL_PIPE_CRC_SOURCE_NONE) { > Nit: use !source here or sourse != INTEL_PIPE_CRC_SOURCE_NONE > elsewhere in the code ? Agreed. > >> @@ -693,10 +718,11 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, >> spin_unlock_irq(&pipe_crc->lock); >> } >> >> - pipe_crc->source = source; >> + ret = do_set_crc_source(dev, pipe, source); >> + if (ret) >> + goto out; >> > We seem to have modified pipe_crc even if the new function fails. > Haven't check if it matters, but definatelly not ideal. If we had modified pipe_crc that's because we were trying to start CRC capture and we initialized the entry storage. As CRC generation is disabled, those changes have no effects. When CRC capture is attempted again, they will be initialized again. To avoid this we would need to split the HW programming in two functions and I'm not sure it would be worth it. >> @@ -720,15 +746,6 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, >> spin_unlock_irq(&pipe_crc->lock); >> >> kfree(entries); >> - >> - if (IS_G4X(dev)) >> - g4x_undo_pipe_scramble_reset(dev, pipe); >> - else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) >> - vlv_undo_pipe_scramble_reset(dev, pipe); >> - else if (IS_HASWELL(dev) && pipe == PIPE_A) >> - hsw_trans_edp_pipe_A_crc_wa(dev, false); >> - >> - hsw_enable_ips(crtc); > The above is the piece I have in mind: > With the introduction of do_set_crc_source() the above are executed > prior to the intel_wait_for_vblank() call. > > Afaics this will not cause any functional change, then again I'm not > that familiar with the i915 vblank code. Yeah, not sure either of when do those changes take effect. >> +int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, >> + size_t *values_cnt) >> +{ > >> + ret = do_set_crc_source(crtc->dev, crtc->index, source); >> + >> + intel_display_power_put(dev_priv, power_domain); >> + >> + *values_cnt = 5; >> + > Please don't overwrite values_cnt if the function fails. Done. Thanks, Tomeu > Regards, > Emil > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 5 September 2016 at 10:45, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > On 2 September 2016 at 17:18, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> Hi Tomeu, >> >> IMHO it would be better to split out the refactoring into preparatory >> patch. It brings a minor change which (not 100% sure on that) should >> not cause issues but is worth pointing out. > > I think at this point it would make sense to change the series > structure only if there was a strong reason, as a few people have > already looked at the patches already. > >> On 5 August 2016 at 11:45, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: >> >>> +static int do_set_crc_source(struct drm_device *dev, enum pipe pipe, >>> + enum intel_pipe_crc_source source) >>> +{ >> >>> + if (source == INTEL_PIPE_CRC_SOURCE_NONE) { >> Nit: use !source here or sourse != INTEL_PIPE_CRC_SOURCE_NONE >> elsewhere in the code ? > > Agreed. > >> >>> @@ -693,10 +718,11 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, >>> spin_unlock_irq(&pipe_crc->lock); >>> } >>> >>> - pipe_crc->source = source; >>> + ret = do_set_crc_source(dev, pipe, source); >>> + if (ret) >>> + goto out; >>> >> We seem to have modified pipe_crc even if the new function fails. >> Haven't check if it matters, but definatelly not ideal. > > If we had modified pipe_crc that's because we were trying to start CRC > capture and we initialized the entry storage. As CRC generation is > disabled, those changes have no effects. When CRC capture is attempted > again, they will be initialized again. > > To avoid this we would need to split the HW programming in two > functions and I'm not sure it would be worth it. > A simple way out will be to keep the "can fail" hunk at the top separate from the rest. This way even if things get reinitialised correctly currently, they won't break if someone applies the (perfectly reasonable imho) assumption "function does not modify any data when it fails". </preach mode> >>> @@ -720,15 +746,6 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, >>> spin_unlock_irq(&pipe_crc->lock); >>> >>> kfree(entries); >>> - >>> - if (IS_G4X(dev)) >>> - g4x_undo_pipe_scramble_reset(dev, pipe); >>> - else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) >>> - vlv_undo_pipe_scramble_reset(dev, pipe); >>> - else if (IS_HASWELL(dev) && pipe == PIPE_A) >>> - hsw_trans_edp_pipe_A_crc_wa(dev, false); >>> - >>> - hsw_enable_ips(crtc); >> The above is the piece I have in mind: >> With the introduction of do_set_crc_source() the above are executed >> prior to the intel_wait_for_vblank() call. >> >> Afaics this will not cause any functional change, then again I'm not >> that familiar with the i915 vblank code. > > Yeah, not sure either of when do those changes take effect. > With this said, it would be way better to keep it separate (with a big fat warning in the commit summary). Speaking of which - why did you fold the separate bugfix/workaround "skip the first one or two frames" in v5 ? Shouldn't it be separate so that people can pick it for -fixes/-stable ? Thanks Emil
On 5 September 2016 at 14:44, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 5 September 2016 at 10:45, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: >> On 2 September 2016 at 17:18, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> Hi Tomeu, >>> >>> IMHO it would be better to split out the refactoring into preparatory >>> patch. It brings a minor change which (not 100% sure on that) should >>> not cause issues but is worth pointing out. >> >> I think at this point it would make sense to change the series >> structure only if there was a strong reason, as a few people have >> already looked at the patches already. >> >>> On 5 August 2016 at 11:45, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: >>> >>>> +static int do_set_crc_source(struct drm_device *dev, enum pipe pipe, >>>> + enum intel_pipe_crc_source source) >>>> +{ >>> >>>> + if (source == INTEL_PIPE_CRC_SOURCE_NONE) { >>> Nit: use !source here or sourse != INTEL_PIPE_CRC_SOURCE_NONE >>> elsewhere in the code ? >> >> Agreed. >> >>> >>>> @@ -693,10 +718,11 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, >>>> spin_unlock_irq(&pipe_crc->lock); >>>> } >>>> >>>> - pipe_crc->source = source; >>>> + ret = do_set_crc_source(dev, pipe, source); >>>> + if (ret) >>>> + goto out; >>>> >>> We seem to have modified pipe_crc even if the new function fails. >>> Haven't check if it matters, but definatelly not ideal. >> >> If we had modified pipe_crc that's because we were trying to start CRC >> capture and we initialized the entry storage. As CRC generation is >> disabled, those changes have no effects. When CRC capture is attempted >> again, they will be initialized again. >> >> To avoid this we would need to split the HW programming in two >> functions and I'm not sure it would be worth it. >> > A simple way out will be to keep the "can fail" hunk at the top > separate from the rest. This way even if things get reinitialised > correctly currently, they won't break if someone applies the > (perfectly reasonable imho) assumption "function does not modify any > data when it fails". > </preach mode> > >>>> @@ -720,15 +746,6 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, >>>> spin_unlock_irq(&pipe_crc->lock); >>>> >>>> kfree(entries); >>>> - >>>> - if (IS_G4X(dev)) >>>> - g4x_undo_pipe_scramble_reset(dev, pipe); >>>> - else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) >>>> - vlv_undo_pipe_scramble_reset(dev, pipe); >>>> - else if (IS_HASWELL(dev) && pipe == PIPE_A) >>>> - hsw_trans_edp_pipe_A_crc_wa(dev, false); >>>> - >>>> - hsw_enable_ips(crtc); >>> The above is the piece I have in mind: >>> With the introduction of do_set_crc_source() the above are executed >>> prior to the intel_wait_for_vblank() call. >>> >>> Afaics this will not cause any functional change, then again I'm not >>> that familiar with the i915 vblank code. >> >> Yeah, not sure either of when do those changes take effect. >> > With this said, it would be way better to keep it separate (with a big > fat warning in the commit summary). > > Speaking of which - why did you fold the separate bugfix/workaround > "skip the first one or two frames" in v5 ? Shouldn't it be separate so > that people can pick it for -fixes/-stable ? Oh, that's only in the new code paths. For the old i915 ABI, the frames are skipped in userspace, but as it's something highly HW-dependent, implementors of the new API need to do it within their drivers. Regards, Tomeu
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1c2aec392412..e5fb9fa86358 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1492,41 +1492,58 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv, { struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; struct intel_pipe_crc_entry *entry; - int head, tail; + struct drm_crtc *crtc = intel_get_crtc_for_pipe(&dev_priv->drm, pipe); + struct drm_driver *driver = dev_priv->drm.driver; + uint32_t crcs[5]; + int head, tail, ret; + u32 frame; spin_lock(&pipe_crc->lock); + if (pipe_crc->source) { + if (!pipe_crc->entries) { + spin_unlock(&pipe_crc->lock); + DRM_DEBUG_KMS("spurious interrupt\n"); + return; + } - if (!pipe_crc->entries) { - spin_unlock(&pipe_crc->lock); - DRM_DEBUG_KMS("spurious interrupt\n"); - return; - } - - head = pipe_crc->head; - tail = pipe_crc->tail; + head = pipe_crc->head; + tail = pipe_crc->tail; - if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) { - spin_unlock(&pipe_crc->lock); - DRM_ERROR("CRC buffer overflowing\n"); - return; - } + if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) { + spin_unlock(&pipe_crc->lock); + DRM_ERROR("CRC buffer overflowing\n"); + return; + } - entry = &pipe_crc->entries[head]; + entry = &pipe_crc->entries[head]; - entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, - pipe); - entry->crc[0] = crc0; - entry->crc[1] = crc1; - entry->crc[2] = crc2; - entry->crc[3] = crc3; - entry->crc[4] = crc4; + entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe); + entry->crc[0] = crc0; + entry->crc[1] = crc1; + entry->crc[2] = crc2; + entry->crc[3] = crc3; + entry->crc[4] = crc4; - head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1); - pipe_crc->head = head; + head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1); + pipe_crc->head = head; - spin_unlock(&pipe_crc->lock); + spin_unlock(&pipe_crc->lock); - wake_up_interruptible(&pipe_crc->wq); + wake_up_interruptible(&pipe_crc->wq); + } else { + spin_unlock(&pipe_crc->lock); + spin_lock(&crtc->crc.lock); + crcs[0] = crc0; + crcs[1] = crc1; + crcs[2] = crc2; + crcs[3] = crc3; + crcs[4] = crc4; + frame = driver->get_vblank_counter(&dev_priv->drm, pipe); + ret = drm_crtc_add_crc_entry(crtc, true, frame, crcs); + spin_unlock(&crtc->crc.lock); + if (!ret) + wake_up_interruptible(&crtc->crc.wq); + } } #else static inline void diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index baeb75388dbe..ccb251fcdf46 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13966,6 +13966,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { .page_flip = intel_crtc_page_flip, .atomic_duplicate_state = intel_crtc_duplicate_state, .atomic_destroy_state = intel_crtc_destroy_state, + .set_crc_source = intel_crtc_set_crc_source, }; /** diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d1af85c97a80..1790bb48ebcf 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1797,6 +1797,8 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state); /* intel_pipe_crc.c */ int intel_pipe_crc_create(struct drm_minor *minor); void intel_pipe_crc_cleanup(struct drm_minor *minor); +int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, + size_t *values_cnt); extern const struct file_operations i915_display_crc_ctl_fops; #endif /* __INTEL_DRV_H__ */ diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c index e7e1ddfa0f9d..5d13471e789e 100644 --- a/drivers/gpu/drm/i915/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c @@ -624,15 +624,62 @@ static int ivb_pipe_crc_ctl_reg(struct drm_device *dev, return 0; } +static int do_set_crc_source(struct drm_device *dev, enum pipe pipe, + enum intel_pipe_crc_source source) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, + pipe)); + u32 val = 0; /* shut up gcc */ + int ret; + + if (IS_GEN2(dev)) + ret = i8xx_pipe_crc_ctl_reg(&source, &val); + else if (INTEL_INFO(dev)->gen < 5) + ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); + else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) + ret = vlv_pipe_crc_ctl_reg(dev, pipe, &source, &val); + else if (IS_GEN5(dev) || IS_GEN6(dev)) + ret = ilk_pipe_crc_ctl_reg(&source, &val); + else + ret = ivb_pipe_crc_ctl_reg(dev, pipe, &source, &val); + + if (ret) + return ret; + + if (source) { + /* + * When IPS gets enabled, the pipe CRC changes. Since IPS gets + * enabled and disabled dynamically based on package C states, + * user space can't make reliable use of the CRCs, so let's just + * completely disable it. + */ + hsw_disable_ips(crtc); + } + + I915_WRITE(PIPE_CRC_CTL(pipe), val); + POSTING_READ(PIPE_CRC_CTL(pipe)); + + if (source == INTEL_PIPE_CRC_SOURCE_NONE) { + if (IS_G4X(dev)) + g4x_undo_pipe_scramble_reset(dev, pipe); + else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) + vlv_undo_pipe_scramble_reset(dev, pipe); + else if (IS_HASWELL(dev) && pipe == PIPE_A) + hsw_trans_edp_pipe_A_crc_wa(dev, false); + + hsw_enable_ips(crtc); + } + + return 0; +} + static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, enum intel_pipe_crc_source source) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; - struct intel_crtc *crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, - pipe)); enum intel_display_power_domain power_domain; - u32 val = 0; /* shut up gcc */ int ret; if (pipe_crc->source == source) @@ -648,20 +695,6 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, return -EIO; } - if (IS_GEN2(dev)) - ret = i8xx_pipe_crc_ctl_reg(&source, &val); - else if (INTEL_INFO(dev)->gen < 5) - ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); - else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) - ret = vlv_pipe_crc_ctl_reg(dev, pipe, &source, &val); - else if (IS_GEN5(dev) || IS_GEN6(dev)) - ret = ilk_pipe_crc_ctl_reg(&source, &val); - else - ret = ivb_pipe_crc_ctl_reg(dev, pipe, &source, &val); - - if (ret != 0) - goto out; - /* none -> real source transition */ if (source) { struct intel_pipe_crc_entry *entries; @@ -677,14 +710,6 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, goto out; } - /* - * When IPS gets enabled, the pipe CRC changes. Since IPS gets - * enabled and disabled dynamically based on package C states, - * user space can't make reliable use of the CRCs, so let's just - * completely disable it. - */ - hsw_disable_ips(crtc); - spin_lock_irq(&pipe_crc->lock); kfree(pipe_crc->entries); pipe_crc->entries = entries; @@ -693,10 +718,11 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, spin_unlock_irq(&pipe_crc->lock); } - pipe_crc->source = source; + ret = do_set_crc_source(dev, pipe, source); + if (ret) + goto out; - I915_WRITE(PIPE_CRC_CTL(pipe), val); - POSTING_READ(PIPE_CRC_CTL(pipe)); + pipe_crc->source = source; /* real source -> none transition */ if (source == INTEL_PIPE_CRC_SOURCE_NONE) { @@ -720,15 +746,6 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, spin_unlock_irq(&pipe_crc->lock); kfree(entries); - - if (IS_G4X(dev)) - g4x_undo_pipe_scramble_reset(dev, pipe); - else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) - vlv_undo_pipe_scramble_reset(dev, pipe); - else if (IS_HASWELL(dev) && pipe == PIPE_A) - hsw_trans_edp_pipe_A_crc_wa(dev, false); - - hsw_enable_ips(crtc); } ret = 0; @@ -821,6 +838,11 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s) { int i; + if (!buf) { + *s = INTEL_PIPE_CRC_SOURCE_NONE; + return 0; + } + for (i = 0; i < ARRAY_SIZE(pipe_crc_sources); i++) if (!strcmp(buf, pipe_crc_sources[i])) { *s = i; @@ -949,3 +971,31 @@ void intel_pipe_crc_cleanup(struct drm_minor *minor) drm_debugfs_remove_files(info_list, 1, minor); } } + +int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, + size_t *values_cnt) +{ + struct drm_i915_private *dev_priv = crtc->dev->dev_private; + enum intel_display_power_domain power_domain; + enum intel_pipe_crc_source source; + int ret; + + if (display_crc_ctl_parse_source(source_name, &source) < 0) { + DRM_DEBUG_DRIVER("unknown source %s\n", source_name); + return -EINVAL; + } + + power_domain = POWER_DOMAIN_PIPE(crtc->index); + if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) { + DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n"); + return -EIO; + } + + ret = do_set_crc_source(crtc->dev, crtc->index, source); + + intel_display_power_put(dev_priv, power_domain); + + *values_cnt = 5; + + return ret; +}
The core provides now an ABI to userspace for generation of frame CRCs, so implement the ->set_crc_source() callback and reuse as much code as possible with the previous ABI implementation. v2: - Leave the legacy implementation in place as the ABI implementation in the core is incompatible with it. v3: - Use the "cooked" vblank counter so we have a whole 32 bits. - Make sure we don't mess with the state of the legacy CRC capture ABI implementation. v4: - Keep use of get_vblank_counter as in the legacy code, will be changed in a followup commit. Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++------- drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_pipe_crc.c | 124 ++++++++++++++++++++++++---------- 4 files changed, 133 insertions(+), 63 deletions(-)