On 3/18/19 1:19 PM, Mario Kleiner wrote: > In VRR mode, proper vblank/pageflip timestamps can only be computed > after the display scanout position has left front-porch. Therefore > delay calls to drm_crtc_handle_vblank(), and thereby calls to > drm_update_vblank_count() and pageflip event delivery, to after the > end of front-porch when in VRR mode. > > We add a new vupdate irq, which triggers at the end of the vupdate > interval, ie. at the end of vblank, and calls the core vblank handler > function. The new irq handler is not executed in standard non-VRR > mode, so vblank handling for fixed refresh rate mode is identical > to the past implementation. > > Signed-off-by: Mario Kleiner <mario.kleiner.de@xxxxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 129 ++++++++++++++++++++- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 9 ++ > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 22 ++++ > .../amd/display/dc/irq/dce110/irq_service_dce110.c | 7 +- > .../amd/display/dc/irq/dce120/irq_service_dce120.c | 7 +- > .../amd/display/dc/irq/dce80/irq_service_dce80.c | 6 +- > .../amd/display/dc/irq/dcn10/irq_service_dcn10.c | 40 +++++-- > 8 files changed, 205 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index f88761a..64167dd 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -827,6 +827,7 @@ struct amdgpu_device { > /* For pre-DCE11. DCE11 and later are in "struct amdgpu_device->dm" */ > struct work_struct hotplug_work; > struct amdgpu_irq_src crtc_irq; > + struct amdgpu_irq_src vupdate_irq; > struct amdgpu_irq_src pageflip_irq; > struct amdgpu_irq_src hpd_irq; > > 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 85e4f87..555d9e9f 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -315,6 +315,32 @@ static void dm_pflip_high_irq(void *interrupt_params) > drm_crtc_vblank_put(&amdgpu_crtc->base); > } > > +static void dm_vupdate_high_irq(void *interrupt_params) > +{ > + struct common_irq_params *irq_params = interrupt_params; > + struct amdgpu_device *adev = irq_params->adev; > + struct amdgpu_crtc *acrtc; > + struct dm_crtc_state *acrtc_state; > + > + acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VUPDATE); > + > + if (acrtc) { > + acrtc_state = to_dm_crtc_state(acrtc->base.state); > + > + DRM_DEBUG_DRIVER("crtc:%d, vupdate-vrr:%d\n", acrtc->crtc_id, > + amdgpu_dm_vrr_active(acrtc_state)); > + > + /* Core vblank handling is done here after end of front-porch in > + * vrr mode, as vblank timestamping will give valid results > + * while now done after front-porch. This will also deliver > + * page-flip completion events that have been queued to us > + * if a pageflip happened inside front-porch. > + */ > + if (amdgpu_dm_vrr_active(acrtc_state)) > + drm_crtc_handle_vblank(&acrtc->base) I was thinking that 3 and 4 might have to be merged, but VRR pflip timestamping seems to be the same as it was before (off by a line or two) since it's not handled here yet. This seems to fix vblank events and timestamping at least. > + } > +} > + > static void dm_crtc_high_irq(void *interrupt_params) > { > struct common_irq_params *irq_params = interrupt_params; > @@ -325,11 +351,24 @@ static void dm_crtc_high_irq(void *interrupt_params) > acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VBLANK); > > if (acrtc) { > - drm_crtc_handle_vblank(&acrtc->base); > - amdgpu_dm_crtc_handle_crc_irq(&acrtc->base); > - > acrtc_state = to_dm_crtc_state(acrtc->base.state); > > + DRM_DEBUG_DRIVER("crtc:%d, vupdate-vrr:%d\n", acrtc->crtc_id, > + amdgpu_dm_vrr_active(acrtc_state)); > + > + /* Core vblank handling at start of front-porch is only possible > + * in non-vrr mode, as only there vblank timestamping will give > + * valid results while done in front-porch. Otherwise defer it > + * to dm_vupdate_high_irq after end of front-porch. > + */ > + if (!amdgpu_dm_vrr_active(acrtc_state)) > + drm_crtc_handle_vblank(&acrtc->base); > + > + /* Following stuff must happen at start of vblank, for crc > + * computation and below-the-range btr support in vrr mode. > + */ > + amdgpu_dm_crtc_handle_crc_irq(&acrtc->base); > + > if (acrtc_state->stream && > acrtc_state->vrr_params.supported && > acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE) { > @@ -1447,6 +1486,27 @@ static int dce110_register_irq_handlers(struct amdgpu_device *adev) > dm_crtc_high_irq, c_irq_params); > } > > + /* Use VUPDATE interrupt */ > + for (i = VISLANDS30_IV_SRCID_D1_V_UPDATE_INT; i <= VISLANDS30_IV_SRCID_D6_V_UPDATE_INT; i+=2) { > + r = amdgpu_irq_add_id(adev, client_id, i, &adev->vupdate_irq); > + if (r) { > + DRM_ERROR("Failed to add vupdate irq id!\n"); > + return r; > + } > + > + int_params.int_context = INTERRUPT_HIGH_IRQ_CONTEXT; > + int_params.irq_source = > + dc_interrupt_to_irq_source(dc, i, 0); > + > + c_irq_params = &adev->dm.vupdate_params[int_params.irq_source - DC_IRQ_SOURCE_VUPDATE1]; > + > + c_irq_params->adev = adev; > + c_irq_params->irq_src = int_params.irq_source; > + > + amdgpu_dm_irq_register_interrupt(adev, &int_params, > + dm_vupdate_high_irq, c_irq_params); > + } > + > /* Use GRPH_PFLIP interrupt */ > for (i = VISLANDS30_IV_SRCID_D1_GRPH_PFLIP; > i <= VISLANDS30_IV_SRCID_D6_GRPH_PFLIP; i += 2) { > @@ -1532,6 +1592,34 @@ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) > dm_crtc_high_irq, c_irq_params); > } > > + /* Use VUPDATE_NO_LOCK interrupt on DCN, which seems to correspond to > + * the regular VUPDATE interrupt on DCE. We want DC_IRQ_SOURCE_VUPDATEx > + * to trigger at end of each vblank, regardless of state of the lock, > + * matching DCE behaviour. > + */ > + for (i = DCN_1_0__SRCID__OTG0_IHC_V_UPDATE_NO_LOCK_INTERRUPT; > + i <= DCN_1_0__SRCID__OTG0_IHC_V_UPDATE_NO_LOCK_INTERRUPT + adev->mode_info.num_crtc - 1; > + i++) { > + r = amdgpu_irq_add_id(adev, SOC15_IH_CLIENTID_DCE, i, &adev->vupdate_irq); > + > + if (r) { > + DRM_ERROR("Failed to add vupdate irq id!\n"); > + return r; > + } > + > + int_params.int_context = INTERRUPT_HIGH_IRQ_CONTEXT; > + int_params.irq_source = > + dc_interrupt_to_irq_source(dc, i, 0); > + > + c_irq_params = &adev->dm.vupdate_params[int_params.irq_source - DC_IRQ_SOURCE_VUPDATE1]; > + > + c_irq_params->adev = adev; > + c_irq_params->irq_src = int_params.irq_source; > + > + amdgpu_dm_irq_register_interrupt(adev, &int_params, > + dm_vupdate_high_irq, c_irq_params); > + } > + > /* Use GRPH_PFLIP interrupt */ > for (i = DCN_1_0__SRCID__HUBP0_FLIP_INTERRUPT; > i <= DCN_1_0__SRCID__HUBP0_FLIP_INTERRUPT + adev->mode_info.num_crtc - 1; > @@ -3226,12 +3314,42 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) > return &state->base; > } > > +static inline int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable) > +{ > + enum dc_irq_source irq_source; > + struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > + struct amdgpu_device *adev = crtc->dev->dev_private; > + int rc; > + > + irq_source = IRQ_TYPE_VUPDATE + acrtc->otg_inst; > + > + rc = dc_interrupt_set(adev->dm.dc, irq_source, enable) ? 0 : -EBUSY; > + > + DRM_DEBUG_DRIVER("%s: crtc %d - vupdate irq %sabling: r=%d\n", > + __func__, acrtc->crtc_id, enable ? "en" : "dis", rc); > + return rc; > +} > > static inline int dm_set_vblank(struct drm_crtc *crtc, bool enable) > { > enum dc_irq_source irq_source; > struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > struct amdgpu_device *adev = crtc->dev->dev_private; > + struct dm_crtc_state *acrtc_state = to_dm_crtc_state(crtc->state); > + int rc = 0; > + > + if (enable) { > + /* vblank irq on -> Only need vupdate irq in vrr mode */ > + if (amdgpu_dm_vrr_active(acrtc_state)) > + rc = dm_set_vupdate_irq(crtc, true); > + } > + else { should be } else { > + /* vblank irq off -> vupdate irq off */ > + rc = dm_set_vupdate_irq(crtc, false); > + } > + > + if (rc) > + return rc; > > irq_source = IRQ_TYPE_VBLANK + acrtc->otg_inst; > return dc_interrupt_set(adev->dm.dc, irq_source, enable) ? 0 : -EBUSY; > @@ -4741,7 +4859,11 @@ static void amdgpu_dm_handle_vrr_transition(struct dm_crtc_state *old_state, > * While VRR is active, we must not disable vblank irq, as a > * reenable after disable would compute bogus vblank/pflip > * timestamps if it likely happened inside display front-porch. > + * > + * We also need vupdate irq for the actual core vblank handling > + * at end of vblank. > */ > + dm_set_vupdate_irq(new_state->base.crtc, true); > drm_crtc_vblank_get(new_state->base.crtc); > DRM_DEBUG_DRIVER("%s: crtc=%u VRR off->on: Get vblank ref\n", > __func__, new_state->base.crtc->base.id); > @@ -4750,6 +4872,7 @@ static void amdgpu_dm_handle_vrr_transition(struct dm_crtc_state *old_state, > /* Transition VRR active -> inactive: > * Allow vblank irq disable again for fixed refresh rate. > */ > + dm_set_vupdate_irq(new_state->base.crtc, false); > drm_crtc_vblank_put(new_state->base.crtc); > DRM_DEBUG_DRIVER("%s: crtc=%u VRR on->off: Drop vblank ref\n", > __func__, new_state->base.crtc->base.id); > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > index f741ea3..0b35f84 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > @@ -182,6 +182,15 @@ struct amdgpu_display_manager { > struct common_irq_params > vblank_params[DC_IRQ_SOURCE_VBLANK6 - DC_IRQ_SOURCE_VBLANK1 + 1]; > > + /** > + * @vupdate_params: > + * > + * Vertical update IRQ parameters, passed to registered handlers when > + * triggered. > + */ > + struct common_irq_params > + vupdate_params[DC_IRQ_SOURCE_VUPDATE6 - DC_IRQ_SOURCE_VUPDATE1 + 1]; > + > spinlock_t irq_handler_list_table_lock; > > struct backlight_device *backlight_dev; > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c > index cd10f77..fee7837 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c > @@ -674,11 +674,30 @@ static int amdgpu_dm_set_crtc_irq_state(struct amdgpu_device *adev, > __func__); > } > > +static int amdgpu_dm_set_vupdate_irq_state(struct amdgpu_device *adev, > + struct amdgpu_irq_src *source, > + unsigned crtc_id, should be unsigned int crtc_id > + enum amdgpu_interrupt_state state) > +{ > + return dm_irq_state( > + adev, > + source, > + crtc_id, > + state, > + IRQ_TYPE_VUPDATE, > + __func__); > +} > + > static const struct amdgpu_irq_src_funcs dm_crtc_irq_funcs = { > .set = amdgpu_dm_set_crtc_irq_state, > .process = amdgpu_dm_irq_handler, > }; > > +static const struct amdgpu_irq_src_funcs dm_vupdate_irq_funcs = { > + .set = amdgpu_dm_set_vupdate_irq_state, > + .process = amdgpu_dm_irq_handler, > +}; > + > static const struct amdgpu_irq_src_funcs dm_pageflip_irq_funcs = { > .set = amdgpu_dm_set_pflip_irq_state, > .process = amdgpu_dm_irq_handler, > @@ -695,6 +714,9 @@ void amdgpu_dm_set_irq_funcs(struct amdgpu_device *adev) > adev->crtc_irq.num_types = adev->mode_info.num_crtc; > adev->crtc_irq.funcs = &dm_crtc_irq_funcs; > > + adev->vupdate_irq.num_types = adev->mode_info.num_crtc; > + adev->vupdate_irq.funcs = &dm_vupdate_irq_funcs; > + > adev->pageflip_irq.num_types = adev->mode_info.num_crtc; > adev->pageflip_irq.funcs = &dm_pageflip_irq_funcs; > > diff --git a/drivers/gpu/drm/amd/display/dc/irq/dce110/irq_service_dce110.c b/drivers/gpu/drm/amd/display/dc/irq/dce110/irq_service_dce110.c > index afe0876..86987f5 100644 > --- a/drivers/gpu/drm/amd/display/dc/irq/dce110/irq_service_dce110.c > +++ b/drivers/gpu/drm/amd/display/dc/irq/dce110/irq_service_dce110.c > @@ -81,6 +81,11 @@ static const struct irq_source_info_funcs vblank_irq_info_funcs = { > .ack = NULL > }; > > +static const struct irq_source_info_funcs vupdate_irq_info_funcs = { > + .set = NULL, > + .ack = NULL > +}; > + > #define hpd_int_entry(reg_num)\ > [DC_IRQ_SOURCE_HPD1 + reg_num] = {\ > .enable_reg = mmHPD ## reg_num ## _DC_HPD_INT_CONTROL,\ > @@ -137,7 +142,7 @@ static const struct irq_source_info_funcs vblank_irq_info_funcs = { > CRTC_V_UPDATE_INT_STATUS__CRTC_V_UPDATE_INT_CLEAR_MASK,\ > .ack_value =\ > CRTC_V_UPDATE_INT_STATUS__CRTC_V_UPDATE_INT_CLEAR_MASK,\ > - .funcs = &vblank_irq_info_funcs\ > + .funcs = &vupdate_irq_info_funcs\ > } > > #define vblank_int_entry(reg_num)\ > diff --git a/drivers/gpu/drm/amd/display/dc/irq/dce120/irq_service_dce120.c b/drivers/gpu/drm/amd/display/dc/irq/dce120/irq_service_dce120.c > index 1ea7256..750ba0a 100644 > --- a/drivers/gpu/drm/amd/display/dc/irq/dce120/irq_service_dce120.c > +++ b/drivers/gpu/drm/amd/display/dc/irq/dce120/irq_service_dce120.c > @@ -84,6 +84,11 @@ static const struct irq_source_info_funcs vblank_irq_info_funcs = { > .ack = NULL > }; > > +static const struct irq_source_info_funcs vupdate_irq_info_funcs = { > + .set = NULL, > + .ack = NULL > +}; > + > #define BASE_INNER(seg) \ > DCE_BASE__INST0_SEG ## seg > > @@ -140,7 +145,7 @@ static const struct irq_source_info_funcs vblank_irq_info_funcs = { > IRQ_REG_ENTRY(CRTC, reg_num,\ > CRTC_INTERRUPT_CONTROL, CRTC_V_UPDATE_INT_MSK,\ > CRTC_V_UPDATE_INT_STATUS, CRTC_V_UPDATE_INT_CLEAR),\ > - .funcs = &vblank_irq_info_funcs\ > + .funcs = &vupdate_irq_info_funcs\ > } > > #define vblank_int_entry(reg_num)\ > diff --git a/drivers/gpu/drm/amd/display/dc/irq/dce80/irq_service_dce80.c b/drivers/gpu/drm/amd/display/dc/irq/dce80/irq_service_dce80.c > index 8a2066c..de218fe 100644 > --- a/drivers/gpu/drm/amd/display/dc/irq/dce80/irq_service_dce80.c > +++ b/drivers/gpu/drm/amd/display/dc/irq/dce80/irq_service_dce80.c > @@ -84,6 +84,10 @@ static const struct irq_source_info_funcs vblank_irq_info_funcs = { > .ack = NULL > }; > > +static const struct irq_source_info_funcs vupdate_irq_info_funcs = { > + .set = NULL, > + .ack = NULL > +}; > > #define hpd_int_entry(reg_num)\ > [DC_IRQ_SOURCE_INVALID + reg_num] = {\ > @@ -142,7 +146,7 @@ static const struct irq_source_info_funcs vblank_irq_info_funcs = { > CRTC_V_UPDATE_INT_STATUS__CRTC_V_UPDATE_INT_CLEAR_MASK,\ > .ack_value =\ > CRTC_V_UPDATE_INT_STATUS__CRTC_V_UPDATE_INT_CLEAR_MASK,\ > - .funcs = &vblank_irq_info_funcs\ > + .funcs = &vupdate_irq_info_funcs\ > } > > #define vblank_int_entry(reg_num)\ > diff --git a/drivers/gpu/drm/amd/display/dc/irq/dcn10/irq_service_dcn10.c b/drivers/gpu/drm/amd/display/dc/irq/dcn10/irq_service_dcn10.c > index e04ae49..10ac6de 100644 > --- a/drivers/gpu/drm/amd/display/dc/irq/dcn10/irq_service_dcn10.c > +++ b/drivers/gpu/drm/amd/display/dc/irq/dcn10/irq_service_dcn10.c > @@ -56,6 +56,18 @@ enum dc_irq_source to_dal_irq_source_dcn10( > return DC_IRQ_SOURCE_VBLANK5; > case DCN_1_0__SRCID__DC_D6_OTG_VSTARTUP: > return DC_IRQ_SOURCE_VBLANK6; > + case DCN_1_0__SRCID__OTG0_IHC_V_UPDATE_NO_LOCK_INTERRUPT: > + return DC_IRQ_SOURCE_VUPDATE1; > + case DCN_1_0__SRCID__OTG1_IHC_V_UPDATE_NO_LOCK_INTERRUPT: > + return DC_IRQ_SOURCE_VUPDATE2; > + case DCN_1_0__SRCID__OTG2_IHC_V_UPDATE_NO_LOCK_INTERRUPT: > + return DC_IRQ_SOURCE_VUPDATE3; > + case DCN_1_0__SRCID__OTG3_IHC_V_UPDATE_NO_LOCK_INTERRUPT: > + return DC_IRQ_SOURCE_VUPDATE4; > + case DCN_1_0__SRCID__OTG4_IHC_V_UPDATE_NO_LOCK_INTERRUPT: > + return DC_IRQ_SOURCE_VUPDATE5; > + case DCN_1_0__SRCID__OTG5_IHC_V_UPDATE_NO_LOCK_INTERRUPT: > + return DC_IRQ_SOURCE_VUPDATE6; > case DCN_1_0__SRCID__HUBP0_FLIP_INTERRUPT: > return DC_IRQ_SOURCE_PFLIP1; > case DCN_1_0__SRCID__HUBP1_FLIP_INTERRUPT: > @@ -153,6 +165,11 @@ static const struct irq_source_info_funcs vblank_irq_info_funcs = { > .ack = NULL > }; > > +static const struct irq_source_info_funcs vupdate_no_lock_irq_info_funcs = { > + .set = NULL, > + .ack = NULL > +}; > + > #define BASE_INNER(seg) \ > DCE_BASE__INST0_SEG ## seg > > @@ -203,12 +220,15 @@ static const struct irq_source_info_funcs vblank_irq_info_funcs = { > .funcs = &pflip_irq_info_funcs\ > } > > -#define vupdate_int_entry(reg_num)\ > +/* vupdate_no_lock_int_entry maps to DC_IRQ_SOURCE_VUPDATEx, to match semantic > + * of DCE's DC_IRQ_SOURCE_VUPDATEx. > + */ > +#define vupdate_no_lock_int_entry(reg_num)\ > [DC_IRQ_SOURCE_VUPDATE1 + reg_num] = {\ > IRQ_REG_ENTRY(OTG, reg_num,\ > - OTG_GLOBAL_SYNC_STATUS, VUPDATE_INT_EN,\ > - OTG_GLOBAL_SYNC_STATUS, VUPDATE_EVENT_CLEAR),\ > - .funcs = &vblank_irq_info_funcs\ > + OTG_GLOBAL_SYNC_STATUS, VUPDATE_NO_LOCK_INT_EN,\ > + OTG_GLOBAL_SYNC_STATUS, VUPDATE_NO_LOCK_EVENT_CLEAR),\ > + .funcs = &vupdate_no_lock_irq_info_funcs\ > } > > #define vblank_int_entry(reg_num)\ > @@ -315,12 +335,12 @@ irq_source_info_dcn10[DAL_IRQ_SOURCES_NUMBER] = { > dc_underflow_int_entry(6), > [DC_IRQ_SOURCE_DMCU_SCP] = dummy_irq_entry(), > [DC_IRQ_SOURCE_VBIOS_SW] = dummy_irq_entry(), > - vupdate_int_entry(0), > - vupdate_int_entry(1), > - vupdate_int_entry(2), > - vupdate_int_entry(3), > - vupdate_int_entry(4), > - vupdate_int_entry(5), > + vupdate_no_lock_int_entry(0), > + vupdate_no_lock_int_entry(1), > + vupdate_no_lock_int_entry(2), > + vupdate_no_lock_int_entry(3), > + vupdate_no_lock_int_entry(4), > + vupdate_no_lock_int_entry(5), > vblank_int_entry(0), > vblank_int_entry(1), > vblank_int_entry(2), > Nicholas Kazlauskas _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx