| Message ID | 20220819110849.192037-3-dakr@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for vc4 hotplug rework | expand |
On Fri, Aug 19, 2022 at 01:08:47PM +0200, Danilo Krummrich wrote: > (Hardware) resources which are bound to the driver and device lifecycle > must not be accessed after the device and driver are unbound. > > However, the DRM device isn't freed as long as the last user closed it, > hence userspace can still call into the driver. > > Therefore protect the critical sections which are accessing those > resources with drm_dev_enter() and drm_dev_exit(). > > Fixes: 9872c7a31921 ("drm/vc4: plane: Switch to drmm_universal_plane_alloc()") > Signed-off-by: Danilo Krummrich <dakr@redhat.com> > --- > drivers/gpu/drm/vc4/vc4_plane.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index eff9c63adfa7..c46acb770036 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -19,6 +19,7 @@ > #include <drm/drm_atomic_helper.h> > #include <drm/drm_atomic_uapi.h> > #include <drm/drm_blend.h> > +#include <drm/drm_drv.h> > #include <drm/drm_fb_dma_helper.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_framebuffer.h> > @@ -1219,6 +1220,10 @@ u32 vc4_plane_write_dlist(struct drm_plane *plane, u32 __iomem *dlist) > { > struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state); > int i; > + int idx; > + > + if (!drm_dev_enter(plane->dev, &idx)) > + goto out; > > vc4_state->hw_dlist = dlist; > > @@ -1226,6 +1231,9 @@ u32 vc4_plane_write_dlist(struct drm_plane *plane, u32 __iomem *dlist) > for (i = 0; i < vc4_state->dlist_count; i++) > writel(vc4_state->dlist[i], &dlist[i]); > > + drm_dev_exit(idx); > + > +out: > return vc4_state->dlist_count; > } > > @@ -1245,6 +1253,7 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb) > struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state); > struct drm_gem_dma_object *bo = drm_fb_dma_get_gem_obj(fb, 0); > uint32_t addr; > + int idx; > > /* We're skipping the address adjustment for negative origin, > * because this is only called on the primary plane. > @@ -1252,12 +1261,17 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb) > WARN_ON_ONCE(plane->state->crtc_x < 0 || plane->state->crtc_y < 0); > addr = bo->dma_addr + fb->offsets[0]; > > + if (!drm_dev_enter(plane->dev, &idx)) > + return; > + > /* Write the new address into the hardware immediately. The > * scanout will start from this address as soon as the FIFO > * needs to refill with pixels. > */ > writel(addr, &vc4_state->hw_dlist[vc4_state->ptr0_offset]); > > + drm_dev_exit(idx); > + You did change the CRTC patch, but the comment to protect the entire function also applies to this one. Maxime
Hi Maxime, On 8/24/22 17:48, Maxime Ripard wrote: >> @@ -1252,12 +1261,17 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb) >> WARN_ON_ONCE(plane->state->crtc_x < 0 || plane->state->crtc_y < 0); >> addr = bo->dma_addr + fb->offsets[0]; >> >> + if (!drm_dev_enter(plane->dev, &idx)) >> + return; >> + >> /* Write the new address into the hardware immediately. The >> * scanout will start from this address as soon as the FIFO >> * needs to refill with pixels. >> */ >> writel(addr, &vc4_state->hw_dlist[vc4_state->ptr0_offset]); >> >> + drm_dev_exit(idx); >> + > > You did change the CRTC patch, but the comment to protect the entire > function also applies to this one. Yes, I changed this one too, but missed this particular function - gonna send a v3. - Danilo > > Maxime
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index eff9c63adfa7..c46acb770036 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -19,6 +19,7 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_atomic_uapi.h> #include <drm/drm_blend.h> +#include <drm/drm_drv.h> #include <drm/drm_fb_dma_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_framebuffer.h> @@ -1219,6 +1220,10 @@ u32 vc4_plane_write_dlist(struct drm_plane *plane, u32 __iomem *dlist) { struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state); int i; + int idx; + + if (!drm_dev_enter(plane->dev, &idx)) + goto out; vc4_state->hw_dlist = dlist; @@ -1226,6 +1231,9 @@ u32 vc4_plane_write_dlist(struct drm_plane *plane, u32 __iomem *dlist) for (i = 0; i < vc4_state->dlist_count; i++) writel(vc4_state->dlist[i], &dlist[i]); + drm_dev_exit(idx); + +out: return vc4_state->dlist_count; } @@ -1245,6 +1253,7 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb) struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state); struct drm_gem_dma_object *bo = drm_fb_dma_get_gem_obj(fb, 0); uint32_t addr; + int idx; /* We're skipping the address adjustment for negative origin, * because this is only called on the primary plane. @@ -1252,12 +1261,17 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb) WARN_ON_ONCE(plane->state->crtc_x < 0 || plane->state->crtc_y < 0); addr = bo->dma_addr + fb->offsets[0]; + if (!drm_dev_enter(plane->dev, &idx)) + return; + /* Write the new address into the hardware immediately. The * scanout will start from this address as soon as the FIFO * needs to refill with pixels. */ writel(addr, &vc4_state->hw_dlist[vc4_state->ptr0_offset]); + drm_dev_exit(idx); + /* Also update the CPU-side dlist copy, so that any later * atomic updates that don't do a new modeset on our plane * also use our updated address. @@ -1271,6 +1285,10 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane); struct vc4_plane_state *vc4_state, *new_vc4_state; + int idx; + + if (!drm_dev_enter(plane->dev, &idx)) + return; swap(plane->state->fb, new_plane_state->fb); plane->state->crtc_x = new_plane_state->crtc_x; @@ -1333,6 +1351,8 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, &vc4_state->hw_dlist[vc4_state->pos2_offset]); writel(vc4_state->dlist[vc4_state->ptr0_offset], &vc4_state->hw_dlist[vc4_state->ptr0_offset]); + + drm_dev_exit(idx); } static int vc4_plane_atomic_async_check(struct drm_plane *plane,
(Hardware) resources which are bound to the driver and device lifecycle must not be accessed after the device and driver are unbound. However, the DRM device isn't freed as long as the last user closed it, hence userspace can still call into the driver. Therefore protect the critical sections which are accessing those resources with drm_dev_enter() and drm_dev_exit(). Fixes: 9872c7a31921 ("drm/vc4: plane: Switch to drmm_universal_plane_alloc()") Signed-off-by: Danilo Krummrich <dakr@redhat.com> --- drivers/gpu/drm/vc4/vc4_plane.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)