| Message ID | 20180203051302.9974-4-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2018-02-03 12:12 AM, Dhinakaran Pandiyan wrote: > 570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the > return type for drm_crtc_vblank_count() to u64. This could cause > potential problems if the return value is used in arithmetic operations > with a 32-bit reference HW vblank count. Explicitly typecasting this down > to u32 either fixes a potential problem or serves to add clarity in case > the typecasting was implicitly done. > > Cc: Keith Packard <keithp@keithp.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > index 38d47559f098..c2fa5d55f04e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > @@ -207,7 +207,7 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc, > amdgpu_bo_unreserve(new_abo); > > work->base = base; > - work->target_vblank = target - drm_crtc_vblank_count(crtc) + > + work->target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) + > amdgpu_get_vblank_counter_kms(dev, work->crtc_id); > > /* we borrow the event spin lock for protecting flip_wrok */ > 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 1ce4c98385e3..b7254a29b34a 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3836,7 +3836,7 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc, > > > /* Prepare wait for target vblank early - before the fence-waits */ > - target_vblank = target - drm_crtc_vblank_count(crtc) + > + target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) + > amdgpu_get_vblank_counter_kms(crtc->dev, acrtc->crtc_id); > > /* TODO This might fail and hence better not used, wait > @@ -3982,7 +3982,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > amdgpu_dm_do_flip( > crtc, > fb, > - drm_crtc_vblank_count(crtc) + *wait_for_vblank, > + (uint32_t)drm_crtc_vblank_count(crtc) + *wait_for_vblank, > dm_state->context); > } > >
On Sat, Feb 3, 2018 at 12:12 AM, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote: > 570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the > return type for drm_crtc_vblank_count() to u64. This could cause > potential problems if the return value is used in arithmetic operations > with a 32-bit reference HW vblank count. Explicitly typecasting this down > to u32 either fixes a potential problem or serves to add clarity in case > the typecasting was implicitly done. > > Cc: Keith Packard <keithp@keithp.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Acked-by: Alex Deucher <alexander.deucher@amd.com> for both this patch and the radeon one. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > index 38d47559f098..c2fa5d55f04e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > @@ -207,7 +207,7 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc, > amdgpu_bo_unreserve(new_abo); > > work->base = base; > - work->target_vblank = target - drm_crtc_vblank_count(crtc) + > + work->target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) + > amdgpu_get_vblank_counter_kms(dev, work->crtc_id); > > /* we borrow the event spin lock for protecting flip_wrok */ > 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 1ce4c98385e3..b7254a29b34a 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3836,7 +3836,7 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc, > > > /* Prepare wait for target vblank early - before the fence-waits */ > - target_vblank = target - drm_crtc_vblank_count(crtc) + > + target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) + > amdgpu_get_vblank_counter_kms(crtc->dev, acrtc->crtc_id); > > /* TODO This might fail and hence better not used, wait > @@ -3982,7 +3982,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > amdgpu_dm_do_flip( > crtc, > fb, > - drm_crtc_vblank_count(crtc) + *wait_for_vblank, > + (uint32_t)drm_crtc_vblank_count(crtc) + *wait_for_vblank, > dm_state->context); > } > > -- > 2.14.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 38d47559f098..c2fa5d55f04e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -207,7 +207,7 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc, amdgpu_bo_unreserve(new_abo); work->base = base; - work->target_vblank = target - drm_crtc_vblank_count(crtc) + + work->target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) + amdgpu_get_vblank_counter_kms(dev, work->crtc_id); /* we borrow the event spin lock for protecting flip_wrok */ 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 1ce4c98385e3..b7254a29b34a 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3836,7 +3836,7 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc, /* Prepare wait for target vblank early - before the fence-waits */ - target_vblank = target - drm_crtc_vblank_count(crtc) + + target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) + amdgpu_get_vblank_counter_kms(crtc->dev, acrtc->crtc_id); /* TODO This might fail and hence better not used, wait @@ -3982,7 +3982,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, amdgpu_dm_do_flip( crtc, fb, - drm_crtc_vblank_count(crtc) + *wait_for_vblank, + (uint32_t)drm_crtc_vblank_count(crtc) + *wait_for_vblank, dm_state->context); }
570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the return type for drm_crtc_vblank_count() to u64. This could cause potential problems if the return value is used in arithmetic operations with a 32-bit reference HW vblank count. Explicitly typecasting this down to u32 either fixes a potential problem or serves to add clarity in case the typecasting was implicitly done. Cc: Keith Packard <keithp@keithp.com> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: Harry Wentland <harry.wentland@amd.com> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)