On Thu, Apr 05, 2018 at 08:15:05PM +0000, Deepak Singh Rawat wrote: > > > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > 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 <thellstrom@xxxxxxxxxx> > > Cc: Sinclair Yeh <syeh@xxxxxxxxxx> > > Cc: VMware Graphics <linux-graphics-maintainer@xxxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel