[PATCH 1/2] drm/amdgpu: fix vkms hrtimer settings
Alex Deucher
alexdeucher at gmail.com
Tue Nov 23 14:40:56 UTC 2021
On Mon, Nov 22, 2021 at 8:59 PM Cui, Flora <Flora.Cui at amd.com> wrote:
>
> [Public]
>
>
> Modprobe -r amdgpu get oops in amdgpu_vkms_sw_fini()
>
> for (i = 0; i < adev->mode_info.num_crtc; i++)
>
> if (adev->mode_info.crtcs[i])
>
> hrtimer_cancel(&adev->mode_info.crtcs[i]->vblank_timer);
>
> adev->mode_info.crtcs[i]->vblank_timer is not initiated as vkms init its own amdgpu_vkms_output-> vblank_hrtimer. This patch drop amdgpu_vkms_output-> vblank_hrtimer and try with adev->mode_info.crtcs[i]->vblank_timer to keep align with amdgpu_dm & dce_vx_0.c
>
>
I think this might be better as two patches. The simple fix for this
problem would be:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index ce982afeff91..b620a6a3cb9f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -504,8 +504,7 @@ static int amdgpu_vkms_sw_fini(void *handle)
int i = 0;
for (i = 0; i < adev->mode_info.num_crtc; i++)
- if (adev->mode_info.crtcs[i])
- hrtimer_cancel(&adev->mode_info.crtcs[i]->vblank_timer);
+ hrtimer_cancel(&adev->amdgpu_vkms_output[i].vblank_hrtimer);
kfree(adev->mode_info.bios_hardcoded_edid);
kfree(adev->amdgpu_vkms_output);
Then apply your patch on top to share more code with the existing dce files.
Alex
>
>
>
> From: Deucher, Alexander <Alexander.Deucher at amd.com>
> Sent: 2021年11月23日 0:43
> To: Chen, Guchun <Guchun.Chen at amd.com>; Cui, Flora <Flora.Cui at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 1/2] drm/amdgpu: fix vkms hrtimer settings
>
>
>
> [Public]
>
>
>
> Can you explain how the current code is failing? It's not immediately obvious to me. I'm not opposed to this change, it's just not clear to me where the current code fails.
>
>
>
> Alex
>
>
>
> ________________________________
>
> From: Chen, Guchun <Guchun.Chen at amd.com>
> Sent: Monday, November 22, 2021 8:49 AM
> To: Cui, Flora <Flora.Cui at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher at amd.com>
> Subject: RE: [PATCH 1/2] drm/amdgpu: fix vkms hrtimer settings
>
>
>
> [Public]
>
> Series is:
> Reviewed-by: Guchun Chen <guchun.chen at amd.com>
>
> +Alex to comment this series as well.
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Cui, Flora <Flora.Cui at amd.com>
> Sent: Monday, November 22, 2021 5:04 PM
> To: amd-gfx at lists.freedesktop.org; Chen, Guchun <Guchun.Chen at amd.com>
> Cc: Cui, Flora <Flora.Cui at amd.com>
> Subject: [PATCH 1/2] drm/amdgpu: fix vkms hrtimer settings
>
> otherwise adev->mode_info.crtcs[] is NULL
>
> Signed-off-by: Flora Cui <flora.cui at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 38 ++++++++++++++++-------- drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.h | 5 ++--
> 2 files changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> index ce982afeff91..6c62c45e3e3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> @@ -16,6 +16,8 @@
> #include "ivsrcid/ivsrcid_vislands30.h"
> #include "amdgpu_vkms.h"
> #include "amdgpu_display.h"
> +#include "atom.h"
> +#include "amdgpu_irq.h"
>
> /**
> * DOC: amdgpu_vkms
> @@ -41,14 +43,13 @@ static const u32 amdgpu_vkms_formats[] = {
>
> static enum hrtimer_restart amdgpu_vkms_vblank_simulate(struct hrtimer *timer) {
> - struct amdgpu_vkms_output *output = container_of(timer,
> - struct amdgpu_vkms_output,
> - vblank_hrtimer);
> - struct drm_crtc *crtc = &output->crtc;
> + struct amdgpu_crtc *amdgpu_crtc = container_of(timer, struct amdgpu_crtc, vblank_timer);
> + struct drm_crtc *crtc = &amdgpu_crtc->base;
> + struct amdgpu_vkms_output *output =
> +drm_crtc_to_amdgpu_vkms_output(crtc);
> u64 ret_overrun;
> bool ret;
>
> - ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer,
> + ret_overrun = hrtimer_forward_now(&amdgpu_crtc->vblank_timer,
> output->period_ns);
> WARN_ON(ret_overrun != 1);
>
> @@ -65,22 +66,21 @@ static int amdgpu_vkms_enable_vblank(struct drm_crtc *crtc)
> unsigned int pipe = drm_crtc_index(crtc);
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> struct amdgpu_vkms_output *out = drm_crtc_to_amdgpu_vkms_output(crtc);
> + struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>
> drm_calc_timestamping_constants(crtc, &crtc->mode);
>
> - hrtimer_init(&out->vblank_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> - out->vblank_hrtimer.function = &amdgpu_vkms_vblank_simulate;
> out->period_ns = ktime_set(0, vblank->framedur_ns);
> - hrtimer_start(&out->vblank_hrtimer, out->period_ns, HRTIMER_MODE_REL);
> + hrtimer_start(&amdgpu_crtc->vblank_timer, out->period_ns,
> +HRTIMER_MODE_REL);
>
> return 0;
> }
>
> static void amdgpu_vkms_disable_vblank(struct drm_crtc *crtc) {
> - struct amdgpu_vkms_output *out = drm_crtc_to_amdgpu_vkms_output(crtc);
> + struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>
> - hrtimer_cancel(&out->vblank_hrtimer);
> + hrtimer_cancel(&amdgpu_crtc->vblank_timer);
> }
>
> static bool amdgpu_vkms_get_vblank_timestamp(struct drm_crtc *crtc, @@ -92,13 +92,14 @@ static bool amdgpu_vkms_get_vblank_timestamp(struct drm_crtc *crtc,
> unsigned int pipe = crtc->index;
> struct amdgpu_vkms_output *output = drm_crtc_to_amdgpu_vkms_output(crtc);
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> + struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>
> if (!READ_ONCE(vblank->enabled)) {
> *vblank_time = ktime_get();
> return true;
> }
>
> - *vblank_time = READ_ONCE(output->vblank_hrtimer.node.expires);
> + *vblank_time = READ_ONCE(amdgpu_crtc->vblank_timer.node.expires);
>
> if (WARN_ON(*vblank_time == vblank->time))
> return true;
> @@ -165,6 +166,8 @@ static const struct drm_crtc_helper_funcs amdgpu_vkms_crtc_helper_funcs = { static int amdgpu_vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> struct drm_plane *primary, struct drm_plane *cursor) {
> + struct amdgpu_device *adev = drm_to_adev(dev);
> + struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> int ret;
>
> ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor, @@ -176,6 +179,17 @@ static int amdgpu_vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>
> drm_crtc_helper_add(crtc, &amdgpu_vkms_crtc_helper_funcs);
>
> + amdgpu_crtc->crtc_id = drm_crtc_index(crtc);
> + adev->mode_info.crtcs[drm_crtc_index(crtc)] = amdgpu_crtc;
> +
> + amdgpu_crtc->pll_id = ATOM_PPLL_INVALID;
> + amdgpu_crtc->encoder = NULL;
> + amdgpu_crtc->connector = NULL;
> + amdgpu_crtc->vsync_timer_enabled = AMDGPU_IRQ_STATE_DISABLE;
> +
> + hrtimer_init(&amdgpu_crtc->vblank_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + amdgpu_crtc->vblank_timer.function = &amdgpu_vkms_vblank_simulate;
> +
> return ret;
> }
>
> @@ -401,7 +415,7 @@ int amdgpu_vkms_output_init(struct drm_device *dev, {
> struct drm_connector *connector = &output->connector;
> struct drm_encoder *encoder = &output->encoder;
> - struct drm_crtc *crtc = &output->crtc;
> + struct drm_crtc *crtc = &output->crtc.base;
> struct drm_plane *primary, *cursor = NULL;
> int ret;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.h
> index 97f1b79c0724..4f8722ff37c2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.h
> @@ -10,15 +10,14 @@
> #define YRES_MAX 16384
>
> #define drm_crtc_to_amdgpu_vkms_output(target) \
> - container_of(target, struct amdgpu_vkms_output, crtc)
> + container_of(target, struct amdgpu_vkms_output, crtc.base)
>
> extern const struct amdgpu_ip_block_version amdgpu_vkms_ip_block;
>
> struct amdgpu_vkms_output {
> - struct drm_crtc crtc;
> + struct amdgpu_crtc crtc;
> struct drm_encoder encoder;
> struct drm_connector connector;
> - struct hrtimer vblank_hrtimer;
> ktime_t period_ns;
> struct drm_pending_vblank_event *event; };
> --
> 2.25.1
More information about the amd-gfx
mailing list