diff mbox series

[v2,4/4] drm/i915/display: Remove MBUS joining invalid TODOs

Message ID 20220322214616.160640-4-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/4] drm/i915/display: Program PIPE_MBUS_DBOX_CTL with adl-p values | expand

Commit Message

Souza, Jose March 22, 2022, 9:46 p.m. UTC
skl_compute_ddb() will for a modeset in all pipes when MBUS joining
changes between states, so all pipes will be disabled, have all
MBUS related registers updated and then each pipe enabled.
So no vblank syncronization is necessary and here droping those TODOs.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 -----
 1 file changed, 5 deletions(-)

Comments

Caz Yokoyama March 22, 2022, 9:58 p.m. UTC | #1
On Tue, Mar 22, 2022 at 2:45 PM José Roberto de Souza <jose.souza@intel.com>
wrote:

> skl_compute_ddb() will for a modeset in all pipes when MBUS joining
> changes between states, so all pipes will be disabled, have all
> MBUS related registers updated and then each pipe enabled.
>
I am not clear what you want to say here. Could you rephrase above 3 lines?


> So no vblank syncronization is necessary and here droping those TODOs.
>

          dropping
-caz


>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index cf290bb704221..9ccf0f062862c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6066,7 +6066,6 @@ skl_compute_ddb(struct intel_atomic_state *state)
>                         return ret;
>
>                 if (old_dbuf_state->joined_mbus !=
> new_dbuf_state->joined_mbus) {
> -                       /* TODO: Implement vblank synchronized MBUS
> joining changes */
>                         ret = intel_modeset_all_pipes(state);
>                         if (ret)
>                                 return ret;
> @@ -8195,10 +8194,6 @@ static void update_mbus_pre_enable(struct
> intel_atomic_state *state)
>         if (!HAS_MBUS_JOINING(dev_priv))
>                 return;
>
> -       /*
> -        * TODO: Implement vblank synchronized MBUS joining changes.
> -        * Must be properly coordinated with dbuf reprogramming.
> -        */
>         if (dbuf_state->joined_mbus) {
>                 mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
>                         MBUS_JOIN_PIPE_SELECT_NONE;
> --
> 2.35.1
>
>
Souza, Jose March 22, 2022, 10:02 p.m. UTC | #2
On Tue, 2022-03-22 at 14:58 -0700, Caz Yokoyama wrote:
> 
> 
> On Tue, Mar 22, 2022 at 2:45 PM José Roberto de Souza <jose.souza@intel.com> wrote:
> > skl_compute_ddb() will for a modeset in all pipes when MBUS joining
> > changes between states, so all pipes will be disabled, have all
> > MBUS related registers updated and then each pipe enabled.
> > 
> 
> I am not clear what you want to say here. Could you rephrase above 3 lines?

Ops it should be:

skl_compute_ddb() will do a modeset in all pipes when MBUS joining changes between atomic commits, so all pipes will be disabled, have all MBUS
related registers updated and then each pipe enabled.


>  
> > So no vblank syncronization is necessary and here droping those TODOs.
> > 
> 
>                                                                                       dropping
> -caz
>  
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 5 -----
> >  1 file changed, 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index cf290bb704221..9ccf0f062862c 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6066,7 +6066,6 @@ skl_compute_ddb(struct intel_atomic_state *state)
> >                         return ret;
> > 
> >                 if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
> > -                       /* TODO: Implement vblank synchronized MBUS joining changes */
> >                         ret = intel_modeset_all_pipes(state);
> >                         if (ret)
> >                                 return ret;
> > @@ -8195,10 +8194,6 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
> >         if (!HAS_MBUS_JOINING(dev_priv))
> >                 return;
> > 
> > -       /*
> > -        * TODO: Implement vblank synchronized MBUS joining changes.
> > -        * Must be properly coordinated with dbuf reprogramming.
> > -        */
> >         if (dbuf_state->joined_mbus) {
> >                 mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
> >                         MBUS_JOIN_PIPE_SELECT_NONE;
> > -- 
> > 2.35.1
> > 
> 
>
Ville Syrjälä March 24, 2022, 11:12 a.m. UTC | #3
On Tue, Mar 22, 2022 at 02:46:16PM -0700, José Roberto de Souza wrote:
> skl_compute_ddb() will for a modeset in all pipes when MBUS joining
> changes between states, so all pipes will be disabled, have all
> MBUS related registers updated and then each pipe enabled.
> So no vblank syncronization is necessary and here droping those TODOs.

The TODO is about removing that modeset. So it should stay put.

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index cf290bb704221..9ccf0f062862c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6066,7 +6066,6 @@ skl_compute_ddb(struct intel_atomic_state *state)
>  			return ret;
>  
>  		if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
> -			/* TODO: Implement vblank synchronized MBUS joining changes */
>  			ret = intel_modeset_all_pipes(state);
>  			if (ret)
>  				return ret;
> @@ -8195,10 +8194,6 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>  	if (!HAS_MBUS_JOINING(dev_priv))
>  		return;
>  
> -	/*
> -	 * TODO: Implement vblank synchronized MBUS joining changes.
> -	 * Must be properly coordinated with dbuf reprogramming.
> -	 */
>  	if (dbuf_state->joined_mbus) {
>  		mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
>  			MBUS_JOIN_PIPE_SELECT_NONE;
> -- 
> 2.35.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cf290bb704221..9ccf0f062862c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6066,7 +6066,6 @@  skl_compute_ddb(struct intel_atomic_state *state)
 			return ret;
 
 		if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
-			/* TODO: Implement vblank synchronized MBUS joining changes */
 			ret = intel_modeset_all_pipes(state);
 			if (ret)
 				return ret;
@@ -8195,10 +8194,6 @@  static void update_mbus_pre_enable(struct intel_atomic_state *state)
 	if (!HAS_MBUS_JOINING(dev_priv))
 		return;
 
-	/*
-	 * TODO: Implement vblank synchronized MBUS joining changes.
-	 * Must be properly coordinated with dbuf reprogramming.
-	 */
 	if (dbuf_state->joined_mbus) {
 		mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
 			MBUS_JOIN_PIPE_SELECT_NONE;