Re: [Intel-gfx] [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers

2020-03-13 Thread Ville Syrjälä
On Fri, Mar 13, 2020 at 01:57:58PM +, Lisovskiy, Stanislav wrote:
> >> >> Add correspondent helpers to be able to get old/new bandwidth
> >> >> global state object.
> >> >>
> >> >> v2: - Fixed typo in function call
> >> >> v3: - Changed new functions naming to use convention proposed
> >> >>   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.
> >>
> >> >Still nak on the rename.
> >>
> >> Cool. Discuss it with Jani Nikula then, to have at least some common 
> >> strategy on how to be picky on me.
> 
> >The strategy is either rename all of these functions or none so that we
> >don't end up with random inconsistencies all over the place.
> 
> 
> Initially Jani Nikula wrote that he is trying to encourage people to call 
> functions
> 
> starting with the module name. OK. Done it.
> 
> 
> You say that the opposite and nack. Now it just turns out that it is again 
> _me_ - poor minded, who didn't understand that I need to rename
> 
> all functions now here as well, including those completely unrelated to that 
> patch.
> 
> Sure - we have "plenty" of time!
> 
> 
> Or may be I shouldn't rename - kind of confused now.

If you do a mass rename do it as a separate series.

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers

2020-03-13 Thread Lisovskiy, Stanislav
>> >> Add correspondent helpers to be able to get old/new bandwidth
>> >> global state object.
>> >>
>> >> v2: - Fixed typo in function call
>> >> v3: - Changed new functions naming to use convention proposed
>> >>   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.
>>
>> >Still nak on the rename.
>>
>> Cool. Discuss it with Jani Nikula then, to have at least some common 
>> strategy on how to be picky on me.

>The strategy is either rename all of these functions or none so that we
>don't end up with random inconsistencies all over the place.


Initially Jani Nikula wrote that he is trying to encourage people to call 
functions

starting with the module name. OK. Done it.


You say that the opposite and nack. Now it just turns out that it is again _me_ 
- poor minded, who didn't understand that I need to rename

all functions now here as well, including those completely unrelated to that 
patch.

Sure - we have "plenty" of time!


Or may be I shouldn't rename - kind of confused now.


Best Regards,

Lisovskiy Stanislav

From: Ville Syrjälä 
Sent: Friday, March 13, 2020 3:26:11 PM
To: Lisovskiy, Stanislav
Cc: intel-gfx@lists.freedesktop.org; Ausmus, James; Saarinen, Jani; Roper, 
Matthew D
Subject: Re: [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers

On Fri, Mar 13, 2020 at 08:49:30AM +, Lisovskiy, Stanislav wrote:
> >> Add correspondent helpers to be able to get old/new bandwidth
> >> global state object.
> >>
> >> v2: - Fixed typo in function call
> >> v3: - Changed new functions naming to use convention proposed
> >>   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.
>
> >Still nak on the rename.
>
> Cool. Discuss it with Jani Nikula then, to have at least some common strategy 
> on how to be picky on me.

The strategy is either rename all of these functions or none so that we
don't end up with random inconsistencies all over the place.

>
> Best Regards,
>
> Lisovskiy Stanislav
> 
> From: Ville Syrjälä 
> Sent: Wednesday, March 11, 2020 6:08:54 PM
> To: Lisovskiy, Stanislav
> Cc: intel-gfx@lists.freedesktop.org; Ausmus, James; Saarinen, Jani; Roper, 
> Matthew D
> Subject: Re: [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers
>
> On Mon, Mar 09, 2020 at 06:11:59PM +0200, Stanislav Lisovskiy wrote:
> > Add correspondent helpers to be able to get old/new bandwidth
> > global state object.
> >
> > v2: - Fixed typo in function call
> > v3: - Changed new functions naming to use convention proposed
> >   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.
>
> Still nak on the rename.
>
> >
> > Signed-off-by: Stanislav Lisovskiy 
> > ---
> >  drivers/gpu/drm/i915/display/intel_bw.c | 33 ++---
> >  drivers/gpu/drm/i915/display/intel_bw.h |  9 +++
> >  2 files changed, 39 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
> > b/drivers/gpu/drm/i915/display/intel_bw.c
> > index 58b264bc318d..bdad7476dc7b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -374,8 +374,35 @@ static unsigned int intel_bw_data_rate(struct 
> > drm_i915_private *dev_priv,
> >return data_rate;
> >  }
> >
> > -static struct intel_bw_state *
> > -intel_atomic_get_bw_state(struct intel_atomic_state *state)
> > +struct intel_bw_state *
> > +intel_bw_get_old_state(struct intel_atomic_state *state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > + struct intel_global_state *bw_state;
> > +
> > + bw_state = intel_atomic_get_old_global_obj_state(state, 
> > _priv->bw_obj);
> > + if (IS_ERR(bw_state))
> > + return ERR_CAST(bw_state);
> > +
> > + return to_intel_bw_state(bw_state);
> > +}
> > +
> > +struct intel_bw_state *
> > +intel_bw_get_new_state(struct intel_atomic_state *state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > + struct intel_global_state *bw_state;
> > +
> > + bw_state = intel_atomic_get_new_global_obj_state(state, 
> > _priv->bw_obj);
> > +
> > + if (IS_ERR(bw_state))
> > + return ERR_CAST(bw_state);
> > +
> > + return to_intel_bw_state(bw_state);
> > +}
> > +
> > +struct intel_bw_state *
> > +intel_bw_get_state(struct intel_atomic_state *state)
> >  {
> >struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> >struct intel_global_state *bw_state;
> > @@ -420,7 +447,7 @@ int intel_bw_atomic_check(struct intel_atomic_state 
> > *state)
> >old_active_planes == new_active_planes)
> >continue;
> >
> > - bw_state  = intel_atomic_get_bw_state(state);
> > + bw_state  = intel_bw_get_state(state);
> >if (IS_ERR(bw_state))
> >return PTR_ERR(bw_state);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.h 
> > 

Re: [Intel-gfx] [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers

2020-03-13 Thread Ville Syrjälä
On Fri, Mar 13, 2020 at 08:49:30AM +, Lisovskiy, Stanislav wrote:
> >> Add correspondent helpers to be able to get old/new bandwidth
> >> global state object.
> >>
> >> v2: - Fixed typo in function call
> >> v3: - Changed new functions naming to use convention proposed
> >>   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.
> 
> >Still nak on the rename.
> 
> Cool. Discuss it with Jani Nikula then, to have at least some common strategy 
> on how to be picky on me.

The strategy is either rename all of these functions or none so that we
don't end up with random inconsistencies all over the place.

> 
> Best Regards,
> 
> Lisovskiy Stanislav
> 
> From: Ville Syrjälä 
> Sent: Wednesday, March 11, 2020 6:08:54 PM
> To: Lisovskiy, Stanislav
> Cc: intel-gfx@lists.freedesktop.org; Ausmus, James; Saarinen, Jani; Roper, 
> Matthew D
> Subject: Re: [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers
> 
> On Mon, Mar 09, 2020 at 06:11:59PM +0200, Stanislav Lisovskiy wrote:
> > Add correspondent helpers to be able to get old/new bandwidth
> > global state object.
> >
> > v2: - Fixed typo in function call
> > v3: - Changed new functions naming to use convention proposed
> >   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.
> 
> Still nak on the rename.
> 
> >
> > Signed-off-by: Stanislav Lisovskiy 
> > ---
> >  drivers/gpu/drm/i915/display/intel_bw.c | 33 ++---
> >  drivers/gpu/drm/i915/display/intel_bw.h |  9 +++
> >  2 files changed, 39 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
> > b/drivers/gpu/drm/i915/display/intel_bw.c
> > index 58b264bc318d..bdad7476dc7b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -374,8 +374,35 @@ static unsigned int intel_bw_data_rate(struct 
> > drm_i915_private *dev_priv,
> >return data_rate;
> >  }
> >
> > -static struct intel_bw_state *
> > -intel_atomic_get_bw_state(struct intel_atomic_state *state)
> > +struct intel_bw_state *
> > +intel_bw_get_old_state(struct intel_atomic_state *state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > + struct intel_global_state *bw_state;
> > +
> > + bw_state = intel_atomic_get_old_global_obj_state(state, 
> > _priv->bw_obj);
> > + if (IS_ERR(bw_state))
> > + return ERR_CAST(bw_state);
> > +
> > + return to_intel_bw_state(bw_state);
> > +}
> > +
> > +struct intel_bw_state *
> > +intel_bw_get_new_state(struct intel_atomic_state *state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > + struct intel_global_state *bw_state;
> > +
> > + bw_state = intel_atomic_get_new_global_obj_state(state, 
> > _priv->bw_obj);
> > +
> > + if (IS_ERR(bw_state))
> > + return ERR_CAST(bw_state);
> > +
> > + return to_intel_bw_state(bw_state);
> > +}
> > +
> > +struct intel_bw_state *
> > +intel_bw_get_state(struct intel_atomic_state *state)
> >  {
> >struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> >struct intel_global_state *bw_state;
> > @@ -420,7 +447,7 @@ int intel_bw_atomic_check(struct intel_atomic_state 
> > *state)
> >old_active_planes == new_active_planes)
> >continue;
> >
> > - bw_state  = intel_atomic_get_bw_state(state);
> > + bw_state  = intel_bw_get_state(state);
> >if (IS_ERR(bw_state))
> >return PTR_ERR(bw_state);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.h 
> > b/drivers/gpu/drm/i915/display/intel_bw.h
> > index a8aa7624c5aa..b5f61463922f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.h
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> > @@ -24,6 +24,15 @@ struct intel_bw_state {
> >
> >  #define to_intel_bw_state(x) container_of((x), struct intel_bw_state, base)
> >
> > +struct intel_bw_state *
> > +intel_bw_get_old_state(struct intel_atomic_state *state);
> > +
> > +struct intel_bw_state *
> > +intel_bw_get_new_state(struct intel_atomic_state *state);
> > +
> > +struct intel_bw_state *
> > +intel_bw_get_state(struct intel_atomic_state *state);
> > +
> >  void intel_bw_init_hw(struct drm_i915_private *dev_priv);
> >  int intel_bw_init(struct drm_i915_private *dev_priv);
> >  int intel_bw_atomic_check(struct intel_atomic_state *state);
> > --
> > 2.24.1.485.gad05a3d8e5
> 
> --
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers

2020-03-13 Thread Lisovskiy, Stanislav
>> Add correspondent helpers to be able to get old/new bandwidth
>> global state object.
>>
>> v2: - Fixed typo in function call
>> v3: - Changed new functions naming to use convention proposed
>>   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.

>Still nak on the rename.

Cool. Discuss it with Jani Nikula then, to have at least some common strategy 
on how to be picky on me.

Best Regards,

Lisovskiy Stanislav

From: Ville Syrjälä 
Sent: Wednesday, March 11, 2020 6:08:54 PM
To: Lisovskiy, Stanislav
Cc: intel-gfx@lists.freedesktop.org; Ausmus, James; Saarinen, Jani; Roper, 
Matthew D
Subject: Re: [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers

On Mon, Mar 09, 2020 at 06:11:59PM +0200, Stanislav Lisovskiy wrote:
> Add correspondent helpers to be able to get old/new bandwidth
> global state object.
>
> v2: - Fixed typo in function call
> v3: - Changed new functions naming to use convention proposed
>   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.

Still nak on the rename.

>
> Signed-off-by: Stanislav Lisovskiy 
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 33 ++---
>  drivers/gpu/drm/i915/display/intel_bw.h |  9 +++
>  2 files changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
> b/drivers/gpu/drm/i915/display/intel_bw.c
> index 58b264bc318d..bdad7476dc7b 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -374,8 +374,35 @@ static unsigned int intel_bw_data_rate(struct 
> drm_i915_private *dev_priv,
>return data_rate;
>  }
>
> -static struct intel_bw_state *
> -intel_atomic_get_bw_state(struct intel_atomic_state *state)
> +struct intel_bw_state *
> +intel_bw_get_old_state(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_global_state *bw_state;
> +
> + bw_state = intel_atomic_get_old_global_obj_state(state, 
> _priv->bw_obj);
> + if (IS_ERR(bw_state))
> + return ERR_CAST(bw_state);
> +
> + return to_intel_bw_state(bw_state);
> +}
> +
> +struct intel_bw_state *
> +intel_bw_get_new_state(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_global_state *bw_state;
> +
> + bw_state = intel_atomic_get_new_global_obj_state(state, 
> _priv->bw_obj);
> +
> + if (IS_ERR(bw_state))
> + return ERR_CAST(bw_state);
> +
> + return to_intel_bw_state(bw_state);
> +}
> +
> +struct intel_bw_state *
> +intel_bw_get_state(struct intel_atomic_state *state)
>  {
>struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>struct intel_global_state *bw_state;
> @@ -420,7 +447,7 @@ int intel_bw_atomic_check(struct intel_atomic_state 
> *state)
>old_active_planes == new_active_planes)
>continue;
>
> - bw_state  = intel_atomic_get_bw_state(state);
> + bw_state  = intel_bw_get_state(state);
>if (IS_ERR(bw_state))
>return PTR_ERR(bw_state);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h 
> b/drivers/gpu/drm/i915/display/intel_bw.h
> index a8aa7624c5aa..b5f61463922f 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -24,6 +24,15 @@ struct intel_bw_state {
>
>  #define to_intel_bw_state(x) container_of((x), struct intel_bw_state, base)
>
> +struct intel_bw_state *
> +intel_bw_get_old_state(struct intel_atomic_state *state);
> +
> +struct intel_bw_state *
> +intel_bw_get_new_state(struct intel_atomic_state *state);
> +
> +struct intel_bw_state *
> +intel_bw_get_state(struct intel_atomic_state *state);
> +
>  void intel_bw_init_hw(struct drm_i915_private *dev_priv);
>  int intel_bw_init(struct drm_i915_private *dev_priv);
>  int intel_bw_atomic_check(struct intel_atomic_state *state);
> --
> 2.24.1.485.gad05a3d8e5

--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v19 3/8] drm/i915: Add intel_bw_get_*_state helpers

2020-03-11 Thread Ville Syrjälä
On Mon, Mar 09, 2020 at 06:11:59PM +0200, Stanislav Lisovskiy wrote:
> Add correspondent helpers to be able to get old/new bandwidth
> global state object.
> 
> v2: - Fixed typo in function call
> v3: - Changed new functions naming to use convention proposed
>   by Jani Nikula, i.e intel_bw_* in intel_bw.c file.

Still nak on the rename.

> 
> Signed-off-by: Stanislav Lisovskiy 
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 33 ++---
>  drivers/gpu/drm/i915/display/intel_bw.h |  9 +++
>  2 files changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
> b/drivers/gpu/drm/i915/display/intel_bw.c
> index 58b264bc318d..bdad7476dc7b 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -374,8 +374,35 @@ static unsigned int intel_bw_data_rate(struct 
> drm_i915_private *dev_priv,
>   return data_rate;
>  }
>  
> -static struct intel_bw_state *
> -intel_atomic_get_bw_state(struct intel_atomic_state *state)
> +struct intel_bw_state *
> +intel_bw_get_old_state(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_global_state *bw_state;
> +
> + bw_state = intel_atomic_get_old_global_obj_state(state, 
> _priv->bw_obj);
> + if (IS_ERR(bw_state))
> + return ERR_CAST(bw_state);
> +
> + return to_intel_bw_state(bw_state);
> +}
> +
> +struct intel_bw_state *
> +intel_bw_get_new_state(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_global_state *bw_state;
> +
> + bw_state = intel_atomic_get_new_global_obj_state(state, 
> _priv->bw_obj);
> +
> + if (IS_ERR(bw_state))
> + return ERR_CAST(bw_state);
> +
> + return to_intel_bw_state(bw_state);
> +}
> +
> +struct intel_bw_state *
> +intel_bw_get_state(struct intel_atomic_state *state)
>  {
>   struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>   struct intel_global_state *bw_state;
> @@ -420,7 +447,7 @@ int intel_bw_atomic_check(struct intel_atomic_state 
> *state)
>   old_active_planes == new_active_planes)
>   continue;
>  
> - bw_state  = intel_atomic_get_bw_state(state);
> + bw_state  = intel_bw_get_state(state);
>   if (IS_ERR(bw_state))
>   return PTR_ERR(bw_state);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h 
> b/drivers/gpu/drm/i915/display/intel_bw.h
> index a8aa7624c5aa..b5f61463922f 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -24,6 +24,15 @@ struct intel_bw_state {
>  
>  #define to_intel_bw_state(x) container_of((x), struct intel_bw_state, base)
>  
> +struct intel_bw_state *
> +intel_bw_get_old_state(struct intel_atomic_state *state);
> +
> +struct intel_bw_state *
> +intel_bw_get_new_state(struct intel_atomic_state *state);
> +
> +struct intel_bw_state *
> +intel_bw_get_state(struct intel_atomic_state *state);
> +
>  void intel_bw_init_hw(struct drm_i915_private *dev_priv);
>  int intel_bw_init(struct drm_i915_private *dev_priv);
>  int intel_bw_atomic_check(struct intel_atomic_state *state);
> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx