On Thu, Apr 05, 2018 at 10:15:57PM +0200, Thomas Hellstrom wrote: > On 04/05/2018 09:50 PM, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrj...@linux.intel.com> > > > > Instead of plane->fb (which we're going to deprecate for atomic drivers) > > we need to look at plane->state->fb. The maze of code leading to > > vmw_kms_helper_dirty() wasn't particularly clear, but my analysis > > concluded that the calls originating from > > vmw_*_primary_plane_atomic_update() > > all pass in the crtc which means we'll never end up in this branch > > of the function. All other callers use drm_modeset_lock_all() somewhere > > higher up, which means accessing plane->state is safe. We'll toss in > > a lockdep assert to catch wrongdoers. > > > > Cc: Thomas Hellstrom <thellst...@vmware.com> > > Cc: Sinclair Yeh <s...@vmware.com> > > Cc: VMware Graphics <linux-graphics-maintai...@vmware.com> > > Cc: Daniel Vetter <daniel.vet...@ffwll.ch> > > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > > --- > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > index a2a796b4cc23..5a824125c231 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > @@ -2326,9 +2326,18 @@ int vmw_kms_helper_dirty(struct vmw_private > > *dev_priv, > > } else { > > list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list, > > head) { > > - if (crtc->primary->fb != &framebuffer->base) > > - continue; > > - units[num_units++] = vmw_crtc_to_du(crtc); > > + struct drm_plane *plane = crtc->primary; > > + > > + /* > > + * vmw_*_primary_plane_atomic_update() pass in the crtc, > > + * and so don't end up here. All other callers use > > + * drm_modeset_lock_all(), hence we can access the > > + * plane state safely. > > + */ > > + lockdep_assert_held(&plane->mutex); > > + > I think we can remove the comment (it's a helper, so current users may > not be future users), > but the lockdep assert should be OK.
OK. > > + if (plane->state->fb != &framebuffer->base) > > + units[num_units++] = vmw_crtc_to_du(crtc); > > This doesn't seem to do what the original code did... Whoops. Good catch. > > > } > > } > > > > /Thomas > -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel