Re: [Intel-gfx] [PATCH 02/14] drm/i915: Fix g4x cxsr enable condition
On Fri, Sep 17, 2021 at 03:32:51PM +0300, Ville Syrjälä wrote: > On Thu, Sep 16, 2021 at 07:24:21PM +0300, Lisovskiy, Stanislav wrote: > > On Fri, May 14, 2021 at 03:57:39PM +0300, Ville Syrjala wrote: > > > From: Ville Syrjälä > > > > > > The intention was to check whether the primary plane is enabled > > > without any sprites planes being enabled. Instead we ended up checking > > > whether just any one of the planes is enabled. g4x isn't vlv/chv and > > > cxsr only works with the primary plane. Fix the check to examine the > > > bitmask of active planes rather than the number of bits set in said > > > bitmask. > > > > > > Signed-off-by: Ville Syrjälä > > > --- > > > drivers/gpu/drm/i915/intel_pm.c | 5 ++--- > > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index 00a5fe424c5a..2fb496fbed43 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -1376,8 +1376,7 @@ static int g4x_compute_pipe_wm(struct > > > intel_crtc_state *crtc_state) > > > struct intel_atomic_state *state = > > > to_intel_atomic_state(crtc_state->uapi.state); > > > struct g4x_wm_state *wm_state = _state->wm.g4x.optimal; > > > - int num_active_planes = hweight8(crtc_state->active_planes & > > > - ~BIT(PLANE_CURSOR)); > > > + u8 active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR); > > > const struct g4x_pipe_wm *raw; > > > const struct intel_plane_state *old_plane_state; > > > const struct intel_plane_state *new_plane_state; > > > @@ -1417,7 +1416,7 @@ static int g4x_compute_pipe_wm(struct > > > intel_crtc_state *crtc_state) > > > wm_state->sr.cursor = raw->plane[PLANE_CURSOR]; > > > wm_state->sr.fbc = raw->fbc; > > > > > > - wm_state->cxsr = num_active_planes == BIT(PLANE_PRIMARY); > > > + wm_state->cxsr = active_planes == BIT(PLANE_PRIMARY); > > > > Shouldn't this be "active_planes & BIT(PLANE_PRIMARY)" as we might > > have other non-cursor planes enabled, which will then fail or am I missing > > something? > > CxSR is possible only when the primary plane is enabled and the > sprite plane is disabled. Ok, that explains. Reviewed-by: Stanislav Lisovskiy > > -- > Ville Syrjälä > Intel
Re: [Intel-gfx] [PATCH 02/14] drm/i915: Fix g4x cxsr enable condition
On Thu, Sep 16, 2021 at 07:24:21PM +0300, Lisovskiy, Stanislav wrote: > On Fri, May 14, 2021 at 03:57:39PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > The intention was to check whether the primary plane is enabled > > without any sprites planes being enabled. Instead we ended up checking > > whether just any one of the planes is enabled. g4x isn't vlv/chv and > > cxsr only works with the primary plane. Fix the check to examine the > > bitmask of active planes rather than the number of bits set in said > > bitmask. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_pm.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index 00a5fe424c5a..2fb496fbed43 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -1376,8 +1376,7 @@ static int g4x_compute_pipe_wm(struct > > intel_crtc_state *crtc_state) > > struct intel_atomic_state *state = > > to_intel_atomic_state(crtc_state->uapi.state); > > struct g4x_wm_state *wm_state = _state->wm.g4x.optimal; > > - int num_active_planes = hweight8(crtc_state->active_planes & > > -~BIT(PLANE_CURSOR)); > > + u8 active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR); > > const struct g4x_pipe_wm *raw; > > const struct intel_plane_state *old_plane_state; > > const struct intel_plane_state *new_plane_state; > > @@ -1417,7 +1416,7 @@ static int g4x_compute_pipe_wm(struct > > intel_crtc_state *crtc_state) > > wm_state->sr.cursor = raw->plane[PLANE_CURSOR]; > > wm_state->sr.fbc = raw->fbc; > > > > - wm_state->cxsr = num_active_planes == BIT(PLANE_PRIMARY); > > + wm_state->cxsr = active_planes == BIT(PLANE_PRIMARY); > > Shouldn't this be "active_planes & BIT(PLANE_PRIMARY)" as we might > have other non-cursor planes enabled, which will then fail or am I missing > something? CxSR is possible only when the primary plane is enabled and the sprite plane is disabled. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 02/14] drm/i915: Fix g4x cxsr enable condition
On Fri, May 14, 2021 at 03:57:39PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > The intention was to check whether the primary plane is enabled > without any sprites planes being enabled. Instead we ended up checking > whether just any one of the planes is enabled. g4x isn't vlv/chv and > cxsr only works with the primary plane. Fix the check to examine the > bitmask of active planes rather than the number of bits set in said > bitmask. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_pm.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 00a5fe424c5a..2fb496fbed43 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -1376,8 +1376,7 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state > *crtc_state) > struct intel_atomic_state *state = > to_intel_atomic_state(crtc_state->uapi.state); > struct g4x_wm_state *wm_state = _state->wm.g4x.optimal; > - int num_active_planes = hweight8(crtc_state->active_planes & > - ~BIT(PLANE_CURSOR)); > + u8 active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR); > const struct g4x_pipe_wm *raw; > const struct intel_plane_state *old_plane_state; > const struct intel_plane_state *new_plane_state; > @@ -1417,7 +1416,7 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state > *crtc_state) > wm_state->sr.cursor = raw->plane[PLANE_CURSOR]; > wm_state->sr.fbc = raw->fbc; > > - wm_state->cxsr = num_active_planes == BIT(PLANE_PRIMARY); > + wm_state->cxsr = active_planes == BIT(PLANE_PRIMARY); Shouldn't this be "active_planes & BIT(PLANE_PRIMARY)" as we might have other non-cursor planes enabled, which will then fail or am I missing something? Stan > > level = G4X_WM_LEVEL_HPLL; > if (!g4x_raw_crtc_wm_is_valid(crtc_state, level)) > -- > 2.26.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/14] drm/i915: Fix g4x cxsr enable condition
From: Ville Syrjälä The intention was to check whether the primary plane is enabled without any sprites planes being enabled. Instead we ended up checking whether just any one of the planes is enabled. g4x isn't vlv/chv and cxsr only works with the primary plane. Fix the check to examine the bitmask of active planes rather than the number of bits set in said bitmask. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 00a5fe424c5a..2fb496fbed43 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1376,8 +1376,7 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state) struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state); struct g4x_wm_state *wm_state = _state->wm.g4x.optimal; - int num_active_planes = hweight8(crtc_state->active_planes & -~BIT(PLANE_CURSOR)); + u8 active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR); const struct g4x_pipe_wm *raw; const struct intel_plane_state *old_plane_state; const struct intel_plane_state *new_plane_state; @@ -1417,7 +1416,7 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state) wm_state->sr.cursor = raw->plane[PLANE_CURSOR]; wm_state->sr.fbc = raw->fbc; - wm_state->cxsr = num_active_planes == BIT(PLANE_PRIMARY); + wm_state->cxsr = active_planes == BIT(PLANE_PRIMARY); level = G4X_WM_LEVEL_HPLL; if (!g4x_raw_crtc_wm_is_valid(crtc_state, level)) -- 2.26.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
