Hello Mario Kleiner,
The patch e1d09dc0ccc6: "drm/amdgpu: Don't hang in
amdgpu_flip_work_func on disabled crtc." from Feb 19, 2016, leads to
the following static checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:127 amdgpu_flip_work_func() warn:
should this be 'repcnt == -1'
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:136 amdgpu_flip_work_func() error:
double unlock 'spin_lock:&crtc->dev->event_lock'
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:136 amdgpu_flip_work_func() error:
double unlock 'irqsave:flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
64 static void amdgpu_flip_work_func(struct work_struct *__work)
65 {
66 struct amdgpu_flip_work *work =
67 container_of(__work, struct amdgpu_flip_work,
flip_work);
68 struct amdgpu_device *adev = work->adev;
69 struct amdgpu_crtc *amdgpuCrtc =
adev->mode_info.crtcs[work->crtc_id];
70
71 struct drm_crtc *crtc = &amdgpuCrtc->base;
72 unsigned long flags;
73 unsigned i, repcnt = 4;
74 int vpos, hpos, stat, min_udelay = 0;
75 struct drm_vblank_crtc *vblank =
&crtc->dev->vblank[work->crtc_id];
76
77 if (amdgpu_flip_handle_fence(work, &work->excl))
78 return;
79
80 for (i = 0; i < work->shared_count; ++i)
81 if (amdgpu_flip_handle_fence(work, &work->shared[i]))
82 return;
83
84 /* We borrow the event spin lock for protecting flip_status */
85 spin_lock_irqsave(&crtc->dev->event_lock, flags);
86
87 /* If this happens to execute within the "virtually extended"
vblank
88 * interval before the start of the real vblank interval then
it needs
89 * to delay programming the mmio flip until the real vblank is
entered.
90 * This prevents completing a flip too early due to the way we
fudge
91 * our vblank counter and vblank timestamps in order to work
around the
92 * problem that the hw fires vblank interrupts before actual
start of
93 * vblank (when line buffer refilling is done for a frame). It
94 * complements the fudging logic in
amdgpu_get_crtc_scanoutpos() for
95 * timestamping and amdgpu_get_vblank_counter_kms() for vblank
counts.
96 *
97 * In practice this won't execute very often unless on very fast
98 * machines because the time window for this to happen is very
small.
99 */
100 while (amdgpuCrtc->enabled && repcnt--) {
^^^^^^^^
Exists the loop with spin_lock held and repcnt == -1.
101 /* GET_DISTANCE_TO_VBLANKSTART returns distance to real
vblank
102 * start in hpos, and to the "fudged earlier" vblank
start in
103 * vpos.
104 */
105 stat = amdgpu_get_crtc_scanoutpos(adev->ddev,
work->crtc_id,
106
GET_DISTANCE_TO_VBLANKSTART,
107 &vpos, &hpos, NULL,
NULL,
108 &crtc->hwmode);
109
110 if ((stat & (DRM_SCANOUTPOS_VALID |
DRM_SCANOUTPOS_ACCURATE)) !=
111 (DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE) ||
112 !(vpos >= 0 && hpos <= 0))
113 break;
114
115 /* Sleep at least until estimated real start of hw
vblank */
116 spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
117 min_udelay = (-hpos + 1) * max(vblank->linedur_ns /
1000, 5);
118 if (min_udelay > vblank->framedur_ns / 2000) {
119 /* Don't wait ridiculously long - something is
wrong */
120 repcnt = 0;
Exit with spin_lock released and repcnt == 0.
121 break;
122 }
123 usleep_range(min_udelay, 2 * min_udelay);
124 spin_lock_irqsave(&crtc->dev->event_lock, flags);
125 };
126
127 if (!repcnt)
^^^^^^
Assumes exit with zero.
128 DRM_DEBUG_DRIVER("Delay problem on crtc %d: min_udelay
%d, "
129 "framedur %d, linedur %d, stat %d,
vpos %d, "
130 "hpos %d\n", work->crtc_id, min_udelay,
131 vblank->framedur_ns / 1000,
132 vblank->linedur_ns / 1000, stat, vpos,
hpos);
133
134 /* set the flip status */
135 amdgpuCrtc->pflip_status = AMDGPU_FLIP_SUBMITTED;
136 spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
Assumes lock held.
137
138 /* Do the flip (mmio) */
139 adev->mode_info.funcs->page_flip(adev, work->crtc_id,
work->base);
140 }
regards,
dan carpenter
