On Thu, Apr 05, 2018 at 08:15:05PM +0000, Deepak Singh Rawat wrote:
>
> >
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> >
> > Instead of looking at plane->fb let's look at the proper new
> > plane state.
> >
> > Not that the code makes a ton of sense. It's only going through the
> > crtcs in the atomic state, so assuming not all of them are included
> > we're not even calculating the total bandwidth here. Also we're
> > not considering whether each crtc is actually enabled or not.
> >
> > 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 | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 6728c6247b4b..a2a796b4cc23 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -1536,9 +1536,13 @@ vmw_kms_atomic_check_modeset(struct
> > drm_device *dev,
> > unsigned long requested_bb_mem = 0;
> >
> > if (dev_priv->active_display_unit ==
> > vmw_du_screen_target) {
> > - if (crtc->primary->fb) {
> > - int cpp = crtc->primary->fb->pitches[0] /
> > - crtc->primary->fb->width;
> > + struct drm_plane *plane = crtc->primary;
> > + struct drm_plane_state *plane_state;
> > +
> > + plane_state =
> > drm_atomic_get_new_plane_state(state, plane);
> > +
> > + if (plane_state && plane_state->fb) {
> > + int cpp = plane_state->fb->format->cpp[0];
>
> Hi Ville,
>
> Thanks for the patch, recently I have done some refactoring of this code area
> which is no yet sent to dri-devel. But the refactored code eliminated the need
> to look the fb
>
> https://cgit.freedesktop.org/mesa/vmwgfx/commit/?id=c54cdb6549b7d1c04ff61e639fc0e6de0dcc1ed6
Hmm. What's the timelike for landing that stuff?
A cursory glance tells me we should just change the current code with
s/cpp/4/ and it should still be fine?
BTW the drm_for_each_crtc(crtc, dev) loop in there doesn't look entirely
kosher. It's potentially going to access crtc->state w/o holding the lock.
>
> There is still some future work to be done in this area.
>
> >
> > requested_bb_mem += crtc->mode.hdisplay
> > * cpp *
> > crtc->mode.vdisplay;
> > --
> > 2.16.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
