| Message ID | 20200110092127.27847-1-tzimmermann@suse.de |
|---|---|
| Headers | show |
| Series | drm: Clean up VBLANK callbacks in struct drm_driver | expand |
Hi Am 10.01.20 um 12:59 schrieb Jani Nikula: > On Fri, 10 Jan 2020, Thomas Zimmermann <tzimmermann@suse.de> wrote: >> The callback struct drm_driver.get_scanout_position() is deprecated in >> favor of struct drm_crtc_helper_funcs.get_scanout_position(). >> >> i915 doesn't use CRTC helpers. The patch duplicates the caller >> drm_calc_vbltimestamp_from_scanoutpos() for i915, such that the callback >> function is not needed. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 3 +- >> drivers/gpu/drm/i915/i915_irq.c | 117 ++++++++++++++++++++++++++++++-- >> drivers/gpu/drm/i915/i915_irq.h | 9 +-- >> 3 files changed, 119 insertions(+), 10 deletions(-) > > Not really enthusiastic about the diffstat in a "cleanup" series. Well, the cleanup is about the content of drm_driver :) > > I wonder if you could add a generic helper version of > drm_calc_vbltimestamp_from_scanoutpos where you pass the > get_scanout_position function as a parameter. Both > drm_calc_vbltimestamp_from_scanoutpos and the new > i915_calc_vbltimestamp_from_scanoutpos would then be fairly thin > wrappers passing in the relevant get_scanout_position function. Of course. Will be in v2 of the series. Best regards Thomas > > This would reduce the almost identical duplication of the function in > i915. > > BR, > Jani. > >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >> index f7385abdd74b..4a0a7fb85c53 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -2769,8 +2769,7 @@ static struct drm_driver driver = { >> .gem_prime_export = i915_gem_prime_export, >> .gem_prime_import = i915_gem_prime_import, >> >> - .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, >> - .get_scanout_position = i915_get_crtc_scanoutpos, >> + .get_vblank_timestamp = i915_calc_vbltimestamp_from_scanoutpos, >> >> .dumb_create = i915_gem_dumb_create, >> .dumb_map_offset = i915_gem_dumb_mmap_offset, >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> index afc6aad9bf8c..99d0c3b0feae 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -52,6 +52,11 @@ >> #include "i915_trace.h" >> #include "intel_pm.h" >> >> +/* Retry timestamp calculation up to 3 times to satisfy >> + * drm_timestamp_precision before giving up. >> + */ >> +#define I915_TIMESTAMP_MAXRETRIES 3 >> + >> /** >> * DOC: interrupt handling >> * >> @@ -762,10 +767,11 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) >> return (position + crtc->scanline_offset) % vtotal; >> } >> >> -bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int index, >> - bool in_vblank_irq, int *vpos, int *hpos, >> - ktime_t *stime, ktime_t *etime, >> - const struct drm_display_mode *mode) >> +static bool i915_get_crtc_scanoutpos(struct drm_device *dev, >> + unsigned int index, bool in_vblank_irq, >> + int *vpos, int *hpos, >> + ktime_t *stime, ktime_t *etime, >> + const struct drm_display_mode *mode) >> { >> struct drm_i915_private *dev_priv = to_i915(dev); >> struct intel_crtc *crtc = to_intel_crtc(drm_crtc_from_index(dev, index)); >> @@ -879,6 +885,109 @@ bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int index, >> return true; >> } >> >> +bool i915_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, >> + unsigned int pipe, >> + int *max_error, >> + ktime_t *vblank_time, >> + bool in_vblank_irq) >> +{ >> + struct timespec64 ts_etime, ts_vblank_time; >> + ktime_t stime, etime; >> + bool vbl_status; >> + struct drm_crtc *crtc; >> + const struct drm_display_mode *mode; >> + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; >> + int vpos, hpos, i; >> + int delta_ns, duration_ns; >> + >> + crtc = drm_crtc_from_index(dev, pipe); >> + >> + if (pipe >= dev->num_crtcs || !crtc) { >> + DRM_ERROR("Invalid crtc %u\n", pipe); >> + return false; >> + } >> + >> + if (drm_drv_uses_atomic_modeset(dev)) >> + mode = &vblank->hwmode; >> + else >> + mode = &crtc->hwmode; >> + >> + /* If mode timing undefined, just return as no-op: >> + * Happens during initial modesetting of a crtc. >> + */ >> + if (mode->crtc_clock == 0) { >> + DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe); >> + WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev)); >> + >> + return false; >> + } >> + >> + /* Get current scanout position with system timestamp. >> + * Repeat query up to DRM_TIMESTAMP_MAXRETRIES times >> + * if single query takes longer than max_error nanoseconds. >> + * >> + * This guarantees a tight bound on maximum error if >> + * code gets preempted or delayed for some reason. >> + */ >> + for (i = 0; i < I915_TIMESTAMP_MAXRETRIES; i++) { >> + /* >> + * Get vertical and horizontal scanout position vpos, hpos, >> + * and bounding timestamps stime, etime, pre/post query. >> + */ >> + vbl_status = i915_get_crtc_scanoutpos(dev, pipe, in_vblank_irq, >> + &vpos, &hpos, &stime, >> + &etime, mode); >> + /* Return as no-op if scanout query unsupported or failed. */ >> + if (!vbl_status) { >> + DRM_DEBUG("crtc %u : scanoutpos query failed.\n", >> + pipe); >> + return false; >> + } >> + >> + /* Compute uncertainty in timestamp of scanout position query. */ >> + duration_ns = ktime_to_ns(etime) - ktime_to_ns(stime); >> + >> + /* Accept result with < max_error nsecs timing uncertainty. */ >> + if (duration_ns <= *max_error) >> + break; >> + } >> + >> + /* Noisy system timing? */ >> + if (i == I915_TIMESTAMP_MAXRETRIES) { >> + DRM_DEBUG("crtc %u: Noisy timestamp %d us > %d us [%d reps].\n", >> + pipe, duration_ns/1000, *max_error/1000, i); >> + } >> + >> + /* Return upper bound of timestamp precision error. */ >> + *max_error = duration_ns; >> + >> + /* Convert scanout position into elapsed time at raw_time query >> + * since start of scanout at first display scanline. delta_ns >> + * can be negative if start of scanout hasn't happened yet. >> + */ >> + delta_ns = div_s64(1000000LL * (vpos * mode->crtc_htotal + hpos), >> + mode->crtc_clock); >> + >> + /* Subtract time delta from raw timestamp to get final >> + * vblank_time timestamp for end of vblank. >> + */ >> + *vblank_time = ktime_sub_ns(etime, delta_ns); >> + >> + if (!drm_debug_enabled(DRM_UT_VBL)) >> + return true; >> + >> + ts_etime = ktime_to_timespec64(etime); >> + ts_vblank_time = ktime_to_timespec64(*vblank_time); >> + >> + DRM_DEBUG_VBL("crtc %u : v p(%d,%d)@ %lld.%06ld -> %lld.%06ld [e %d us, %d rep]\n", >> + pipe, hpos, vpos, >> + (u64)ts_etime.tv_sec, ts_etime.tv_nsec / 1000, >> + (u64)ts_vblank_time.tv_sec, ts_vblank_time.tv_nsec / 1000, >> + duration_ns / 1000, i); >> + >> + return true; >> +} >> + >> int intel_get_crtc_scanline(struct intel_crtc *crtc) >> { >> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h >> index 812c47a9c2d6..5f7b133ce721 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.h >> +++ b/drivers/gpu/drm/i915/i915_irq.h >> @@ -101,10 +101,11 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, >> void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, >> u8 pipe_mask); >> >> -bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, >> - bool in_vblank_irq, int *vpos, int *hpos, >> - ktime_t *stime, ktime_t *etime, >> - const struct drm_display_mode *mode); >> +bool i915_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, >> + unsigned int pipe, >> + int *max_error, >> + ktime_t *vblank_time, >> + bool in_vblank_irq); >> >> u32 i915_get_vblank_counter(struct drm_crtc *crtc); >> u32 g4x_get_vblank_counter(struct drm_crtc *crtc); >
On Fri, 10 Jan 2020, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Hi > > Am 10.01.20 um 12:59 schrieb Jani Nikula: >> On Fri, 10 Jan 2020, Thomas Zimmermann <tzimmermann@suse.de> wrote: >>> The callback struct drm_driver.get_scanout_position() is deprecated in >>> favor of struct drm_crtc_helper_funcs.get_scanout_position(). >>> >>> i915 doesn't use CRTC helpers. The patch duplicates the caller >>> drm_calc_vbltimestamp_from_scanoutpos() for i915, such that the callback >>> function is not needed. >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> --- >>> drivers/gpu/drm/i915/i915_drv.c | 3 +- >>> drivers/gpu/drm/i915/i915_irq.c | 117 ++++++++++++++++++++++++++++++-- >>> drivers/gpu/drm/i915/i915_irq.h | 9 +-- >>> 3 files changed, 119 insertions(+), 10 deletions(-) >> >> Not really enthusiastic about the diffstat in a "cleanup" series. > > Well, the cleanup is about the content of drm_driver :) > >> >> I wonder if you could add a generic helper version of >> drm_calc_vbltimestamp_from_scanoutpos where you pass the >> get_scanout_position function as a parameter. Both >> drm_calc_vbltimestamp_from_scanoutpos and the new >> i915_calc_vbltimestamp_from_scanoutpos would then be fairly thin >> wrappers passing in the relevant get_scanout_position function. > > Of course. Will be in v2 of the series. Please give Ville (Cc'd) a moment before sending v2 in case he wants to chime in on this. Thanks, Jani. > > Best regards > Thomas > >> >> This would reduce the almost identical duplication of the function in >> i915. >> >> BR, >> Jani. >> >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >>> index f7385abdd74b..4a0a7fb85c53 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.c >>> +++ b/drivers/gpu/drm/i915/i915_drv.c >>> @@ -2769,8 +2769,7 @@ static struct drm_driver driver = { >>> .gem_prime_export = i915_gem_prime_export, >>> .gem_prime_import = i915_gem_prime_import, >>> >>> - .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, >>> - .get_scanout_position = i915_get_crtc_scanoutpos, >>> + .get_vblank_timestamp = i915_calc_vbltimestamp_from_scanoutpos, >>> >>> .dumb_create = i915_gem_dumb_create, >>> .dumb_map_offset = i915_gem_dumb_mmap_offset, >>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >>> index afc6aad9bf8c..99d0c3b0feae 100644 >>> --- a/drivers/gpu/drm/i915/i915_irq.c >>> +++ b/drivers/gpu/drm/i915/i915_irq.c >>> @@ -52,6 +52,11 @@ >>> #include "i915_trace.h" >>> #include "intel_pm.h" >>> >>> +/* Retry timestamp calculation up to 3 times to satisfy >>> + * drm_timestamp_precision before giving up. >>> + */ >>> +#define I915_TIMESTAMP_MAXRETRIES 3 >>> + >>> /** >>> * DOC: interrupt handling >>> * >>> @@ -762,10 +767,11 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) >>> return (position + crtc->scanline_offset) % vtotal; >>> } >>> >>> -bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int index, >>> - bool in_vblank_irq, int *vpos, int *hpos, >>> - ktime_t *stime, ktime_t *etime, >>> - const struct drm_display_mode *mode) >>> +static bool i915_get_crtc_scanoutpos(struct drm_device *dev, >>> + unsigned int index, bool in_vblank_irq, >>> + int *vpos, int *hpos, >>> + ktime_t *stime, ktime_t *etime, >>> + const struct drm_display_mode *mode) >>> { >>> struct drm_i915_private *dev_priv = to_i915(dev); >>> struct intel_crtc *crtc = to_intel_crtc(drm_crtc_from_index(dev, index)); >>> @@ -879,6 +885,109 @@ bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int index, >>> return true; >>> } >>> >>> +bool i915_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, >>> + unsigned int pipe, >>> + int *max_error, >>> + ktime_t *vblank_time, >>> + bool in_vblank_irq) >>> +{ >>> + struct timespec64 ts_etime, ts_vblank_time; >>> + ktime_t stime, etime; >>> + bool vbl_status; >>> + struct drm_crtc *crtc; >>> + const struct drm_display_mode *mode; >>> + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; >>> + int vpos, hpos, i; >>> + int delta_ns, duration_ns; >>> + >>> + crtc = drm_crtc_from_index(dev, pipe); >>> + >>> + if (pipe >= dev->num_crtcs || !crtc) { >>> + DRM_ERROR("Invalid crtc %u\n", pipe); >>> + return false; >>> + } >>> + >>> + if (drm_drv_uses_atomic_modeset(dev)) >>> + mode = &vblank->hwmode; >>> + else >>> + mode = &crtc->hwmode; >>> + >>> + /* If mode timing undefined, just return as no-op: >>> + * Happens during initial modesetting of a crtc. >>> + */ >>> + if (mode->crtc_clock == 0) { >>> + DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe); >>> + WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev)); >>> + >>> + return false; >>> + } >>> + >>> + /* Get current scanout position with system timestamp. >>> + * Repeat query up to DRM_TIMESTAMP_MAXRETRIES times >>> + * if single query takes longer than max_error nanoseconds. >>> + * >>> + * This guarantees a tight bound on maximum error if >>> + * code gets preempted or delayed for some reason. >>> + */ >>> + for (i = 0; i < I915_TIMESTAMP_MAXRETRIES; i++) { >>> + /* >>> + * Get vertical and horizontal scanout position vpos, hpos, >>> + * and bounding timestamps stime, etime, pre/post query. >>> + */ >>> + vbl_status = i915_get_crtc_scanoutpos(dev, pipe, in_vblank_irq, >>> + &vpos, &hpos, &stime, >>> + &etime, mode); >>> + /* Return as no-op if scanout query unsupported or failed. */ >>> + if (!vbl_status) { >>> + DRM_DEBUG("crtc %u : scanoutpos query failed.\n", >>> + pipe); >>> + return false; >>> + } >>> + >>> + /* Compute uncertainty in timestamp of scanout position query. */ >>> + duration_ns = ktime_to_ns(etime) - ktime_to_ns(stime); >>> + >>> + /* Accept result with < max_error nsecs timing uncertainty. */ >>> + if (duration_ns <= *max_error) >>> + break; >>> + } >>> + >>> + /* Noisy system timing? */ >>> + if (i == I915_TIMESTAMP_MAXRETRIES) { >>> + DRM_DEBUG("crtc %u: Noisy timestamp %d us > %d us [%d reps].\n", >>> + pipe, duration_ns/1000, *max_error/1000, i); >>> + } >>> + >>> + /* Return upper bound of timestamp precision error. */ >>> + *max_error = duration_ns; >>> + >>> + /* Convert scanout position into elapsed time at raw_time query >>> + * since start of scanout at first display scanline. delta_ns >>> + * can be negative if start of scanout hasn't happened yet. >>> + */ >>> + delta_ns = div_s64(1000000LL * (vpos * mode->crtc_htotal + hpos), >>> + mode->crtc_clock); >>> + >>> + /* Subtract time delta from raw timestamp to get final >>> + * vblank_time timestamp for end of vblank. >>> + */ >>> + *vblank_time = ktime_sub_ns(etime, delta_ns); >>> + >>> + if (!drm_debug_enabled(DRM_UT_VBL)) >>> + return true; >>> + >>> + ts_etime = ktime_to_timespec64(etime); >>> + ts_vblank_time = ktime_to_timespec64(*vblank_time); >>> + >>> + DRM_DEBUG_VBL("crtc %u : v p(%d,%d)@ %lld.%06ld -> %lld.%06ld [e %d us, %d rep]\n", >>> + pipe, hpos, vpos, >>> + (u64)ts_etime.tv_sec, ts_etime.tv_nsec / 1000, >>> + (u64)ts_vblank_time.tv_sec, ts_vblank_time.tv_nsec / 1000, >>> + duration_ns / 1000, i); >>> + >>> + return true; >>> +} >>> + >>> int intel_get_crtc_scanline(struct intel_crtc *crtc) >>> { >>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >>> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h >>> index 812c47a9c2d6..5f7b133ce721 100644 >>> --- a/drivers/gpu/drm/i915/i915_irq.h >>> +++ b/drivers/gpu/drm/i915/i915_irq.h >>> @@ -101,10 +101,11 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, >>> void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, >>> u8 pipe_mask); >>> >>> -bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, >>> - bool in_vblank_irq, int *vpos, int *hpos, >>> - ktime_t *stime, ktime_t *etime, >>> - const struct drm_display_mode *mode); >>> +bool i915_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, >>> + unsigned int pipe, >>> + int *max_error, >>> + ktime_t *vblank_time, >>> + bool in_vblank_irq); >>> >>> u32 i915_get_vblank_counter(struct drm_crtc *crtc); >>> u32 g4x_get_vblank_counter(struct drm_crtc *crtc); >>
On Fri, Jan 10, 2020 at 4:21 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > The callback struct drm_driver.get_scanout_position() is deprecated in > favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert > amdgpu over. > I would prefer to just change the signature of amdgpu_display_get_crtc_scanoutpos() to match the new API rather than wrapping it again. Alex > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 12 ++++++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 ----------- > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 5 +++++ > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 1 + > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 1 + > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 + > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 1 + > drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 1 + > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++- > 9 files changed, 24 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > index 4e699071d144..a1e769d4417d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > @@ -914,3 +914,15 @@ int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int crtc) > return AMDGPU_CRTC_IRQ_NONE; > } > } > + > +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc, > + bool in_vblank_irq, int *vpos, > + int *hpos, ktime_t *stime, ktime_t *etime, > + const struct drm_display_mode *mode) > +{ > + struct drm_device *dev = crtc->dev; > + unsigned int pipe = crtc->index; > + > + return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, > + stime, etime, mode); > +} > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index 3f6f14ce1511..0749285dd1c7 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -1367,16 +1367,6 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv) > return 0; > } > > -static bool > -amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe, > - bool in_vblank_irq, int *vpos, int *hpos, > - ktime_t *stime, ktime_t *etime, > - const struct drm_display_mode *mode) > -{ > - return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, > - stime, etime, mode); > -} > - > static struct drm_driver kms_driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_ATOMIC | > @@ -1391,7 +1381,6 @@ static struct drm_driver kms_driver = { > .enable_vblank = amdgpu_enable_vblank_kms, > .disable_vblank = amdgpu_disable_vblank_kms, > .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, > - .get_scanout_position = amdgpu_get_crtc_scanout_position, > .irq_handler = amdgpu_irq_handler, > .ioctls = amdgpu_ioctls_kms, > .gem_free_object_unlocked = amdgpu_gem_object_free, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > index eb9975f4decb..37ba07e2feb5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > @@ -612,6 +612,11 @@ void amdgpu_panel_mode_fixup(struct drm_encoder *encoder, > struct drm_display_mode *adjusted_mode); > int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int crtc); > > +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc, > + bool in_vblank_irq, int *vpos, > + int *hpos, ktime_t *stime, ktime_t *etime, > + const struct drm_display_mode *mode); > + > /* fbdev layer */ > int amdgpu_fbdev_init(struct amdgpu_device *adev); > void amdgpu_fbdev_fini(struct amdgpu_device *adev); > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index 40d2ac723dd6..bdc1e0f036d4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -2685,6 +2685,7 @@ static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = { > .prepare = dce_v10_0_crtc_prepare, > .commit = dce_v10_0_crtc_commit, > .disable = dce_v10_0_crtc_disable, > + .get_scanout_position = amdgpu_crtc_get_scanout_position, > }; > > static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index 898ef72d423c..0319da5f7bf9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -2793,6 +2793,7 @@ static const struct drm_crtc_helper_funcs dce_v11_0_crtc_helper_funcs = { > .prepare = dce_v11_0_crtc_prepare, > .commit = dce_v11_0_crtc_commit, > .disable = dce_v11_0_crtc_disable, > + .get_scanout_position = amdgpu_crtc_get_scanout_position, > }; > > static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > index db15a112becc..78642c3b14fc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > @@ -2575,6 +2575,7 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = { > .prepare = dce_v6_0_crtc_prepare, > .commit = dce_v6_0_crtc_commit, > .disable = dce_v6_0_crtc_disable, > + .get_scanout_position = amdgpu_crtc_get_scanout_position, > }; > > static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index f06c9022c1fd..1e8d4975435a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -2593,6 +2593,7 @@ static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = { > .prepare = dce_v8_0_crtc_prepare, > .commit = dce_v8_0_crtc_commit, > .disable = dce_v8_0_crtc_disable, > + .get_scanout_position = amdgpu_crtc_get_scanout_position, > }; > > static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > index e4f94863332c..4b2f915aba47 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > @@ -218,6 +218,7 @@ static const struct drm_crtc_helper_funcs dce_virtual_crtc_helper_funcs = { > .prepare = dce_virtual_crtc_prepare, > .commit = dce_virtual_crtc_commit, > .disable = dce_virtual_crtc_disable, > + .get_scanout_position = amdgpu_crtc_get_scanout_position, > }; > > static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index) > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index f2db400a3920..39c5cf242c1b 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -4821,7 +4821,8 @@ static bool dm_crtc_helper_mode_fixup(struct drm_crtc *crtc, > static const struct drm_crtc_helper_funcs amdgpu_dm_crtc_helper_funcs = { > .disable = dm_crtc_helper_disable, > .atomic_check = dm_crtc_helper_atomic_check, > - .mode_fixup = dm_crtc_helper_mode_fixup > + .mode_fixup = dm_crtc_helper_mode_fixup, > + .get_scanout_position = amdgpu_crtc_get_scanout_position, > }; > > static void dm_encoder_helper_disable(struct drm_encoder *encoder) > -- > 2.24.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Fri, Jan 10, 2020 at 4:22 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > VBLANK callbacks in struct drm_driver are deprecated in favor of > their equivalents in struct drm_crtc_funcs. Convert amdgpu over. I think I'd prefer to just update the signatures of the relevant functions rather than wrapping them. Alex > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +++ > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ---- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 24 +++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 4 ++++ > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 4 ++++ > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 4 ++++ > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 4 ++++ > drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 ++++ > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++ > 9 files changed, 49 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index 81a531b652aa..c1262ab588c9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1197,6 +1197,9 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon); > u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe); > int amdgpu_enable_vblank_kms(struct drm_device *dev, unsigned int pipe); > void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); > +u32 amdgpu_crtc_get_vblank_counter(struct drm_crtc *crtc); > +int amdgpu_crtc_enable_vblank(struct drm_crtc *crtc); > +void amdgpu_crtc_disable_vblank(struct drm_crtc *crtc); > long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd, > unsigned long arg); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index 0749285dd1c7..9baa1ddf8693 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -1377,10 +1377,6 @@ static struct drm_driver kms_driver = { > .postclose = amdgpu_driver_postclose_kms, > .lastclose = amdgpu_driver_lastclose_kms, > .unload = amdgpu_driver_unload_kms, > - .get_vblank_counter = amdgpu_get_vblank_counter_kms, > - .enable_vblank = amdgpu_enable_vblank_kms, > - .disable_vblank = amdgpu_disable_vblank_kms, > - .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, > .irq_handler = amdgpu_irq_handler, > .ioctls = amdgpu_ioctls_kms, > .gem_free_object_unlocked = amdgpu_gem_object_free, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index 60591dbc2097..efe4671fb032 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -1174,6 +1174,14 @@ u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe) > return count; > } > > +u32 amdgpu_crtc_get_vblank_counter(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + unsigned int pipe = crtc->index; > + > + return amdgpu_get_vblank_counter_kms(dev, pipe); > +} > + > /** > * amdgpu_enable_vblank_kms - enable vblank interrupt > * > @@ -1191,6 +1199,14 @@ int amdgpu_enable_vblank_kms(struct drm_device *dev, unsigned int pipe) > return amdgpu_irq_get(adev, &adev->crtc_irq, idx); > } > > +int amdgpu_crtc_enable_vblank(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + unsigned int pipe = crtc->index; > + > + return amdgpu_enable_vblank_kms(dev, pipe); > +} > + > /** > * amdgpu_disable_vblank_kms - disable vblank interrupt > * > @@ -1207,6 +1223,14 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe) > amdgpu_irq_put(adev, &adev->crtc_irq, idx); > } > > +void amdgpu_crtc_disable_vblank(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + unsigned int pipe = crtc->index; > + > + amdgpu_disable_vblank_kms(dev, pipe); > +} > + > const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index bdc1e0f036d4..8e62f46f0bfd 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -2494,6 +2494,10 @@ static const struct drm_crtc_funcs dce_v10_0_crtc_funcs = { > .set_config = amdgpu_display_crtc_set_config, > .destroy = dce_v10_0_crtc_destroy, > .page_flip_target = amdgpu_display_crtc_page_flip_target, > + .get_vblank_counter = amdgpu_crtc_get_vblank_counter, > + .enable_vblank = amdgpu_crtc_enable_vblank, > + .disable_vblank = amdgpu_crtc_disable_vblank, > + .get_vblank_timestamp = drm_crtc_calc_vbltimestamp_from_scanoutpos, > }; > > static void dce_v10_0_crtc_dpms(struct drm_crtc *crtc, int mode) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index 0319da5f7bf9..9e37e4a78403 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -2573,6 +2573,10 @@ static const struct drm_crtc_funcs dce_v11_0_crtc_funcs = { > .set_config = amdgpu_display_crtc_set_config, > .destroy = dce_v11_0_crtc_destroy, > .page_flip_target = amdgpu_display_crtc_page_flip_target, > + .get_vblank_counter = amdgpu_crtc_get_vblank_counter, > + .enable_vblank = amdgpu_crtc_enable_vblank, > + .disable_vblank = amdgpu_crtc_disable_vblank, > + .get_vblank_timestamp = drm_crtc_calc_vbltimestamp_from_scanoutpos, > }; > > static void dce_v11_0_crtc_dpms(struct drm_crtc *crtc, int mode) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > index 78642c3b14fc..2d4c99d3b352 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > @@ -2388,6 +2388,10 @@ static const struct drm_crtc_funcs dce_v6_0_crtc_funcs = { > .set_config = amdgpu_display_crtc_set_config, > .destroy = dce_v6_0_crtc_destroy, > .page_flip_target = amdgpu_display_crtc_page_flip_target, > + .get_vblank_counter = amdgpu_crtc_get_vblank_counter, > + .enable_vblank = amdgpu_crtc_enable_vblank, > + .disable_vblank = amdgpu_crtc_disable_vblank, > + .get_vblank_timestamp = drm_crtc_calc_vbltimestamp_from_scanoutpos, > }; > > static void dce_v6_0_crtc_dpms(struct drm_crtc *crtc, int mode) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index 1e8d4975435a..cd51fd155349 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -2395,6 +2395,10 @@ static const struct drm_crtc_funcs dce_v8_0_crtc_funcs = { > .set_config = amdgpu_display_crtc_set_config, > .destroy = dce_v8_0_crtc_destroy, > .page_flip_target = amdgpu_display_crtc_page_flip_target, > + .get_vblank_counter = amdgpu_crtc_get_vblank_counter, > + .enable_vblank = amdgpu_crtc_enable_vblank, > + .disable_vblank = amdgpu_crtc_disable_vblank, > + .get_vblank_timestamp = drm_crtc_calc_vbltimestamp_from_scanoutpos, > }; > > static void dce_v8_0_crtc_dpms(struct drm_crtc *crtc, int mode) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > index 4b2f915aba47..53c260deb384 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > @@ -123,6 +123,10 @@ static const struct drm_crtc_funcs dce_virtual_crtc_funcs = { > .set_config = amdgpu_display_crtc_set_config, > .destroy = dce_virtual_crtc_destroy, > .page_flip_target = amdgpu_display_crtc_page_flip_target, > + .get_vblank_counter = amdgpu_crtc_get_vblank_counter, > + .enable_vblank = amdgpu_crtc_enable_vblank, > + .disable_vblank = amdgpu_crtc_disable_vblank, > + .get_vblank_timestamp = drm_crtc_calc_vbltimestamp_from_scanoutpos, > }; > > static void dce_virtual_crtc_dpms(struct drm_crtc *crtc, int mode) > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 39c5cf242c1b..0c51c4dd8c46 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -4214,8 +4214,10 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { > .set_crc_source = amdgpu_dm_crtc_set_crc_source, > .verify_crc_source = amdgpu_dm_crtc_verify_crc_source, > .get_crc_sources = amdgpu_dm_crtc_get_crc_sources, > + .get_vblank_counter = amdgpu_crtc_get_vblank_counter, > .enable_vblank = dm_enable_vblank, > .disable_vblank = dm_disable_vblank, > + .get_vblank_timestamp = drm_crtc_calc_vbltimestamp_from_scanoutpos, > }; > > static enum drm_connector_status > -- > 2.24.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Hi Am 13.01.20 um 19:52 schrieb Alex Deucher: > On Fri, Jan 10, 2020 at 4:21 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> >> The callback struct drm_driver.get_scanout_position() is deprecated in >> favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert >> amdgpu over. >> > > I would prefer to just change the signature of > amdgpu_display_get_crtc_scanoutpos() to match the new API rather than > wrapping it again. While trying to adapt the siganture, I found that amdgpu_display_get_crtc_scanoutpos() requires a flags argument that is not mappable to the callback API. That wrapper function is necessary. Best regards Thomas > > Alex > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 12 ++++++++++++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 ----------- >> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 5 +++++ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 1 + >> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 1 + >> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 + >> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 1 + >> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 1 + >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++- >> 9 files changed, 24 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >> index 4e699071d144..a1e769d4417d 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >> @@ -914,3 +914,15 @@ int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int crtc) >> return AMDGPU_CRTC_IRQ_NONE; >> } >> } >> + >> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc, >> + bool in_vblank_irq, int *vpos, >> + int *hpos, ktime_t *stime, ktime_t *etime, >> + const struct drm_display_mode *mode) >> +{ >> + struct drm_device *dev = crtc->dev; >> + unsigned int pipe = crtc->index; >> + >> + return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, >> + stime, etime, mode); >> +} >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> index 3f6f14ce1511..0749285dd1c7 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> @@ -1367,16 +1367,6 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv) >> return 0; >> } >> >> -static bool >> -amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe, >> - bool in_vblank_irq, int *vpos, int *hpos, >> - ktime_t *stime, ktime_t *etime, >> - const struct drm_display_mode *mode) >> -{ >> - return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, >> - stime, etime, mode); >> -} >> - >> static struct drm_driver kms_driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_ATOMIC | >> @@ -1391,7 +1381,6 @@ static struct drm_driver kms_driver = { >> .enable_vblank = amdgpu_enable_vblank_kms, >> .disable_vblank = amdgpu_disable_vblank_kms, >> .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, >> - .get_scanout_position = amdgpu_get_crtc_scanout_position, >> .irq_handler = amdgpu_irq_handler, >> .ioctls = amdgpu_ioctls_kms, >> .gem_free_object_unlocked = amdgpu_gem_object_free, >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h >> index eb9975f4decb..37ba07e2feb5 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h >> @@ -612,6 +612,11 @@ void amdgpu_panel_mode_fixup(struct drm_encoder *encoder, >> struct drm_display_mode *adjusted_mode); >> int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int crtc); >> >> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc, >> + bool in_vblank_irq, int *vpos, >> + int *hpos, ktime_t *stime, ktime_t *etime, >> + const struct drm_display_mode *mode); >> + >> /* fbdev layer */ >> int amdgpu_fbdev_init(struct amdgpu_device *adev); >> void amdgpu_fbdev_fini(struct amdgpu_device *adev); >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c >> index 40d2ac723dd6..bdc1e0f036d4 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c >> @@ -2685,6 +2685,7 @@ static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = { >> .prepare = dce_v10_0_crtc_prepare, >> .commit = dce_v10_0_crtc_commit, >> .disable = dce_v10_0_crtc_disable, >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, >> }; >> >> static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index) >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c >> index 898ef72d423c..0319da5f7bf9 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c >> @@ -2793,6 +2793,7 @@ static const struct drm_crtc_helper_funcs dce_v11_0_crtc_helper_funcs = { >> .prepare = dce_v11_0_crtc_prepare, >> .commit = dce_v11_0_crtc_commit, >> .disable = dce_v11_0_crtc_disable, >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, >> }; >> >> static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index) >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c >> index db15a112becc..78642c3b14fc 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c >> @@ -2575,6 +2575,7 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = { >> .prepare = dce_v6_0_crtc_prepare, >> .commit = dce_v6_0_crtc_commit, >> .disable = dce_v6_0_crtc_disable, >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, >> }; >> >> static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index) >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c >> index f06c9022c1fd..1e8d4975435a 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c >> @@ -2593,6 +2593,7 @@ static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = { >> .prepare = dce_v8_0_crtc_prepare, >> .commit = dce_v8_0_crtc_commit, >> .disable = dce_v8_0_crtc_disable, >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, >> }; >> >> static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index) >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c >> index e4f94863332c..4b2f915aba47 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c >> @@ -218,6 +218,7 @@ static const struct drm_crtc_helper_funcs dce_virtual_crtc_helper_funcs = { >> .prepare = dce_virtual_crtc_prepare, >> .commit = dce_virtual_crtc_commit, >> .disable = dce_virtual_crtc_disable, >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, >> }; >> >> static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index) >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> index f2db400a3920..39c5cf242c1b 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -4821,7 +4821,8 @@ static bool dm_crtc_helper_mode_fixup(struct drm_crtc *crtc, >> static const struct drm_crtc_helper_funcs amdgpu_dm_crtc_helper_funcs = { >> .disable = dm_crtc_helper_disable, >> .atomic_check = dm_crtc_helper_atomic_check, >> - .mode_fixup = dm_crtc_helper_mode_fixup >> + .mode_fixup = dm_crtc_helper_mode_fixup, >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, >> }; >> >> static void dm_encoder_helper_disable(struct drm_encoder *encoder) >> -- >> 2.24.1 >> >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Wed, Jan 15, 2020 at 4:41 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi > > Am 13.01.20 um 19:52 schrieb Alex Deucher: > > On Fri, Jan 10, 2020 at 4:21 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > >> > >> The callback struct drm_driver.get_scanout_position() is deprecated in > >> favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert > >> amdgpu over. > >> > > > > I would prefer to just change the signature of > > amdgpu_display_get_crtc_scanoutpos() to match the new API rather than > > wrapping it again. > > While trying to adapt the siganture, I found that > amdgpu_display_get_crtc_scanoutpos() requires a flags argument that is > not mappable to the callback API. That wrapper function is necessary. > No worries. We can clean them up later. Wrapping is fine. Alex > Best regards > Thomas > > > > > Alex > > > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 12 ++++++++++++ > >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 ----------- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 5 +++++ > >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 1 + > >> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 1 + > >> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 + > >> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 1 + > >> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 1 + > >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++- > >> 9 files changed, 24 insertions(+), 12 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > >> index 4e699071d144..a1e769d4417d 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > >> @@ -914,3 +914,15 @@ int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int crtc) > >> return AMDGPU_CRTC_IRQ_NONE; > >> } > >> } > >> + > >> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc, > >> + bool in_vblank_irq, int *vpos, > >> + int *hpos, ktime_t *stime, ktime_t *etime, > >> + const struct drm_display_mode *mode) > >> +{ > >> + struct drm_device *dev = crtc->dev; > >> + unsigned int pipe = crtc->index; > >> + > >> + return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, > >> + stime, etime, mode); > >> +} > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> index 3f6f14ce1511..0749285dd1c7 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> @@ -1367,16 +1367,6 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv) > >> return 0; > >> } > >> > >> -static bool > >> -amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe, > >> - bool in_vblank_irq, int *vpos, int *hpos, > >> - ktime_t *stime, ktime_t *etime, > >> - const struct drm_display_mode *mode) > >> -{ > >> - return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, > >> - stime, etime, mode); > >> -} > >> - > >> static struct drm_driver kms_driver = { > >> .driver_features = > >> DRIVER_USE_AGP | DRIVER_ATOMIC | > >> @@ -1391,7 +1381,6 @@ static struct drm_driver kms_driver = { > >> .enable_vblank = amdgpu_enable_vblank_kms, > >> .disable_vblank = amdgpu_disable_vblank_kms, > >> .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, > >> - .get_scanout_position = amdgpu_get_crtc_scanout_position, > >> .irq_handler = amdgpu_irq_handler, > >> .ioctls = amdgpu_ioctls_kms, > >> .gem_free_object_unlocked = amdgpu_gem_object_free, > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > >> index eb9975f4decb..37ba07e2feb5 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > >> @@ -612,6 +612,11 @@ void amdgpu_panel_mode_fixup(struct drm_encoder *encoder, > >> struct drm_display_mode *adjusted_mode); > >> int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int crtc); > >> > >> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc, > >> + bool in_vblank_irq, int *vpos, > >> + int *hpos, ktime_t *stime, ktime_t *etime, > >> + const struct drm_display_mode *mode); > >> + > >> /* fbdev layer */ > >> int amdgpu_fbdev_init(struct amdgpu_device *adev); > >> void amdgpu_fbdev_fini(struct amdgpu_device *adev); > >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > >> index 40d2ac723dd6..bdc1e0f036d4 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > >> @@ -2685,6 +2685,7 @@ static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = { > >> .prepare = dce_v10_0_crtc_prepare, > >> .commit = dce_v10_0_crtc_commit, > >> .disable = dce_v10_0_crtc_disable, > >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, > >> }; > >> > >> static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index) > >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > >> index 898ef72d423c..0319da5f7bf9 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > >> @@ -2793,6 +2793,7 @@ static const struct drm_crtc_helper_funcs dce_v11_0_crtc_helper_funcs = { > >> .prepare = dce_v11_0_crtc_prepare, > >> .commit = dce_v11_0_crtc_commit, > >> .disable = dce_v11_0_crtc_disable, > >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, > >> }; > >> > >> static int dce_v11_0_crtc_init(struct amdgpu_device *adev, int index) > >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > >> index db15a112becc..78642c3b14fc 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > >> @@ -2575,6 +2575,7 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = { > >> .prepare = dce_v6_0_crtc_prepare, > >> .commit = dce_v6_0_crtc_commit, > >> .disable = dce_v6_0_crtc_disable, > >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, > >> }; > >> > >> static int dce_v6_0_crtc_init(struct amdgpu_device *adev, int index) > >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > >> index f06c9022c1fd..1e8d4975435a 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > >> @@ -2593,6 +2593,7 @@ static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = { > >> .prepare = dce_v8_0_crtc_prepare, > >> .commit = dce_v8_0_crtc_commit, > >> .disable = dce_v8_0_crtc_disable, > >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, > >> }; > >> > >> static int dce_v8_0_crtc_init(struct amdgpu_device *adev, int index) > >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > >> index e4f94863332c..4b2f915aba47 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c > >> @@ -218,6 +218,7 @@ static const struct drm_crtc_helper_funcs dce_virtual_crtc_helper_funcs = { > >> .prepare = dce_virtual_crtc_prepare, > >> .commit = dce_virtual_crtc_commit, > >> .disable = dce_virtual_crtc_disable, > >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, > >> }; > >> > >> static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index) > >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > >> index f2db400a3920..39c5cf242c1b 100644 > >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > >> @@ -4821,7 +4821,8 @@ static bool dm_crtc_helper_mode_fixup(struct drm_crtc *crtc, > >> static const struct drm_crtc_helper_funcs amdgpu_dm_crtc_helper_funcs = { > >> .disable = dm_crtc_helper_disable, > >> .atomic_check = dm_crtc_helper_atomic_check, > >> - .mode_fixup = dm_crtc_helper_mode_fixup > >> + .mode_fixup = dm_crtc_helper_mode_fixup, > >> + .get_scanout_position = amdgpu_crtc_get_scanout_position, > >> }; > >> > >> static void dm_encoder_helper_disable(struct drm_encoder *encoder) > >> -- > >> 2.24.1 > >> > >> _______________________________________________ > >> amd-gfx mailing list > >> amd-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer >
VBLANK handlers in struct drm_driver are deprecated. Only legacy, non-KMS drivers are supposed to used them. DRM drivers with kernel modesetting are supposed to use VBLANK callbacks of the CRTC infrastructure. This patchset converts all DRM drivers to CRTC VBLANK callbacks and cleans up struct drm_driver. The remaining VBLANK callbacks in struct drm_driver are only used by legacy drivers. Patches 1 to 9 move get_scanout_position() to struct drm_crtc_helper_funcs and convert drivers over. The callback is a helper for the default implementation of get_vblank_timestamp() (i.e., drm_calc_vbltimestamp_from_scanoutpos()). The original callback is removed from struct drm_driver. Patch 10 changes the VBLANK code to evaluate vblank_disable_immediate in struct derm_device. This simplifies the later integration of CRTC VBLANK callbacks. If necessary, a future patch could move vblank_disable_immedate to struct drm_crtc, so that high-precision VBLANKs could be enabled on a per-CRTC basis. Patches 11 to 23 move get_vblank_timestamp() to struct drm_crtc_funcs and convert DRM drivers over. All VBLANK callbacks are removed from struct drm_driver, except for get_vblank_counter(), enable_vblank(), and disable_vblank(). These interfaces are moved to the legacy section at the end of the structure. To cover all affected drivers, I build the patchset in x86, x86-64, arm and aarch64. I smoke-tested amdgpu, gma500, i915, radeon and vc4 on respective hardware. Thomas Zimmermann (23): drm: Add get_scanout_position() to struct drm_crtc_helper_funcs drm/amdgpu: Convert to struct drm_crtc_helper_funcs.get_scanout_position() drm/i915: Don't use struct drm_driver.get_scanout_position() drm/nouveau: Convert to struct drm_crtc_helper_funcs.get_scanout_position() drm/radeon: Convert to struct drm_crtc_helper_funcs.get_scanout_position() drm/msm: Convert to struct drm_crtc_helper_funcs.get_scanout_position() drm/vc4: Convert to struct drm_crtc_helper_funcs.get_scanout_position() drm/stm: Convert to struct drm_crtc_helper_funcs.get_scanout_position() drm: Remove struct drm_driver.get_scanout_position() drm: Evaluate struct drm_device.vblank_disable_immediate on each use drm: Add get_vblank_timestamp() to struct drm_crtc_funcs drm/amdgpu: Convert to CRTC VBLANK callbacks drm/gma500: Convert to CRTC VBLANK callbacks drm/i915: Convert to CRTC VBLANK callbacks drm/msm: Convert to CRTC VBLANK callbacks drm/nouveau: Convert to CRTC VBLANK callbacks drm/radeon: Convert to CRTC VBLANK callbacks drm/sti: Convert to CRTC VBLANK callbacks drm/stm: Convert to CRTC VBLANK callbacks drm/vc4: Convert to CRTC VBLANK callbacks drm/vkms: Convert to CRTC VBLANK callbacks drm/vmwgfx: Convert to CRTC VBLANK callbacks drm: Cleanup VBLANK callbacks in struct drm_driver drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 + drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 12 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 15 -- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 24 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 5 + drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 5 + drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 5 + drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 5 + drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 5 + drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 5 + .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +- drivers/gpu/drm/drm_vblank.c | 128 +++++++++----- drivers/gpu/drm/gma500/cdv_intel_display.c | 3 + drivers/gpu/drm/gma500/psb_drv.c | 4 - drivers/gpu/drm/gma500/psb_drv.h | 6 +- drivers/gpu/drm/gma500/psb_intel_display.c | 3 + drivers/gpu/drm/gma500/psb_irq.c | 12 +- drivers/gpu/drm/gma500/psb_irq.h | 7 +- drivers/gpu/drm/i915/display/intel_display.c | 7 + drivers/gpu/drm/i915/i915_drv.c | 3 - drivers/gpu/drm/i915/i915_irq.c | 110 +++++++++++- drivers/gpu/drm/i915/i915_irq.h | 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 + drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 2 + drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 82 +++++++++ drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 95 ----------- drivers/gpu/drm/msm/msm_drv.c | 10 +- drivers/gpu/drm/msm/msm_drv.h | 3 + drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 + drivers/gpu/drm/nouveau/dispnv50/head.c | 5 + drivers/gpu/drm/nouveau/nouveau_display.c | 28 +--- drivers/gpu/drm/nouveau/nouveau_display.h | 6 +- drivers/gpu/drm/nouveau/nouveau_drm.c | 5 - drivers/gpu/drm/radeon/atombios_crtc.c | 1 + drivers/gpu/drm/radeon/radeon_display.c | 25 ++- drivers/gpu/drm/radeon/radeon_drv.c | 18 -- drivers/gpu/drm/radeon/radeon_kms.c | 29 ++-- drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 3 +- drivers/gpu/drm/radeon/radeon_mode.h | 6 + drivers/gpu/drm/sti/sti_crtc.c | 11 +- drivers/gpu/drm/sti/sti_crtc.h | 2 - drivers/gpu/drm/sti/sti_drv.c | 3 - drivers/gpu/drm/stm/drv.c | 2 - drivers/gpu/drm/stm/ltdc.c | 66 ++++---- drivers/gpu/drm/stm/ltdc.h | 5 - drivers/gpu/drm/vc4/vc4_crtc.c | 13 +- drivers/gpu/drm/vc4/vc4_drv.c | 3 - drivers/gpu/drm/vc4/vc4_drv.h | 4 - drivers/gpu/drm/vkms/vkms_crtc.c | 9 +- drivers/gpu/drm/vkms/vkms_drv.c | 1 - drivers/gpu/drm/vkms/vkms_drv.h | 4 - drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 6 +- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 +- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 3 + drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 5 +- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 3 + include/drm/drm_crtc.h | 41 +++++ include/drm/drm_drv.h | 156 +----------------- include/drm/drm_modeset_helper_vtables.h | 47 ++++++ include/drm/drm_vblank.h | 4 + 61 files changed, 631 insertions(+), 474 deletions(-) -- 2.24.1