linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config
@ 2019-09-09 13:52 Benjamin Gaignard
  2019-09-09 13:52 ` [PATCH] drm: atomic helper: fix W=1 warnings Benjamin Gaignard
  2019-09-10 12:58 ` [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config Harry Wentland
  0 siblings, 2 replies; 16+ messages in thread
From: Benjamin Gaignard @ 2019-09-09 13:52 UTC (permalink / raw)
  To: benjamin.gaignard, airlied, daniel
  Cc: dri-devel, linux-kernel, Benjamin Gaignard

Change scale_increment_interval and nfl_bpg_offset fields to
u32 to avoid W=1 warnings because we are testing them against
65535.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 include/drm/drm_dsc.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
index 887954cbfc60..e495024e901c 100644
--- a/include/drm/drm_dsc.h
+++ b/include/drm/drm_dsc.h
@@ -207,11 +207,13 @@ struct drm_dsc_config {
 	 * Number of group times between incrementing the scale factor value
 	 * used at the beginning of a slice.
 	 */
-	u16 scale_increment_interval;
+	u32 scale_increment_interval;
+
 	/**
 	 * @nfl_bpg_offset: Non first line BPG offset to be used
 	 */
-	u16 nfl_bpg_offset;
+
+	u32 nfl_bpg_offset;
 	/**
 	 * @slice_bpg_offset: BPG offset used to enforce slice bit
 	 */
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] drm: atomic helper: fix W=1 warnings
  2019-09-09 13:52 [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config Benjamin Gaignard
@ 2019-09-09 13:52 ` Benjamin Gaignard
  2019-09-16 13:19   ` Benjamin Gaignard
  2019-10-03 14:27   ` Ville Syrjälä
  2019-09-10 12:58 ` [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config Harry Wentland
  1 sibling, 2 replies; 16+ messages in thread
From: Benjamin Gaignard @ 2019-09-09 13:52 UTC (permalink / raw)
  To: benjamin.gaignard, airlied, daniel
  Cc: dri-devel, linux-kernel, Benjamin Gaignard

Fix warnings with W=1.
Few for_each macro set variables that are never used later.
Prevent warning by marking these variables as __maybe_unused.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index aa16ea17ff9b..b69d17b0b9bd 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
 	      struct drm_encoder *encoder)
 {
 	struct drm_crtc_state *crtc_state;
-	struct drm_connector *connector;
+	struct drm_connector __maybe_unused *connector;
 	struct drm_connector_state *old_connector_state, *new_connector_state;
 	int i;
 
@@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *new_crtc_state;
-	struct drm_connector *connector;
+	struct drm_connector __maybe_unused *connector;
 	struct drm_connector_state *new_conn_state;
 	int i;
 	int ret;
@@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-	struct drm_connector *connector;
+	struct drm_connector __maybe_unused *connector;
 	struct drm_connector_state *old_connector_state, *new_connector_state;
 	int i, ret;
 	unsigned connectors_mask = 0;
@@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
 static void
 disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 {
-	struct drm_connector *connector;
+	struct drm_connector __maybe_unused *connector;
 	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
@@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *new_crtc_state;
-	struct drm_connector *connector;
+	struct drm_connector __maybe_unused *connector;
 	struct drm_connector_state *new_conn_state;
 	int i;
 
@@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc_state *new_crtc_state;
-	struct drm_connector *connector;
+	struct drm_connector __maybe_unused *connector;
 	struct drm_connector_state *new_conn_state;
 	int i;
 
@@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
 				      struct drm_atomic_state *state,
 				      bool pre_swap)
 {
-	struct drm_plane *plane;
+	struct drm_plane __maybe_unused *plane;
 	struct drm_plane_state *new_plane_state;
 	int i, ret;
 
@@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 		struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+	struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
 	int i, ret;
 	unsigned crtc_mask = 0;
 
@@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
 int drm_atomic_helper_async_check(struct drm_device *dev,
 				   struct drm_atomic_state *state)
 {
-	struct drm_crtc *crtc;
+	struct drm_crtc __maybe_unused *crtc;
 	struct drm_crtc_state *crtc_state;
 	struct drm_plane *plane = NULL;
 	struct drm_plane_state *old_plane_state = NULL;
@@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-	struct drm_connector *conn;
+	struct drm_connector __maybe_unused *conn;
 	struct drm_connector_state *old_conn_state, *new_conn_state;
-	struct drm_plane *plane;
+	struct drm_plane __maybe_unused *plane;
 	struct drm_plane_state *old_plane_state, *new_plane_state;
 	struct drm_crtc_commit *commit;
 	int i, ret;
@@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
  */
 void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
 {
-	struct drm_crtc *crtc;
+	struct drm_crtc __maybe_unused *crtc;
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
@@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state)
 {
-	struct drm_connector *connector;
+	struct drm_connector __maybe_unused *connector;
 	struct drm_connector_state *new_conn_state;
 	struct drm_plane *plane;
 	struct drm_plane_state *new_plane_state;
@@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
 {
 	struct drm_atomic_state *state;
 	struct drm_connector_state *conn_state;
-	struct drm_connector *conn;
+	struct drm_connector __maybe_unused *conn;
 	struct drm_plane_state *plane_state;
-	struct drm_plane *plane;
+	struct drm_plane __maybe_unused *plane;
 	struct drm_crtc_state *crtc_state;
 	struct drm_crtc *crtc;
 	int ret, i;
@@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
 {
 	int i, ret;
 	struct drm_plane *plane;
-	struct drm_plane_state *new_plane_state;
+	struct drm_plane_state __maybe_unused *new_plane_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *new_conn_state;
+	struct drm_connector_state __maybe_unused *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *new_crtc_state;
+	struct drm_crtc_state __maybe_unused *new_crtc_state;
 
 	state->acquire_ctx = ctx;
 
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config
  2019-09-09 13:52 [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config Benjamin Gaignard
  2019-09-09 13:52 ` [PATCH] drm: atomic helper: fix W=1 warnings Benjamin Gaignard
@ 2019-09-10 12:58 ` Harry Wentland
  2019-09-10 17:58   ` Manasi Navare
  1 sibling, 1 reply; 16+ messages in thread
From: Harry Wentland @ 2019-09-10 12:58 UTC (permalink / raw)
  To: Benjamin Gaignard, benjamin.gaignard, airlied, daniel
  Cc: linux-kernel, dri-devel, Navare, Manasi D, Gaurav K Singh

+Manasi, Gaurav

On 2019-09-09 9:52 a.m., Benjamin Gaignard wrote:
> Change scale_increment_interval and nfl_bpg_offset fields to
> u32 to avoid W=1 warnings because we are testing them against
> 65535.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>   include/drm/drm_dsc.h | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> index 887954cbfc60..e495024e901c 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -207,11 +207,13 @@ struct drm_dsc_config {
>   	 * Number of group times between incrementing the scale factor value
>   	 * used at the beginning of a slice.
>   	 */
> -	u16 scale_increment_interval;
> +	u32 scale_increment_interval;

The DSC spec defines both as u16. I think the check in drm_dsc.c is 
useless and should be dropped.

Harry

> +
>   	/**
>   	 * @nfl_bpg_offset: Non first line BPG offset to be used
>   	 */
> -	u16 nfl_bpg_offset;
> +
> +	u32 nfl_bpg_offset;
>   	/**
>   	 * @slice_bpg_offset: BPG offset used to enforce slice bit
>   	 */
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config
  2019-09-10 12:58 ` [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config Harry Wentland
@ 2019-09-10 17:58   ` Manasi Navare
  0 siblings, 0 replies; 16+ messages in thread
From: Manasi Navare @ 2019-09-10 17:58 UTC (permalink / raw)
  To: Harry Wentland
  Cc: Benjamin Gaignard, benjamin.gaignard, airlied, daniel,
	linux-kernel, dri-devel, Gaurav K Singh

On Tue, Sep 10, 2019 at 12:58:24PM +0000, Harry Wentland wrote:
> +Manasi, Gaurav
> 
> On 2019-09-09 9:52 a.m., Benjamin Gaignard wrote:
> > Change scale_increment_interval and nfl_bpg_offset fields to
> > u32 to avoid W=1 warnings because we are testing them against
> > 65535.
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > ---
> >   include/drm/drm_dsc.h | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> > index 887954cbfc60..e495024e901c 100644
> > --- a/include/drm/drm_dsc.h
> > +++ b/include/drm/drm_dsc.h
> > @@ -207,11 +207,13 @@ struct drm_dsc_config {
> >   	 * Number of group times between incrementing the scale factor value
> >   	 * used at the beginning of a slice.
> >   	 */
> > -	u16 scale_increment_interval;
> > +	u32 scale_increment_interval;
> 
> The DSC spec defines both as u16. I think the check in drm_dsc.c is 
> useless and should be dropped.
>

I agree with Harry here, all these variables should match the number of bits
in the spec, increasing them to u32 allows more values which violates the
DSC spec.

It should stay u16

Manasi
 
> Harry
> 
> > +
> >   	/**
> >   	 * @nfl_bpg_offset: Non first line BPG offset to be used
> >   	 */
> > -	u16 nfl_bpg_offset;
> > +
> > +	u32 nfl_bpg_offset;
> >   	/**
> >   	 * @slice_bpg_offset: BPG offset used to enforce slice bit
> >   	 */
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-09-09 13:52 ` [PATCH] drm: atomic helper: fix W=1 warnings Benjamin Gaignard
@ 2019-09-16 13:19   ` Benjamin Gaignard
  2019-10-03  9:52     ` Benjamin Gaignard
  2019-10-03 14:27   ` Ville Syrjälä
  1 sibling, 1 reply; 16+ messages in thread
From: Benjamin Gaignard @ 2019-09-16 13:19 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: David Airlie, Daniel Vetter, ML dri-devel, Linux Kernel Mailing List

Le lun. 9 sept. 2019 à 16:41, Benjamin Gaignard
<benjamin.gaignard@st.com> a écrit :
>
> Fix warnings with W=1.
> Few for_each macro set variables that are never used later.
> Prevent warning by marking these variables as __maybe_unused.
>

A little up on this one because it may exist others ways to fix these warnings.
Get feedback on this path could give the direction for similar ones in drm.

Thanks,
Benjamin

> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index aa16ea17ff9b..b69d17b0b9bd 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
>               struct drm_encoder *encoder)
>  {
>         struct drm_crtc_state *crtc_state;
> -       struct drm_connector *connector;
> +       struct drm_connector __maybe_unused *connector;
>         struct drm_connector_state *old_connector_state, *new_connector_state;
>         int i;
>
> @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
>  {
>         struct drm_crtc *crtc;
>         struct drm_crtc_state *new_crtc_state;
> -       struct drm_connector *connector;
> +       struct drm_connector __maybe_unused *connector;
>         struct drm_connector_state *new_conn_state;
>         int i;
>         int ret;
> @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  {
>         struct drm_crtc *crtc;
>         struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> -       struct drm_connector *connector;
> +       struct drm_connector __maybe_unused *connector;
>         struct drm_connector_state *old_connector_state, *new_connector_state;
>         int i, ret;
>         unsigned connectors_mask = 0;
> @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
>  static void
>  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  {
> -       struct drm_connector *connector;
> +       struct drm_connector __maybe_unused *connector;
>         struct drm_connector_state *old_conn_state, *new_conn_state;
>         struct drm_crtc *crtc;
>         struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
>  {
>         struct drm_crtc *crtc;
>         struct drm_crtc_state *new_crtc_state;
> -       struct drm_connector *connector;
> +       struct drm_connector __maybe_unused *connector;
>         struct drm_connector_state *new_conn_state;
>         int i;
>
> @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>         struct drm_crtc *crtc;
>         struct drm_crtc_state *old_crtc_state;
>         struct drm_crtc_state *new_crtc_state;
> -       struct drm_connector *connector;
> +       struct drm_connector __maybe_unused *connector;
>         struct drm_connector_state *new_conn_state;
>         int i;
>
> @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
>                                       struct drm_atomic_state *state,
>                                       bool pre_swap)
>  {
> -       struct drm_plane *plane;
> +       struct drm_plane __maybe_unused *plane;
>         struct drm_plane_state *new_plane_state;
>         int i, ret;
>
> @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>                 struct drm_atomic_state *old_state)
>  {
>         struct drm_crtc *crtc;
> -       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> +       struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
>         int i, ret;
>         unsigned crtc_mask = 0;
>
> @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
>  int drm_atomic_helper_async_check(struct drm_device *dev,
>                                    struct drm_atomic_state *state)
>  {
> -       struct drm_crtc *crtc;
> +       struct drm_crtc __maybe_unused *crtc;
>         struct drm_crtc_state *crtc_state;
>         struct drm_plane *plane = NULL;
>         struct drm_plane_state *old_plane_state = NULL;
> @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
>  {
>         struct drm_crtc *crtc;
>         struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> -       struct drm_connector *conn;
> +       struct drm_connector __maybe_unused *conn;
>         struct drm_connector_state *old_conn_state, *new_conn_state;
> -       struct drm_plane *plane;
> +       struct drm_plane __maybe_unused *plane;
>         struct drm_plane_state *old_plane_state, *new_plane_state;
>         struct drm_crtc_commit *commit;
>         int i, ret;
> @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
>   */
>  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
>  {
> -       struct drm_crtc *crtc;
> +       struct drm_crtc __maybe_unused *crtc;
>         struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>         struct drm_crtc_commit *commit;
>         int i;
> @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
>  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>                                      struct drm_atomic_state *state)
>  {
> -       struct drm_connector *connector;
> +       struct drm_connector __maybe_unused *connector;
>         struct drm_connector_state *new_conn_state;
>         struct drm_plane *plane;
>         struct drm_plane_state *new_plane_state;
> @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
>  {
>         struct drm_atomic_state *state;
>         struct drm_connector_state *conn_state;
> -       struct drm_connector *conn;
> +       struct drm_connector __maybe_unused *conn;
>         struct drm_plane_state *plane_state;
> -       struct drm_plane *plane;
> +       struct drm_plane __maybe_unused *plane;
>         struct drm_crtc_state *crtc_state;
>         struct drm_crtc *crtc;
>         int ret, i;
> @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
>  {
>         int i, ret;
>         struct drm_plane *plane;
> -       struct drm_plane_state *new_plane_state;
> +       struct drm_plane_state __maybe_unused *new_plane_state;
>         struct drm_connector *connector;
> -       struct drm_connector_state *new_conn_state;
> +       struct drm_connector_state __maybe_unused *new_conn_state;
>         struct drm_crtc *crtc;
> -       struct drm_crtc_state *new_crtc_state;
> +       struct drm_crtc_state __maybe_unused *new_crtc_state;
>
>         state->acquire_ctx = ctx;
>
> --
> 2.15.0
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-09-16 13:19   ` Benjamin Gaignard
@ 2019-10-03  9:52     ` Benjamin Gaignard
  0 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gaignard @ 2019-10-03  9:52 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: David Airlie, Daniel Vetter, ML dri-devel, Linux Kernel Mailing List

Le lun. 16 sept. 2019 à 15:19, Benjamin Gaignard
<benjamin.gaignard@linaro.org> a écrit :
>
> Le lun. 9 sept. 2019 à 16:41, Benjamin Gaignard
> <benjamin.gaignard@st.com> a écrit :
> >
> > Fix warnings with W=1.
> > Few for_each macro set variables that are never used later.
> > Prevent warning by marking these variables as __maybe_unused.
> >
>

Gentle Ping

benjamin

> A little up on this one because it may exist others ways to fix these warnings.
> Get feedback on this path could give the direction for similar ones in drm.
>
> Thanks,
> Benjamin
>
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> >  1 file changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index aa16ea17ff9b..b69d17b0b9bd 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> >               struct drm_encoder *encoder)
> >  {
> >         struct drm_crtc_state *crtc_state;
> > -       struct drm_connector *connector;
> > +       struct drm_connector __maybe_unused *connector;
> >         struct drm_connector_state *old_connector_state, *new_connector_state;
> >         int i;
> >
> > @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
> >  {
> >         struct drm_crtc *crtc;
> >         struct drm_crtc_state *new_crtc_state;
> > -       struct drm_connector *connector;
> > +       struct drm_connector __maybe_unused *connector;
> >         struct drm_connector_state *new_conn_state;
> >         int i;
> >         int ret;
> > @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> >  {
> >         struct drm_crtc *crtc;
> >         struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > -       struct drm_connector *connector;
> > +       struct drm_connector __maybe_unused *connector;
> >         struct drm_connector_state *old_connector_state, *new_connector_state;
> >         int i, ret;
> >         unsigned connectors_mask = 0;
> > @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
> >  static void
> >  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> >  {
> > -       struct drm_connector *connector;
> > +       struct drm_connector __maybe_unused *connector;
> >         struct drm_connector_state *old_conn_state, *new_conn_state;
> >         struct drm_crtc *crtc;
> >         struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
> >  {
> >         struct drm_crtc *crtc;
> >         struct drm_crtc_state *new_crtc_state;
> > -       struct drm_connector *connector;
> > +       struct drm_connector __maybe_unused *connector;
> >         struct drm_connector_state *new_conn_state;
> >         int i;
> >
> > @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> >         struct drm_crtc *crtc;
> >         struct drm_crtc_state *old_crtc_state;
> >         struct drm_crtc_state *new_crtc_state;
> > -       struct drm_connector *connector;
> > +       struct drm_connector __maybe_unused *connector;
> >         struct drm_connector_state *new_conn_state;
> >         int i;
> >
> > @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> >                                       struct drm_atomic_state *state,
> >                                       bool pre_swap)
> >  {
> > -       struct drm_plane *plane;
> > +       struct drm_plane __maybe_unused *plane;
> >         struct drm_plane_state *new_plane_state;
> >         int i, ret;
> >
> > @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
> >                 struct drm_atomic_state *old_state)
> >  {
> >         struct drm_crtc *crtc;
> > -       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > +       struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
> >         int i, ret;
> >         unsigned crtc_mask = 0;
> >
> > @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
> >  int drm_atomic_helper_async_check(struct drm_device *dev,
> >                                    struct drm_atomic_state *state)
> >  {
> > -       struct drm_crtc *crtc;
> > +       struct drm_crtc __maybe_unused *crtc;
> >         struct drm_crtc_state *crtc_state;
> >         struct drm_plane *plane = NULL;
> >         struct drm_plane_state *old_plane_state = NULL;
> > @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> >  {
> >         struct drm_crtc *crtc;
> >         struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > -       struct drm_connector *conn;
> > +       struct drm_connector __maybe_unused *conn;
> >         struct drm_connector_state *old_conn_state, *new_conn_state;
> > -       struct drm_plane *plane;
> > +       struct drm_plane __maybe_unused *plane;
> >         struct drm_plane_state *old_plane_state, *new_plane_state;
> >         struct drm_crtc_commit *commit;
> >         int i, ret;
> > @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
> >   */
> >  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
> >  {
> > -       struct drm_crtc *crtc;
> > +       struct drm_crtc __maybe_unused *crtc;
> >         struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> >         struct drm_crtc_commit *commit;
> >         int i;
> > @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
> >  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> >                                      struct drm_atomic_state *state)
> >  {
> > -       struct drm_connector *connector;
> > +       struct drm_connector __maybe_unused *connector;
> >         struct drm_connector_state *new_conn_state;
> >         struct drm_plane *plane;
> >         struct drm_plane_state *new_plane_state;
> > @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
> >  {
> >         struct drm_atomic_state *state;
> >         struct drm_connector_state *conn_state;
> > -       struct drm_connector *conn;
> > +       struct drm_connector __maybe_unused *conn;
> >         struct drm_plane_state *plane_state;
> > -       struct drm_plane *plane;
> > +       struct drm_plane __maybe_unused *plane;
> >         struct drm_crtc_state *crtc_state;
> >         struct drm_crtc *crtc;
> >         int ret, i;
> > @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> >  {
> >         int i, ret;
> >         struct drm_plane *plane;
> > -       struct drm_plane_state *new_plane_state;
> > +       struct drm_plane_state __maybe_unused *new_plane_state;
> >         struct drm_connector *connector;
> > -       struct drm_connector_state *new_conn_state;
> > +       struct drm_connector_state __maybe_unused *new_conn_state;
> >         struct drm_crtc *crtc;
> > -       struct drm_crtc_state *new_crtc_state;
> > +       struct drm_crtc_state __maybe_unused *new_crtc_state;
> >
> >         state->acquire_ctx = ctx;
> >
> > --
> > 2.15.0
> >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-09-09 13:52 ` [PATCH] drm: atomic helper: fix W=1 warnings Benjamin Gaignard
  2019-09-16 13:19   ` Benjamin Gaignard
@ 2019-10-03 14:27   ` Ville Syrjälä
  2019-10-03 14:46     ` Benjamin Gaignard
  1 sibling, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-10-03 14:27 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: benjamin.gaignard, airlied, daniel, linux-kernel, dri-devel

On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> Fix warnings with W=1.
> Few for_each macro set variables that are never used later.
> Prevent warning by marking these variables as __maybe_unused.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index aa16ea17ff9b..b69d17b0b9bd 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
>  	      struct drm_encoder *encoder)
>  {
>  	struct drm_crtc_state *crtc_state;
> -	struct drm_connector *connector;
> +	struct drm_connector __maybe_unused *connector;

Rather ugly. IMO would be nicer if we could hide something inside
the iterator macros to suppress the warning.

>  	struct drm_connector_state *old_connector_state, *new_connector_state;
>  	int i;
>  
> @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
>  {
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *new_crtc_state;
> -	struct drm_connector *connector;
> +	struct drm_connector __maybe_unused *connector;
>  	struct drm_connector_state *new_conn_state;
>  	int i;
>  	int ret;
> @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  {
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> -	struct drm_connector *connector;
> +	struct drm_connector __maybe_unused *connector;
>  	struct drm_connector_state *old_connector_state, *new_connector_state;
>  	int i, ret;
>  	unsigned connectors_mask = 0;
> @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
>  static void
>  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  {
> -	struct drm_connector *connector;
> +	struct drm_connector __maybe_unused *connector;
>  	struct drm_connector_state *old_conn_state, *new_conn_state;
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
>  {
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *new_crtc_state;
> -	struct drm_connector *connector;
> +	struct drm_connector __maybe_unused *connector;
>  	struct drm_connector_state *new_conn_state;
>  	int i;
>  
> @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *old_crtc_state;
>  	struct drm_crtc_state *new_crtc_state;
> -	struct drm_connector *connector;
> +	struct drm_connector __maybe_unused *connector;
>  	struct drm_connector_state *new_conn_state;
>  	int i;
>  
> @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
>  				      struct drm_atomic_state *state,
>  				      bool pre_swap)
>  {
> -	struct drm_plane *plane;
> +	struct drm_plane __maybe_unused *plane;
>  	struct drm_plane_state *new_plane_state;
>  	int i, ret;
>  
> @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>  		struct drm_atomic_state *old_state)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> +	struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
>  	int i, ret;
>  	unsigned crtc_mask = 0;
>  
> @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
>  int drm_atomic_helper_async_check(struct drm_device *dev,
>  				   struct drm_atomic_state *state)
>  {
> -	struct drm_crtc *crtc;
> +	struct drm_crtc __maybe_unused *crtc;
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_plane *plane = NULL;
>  	struct drm_plane_state *old_plane_state = NULL;
> @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
>  {
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> -	struct drm_connector *conn;
> +	struct drm_connector __maybe_unused *conn;
>  	struct drm_connector_state *old_conn_state, *new_conn_state;
> -	struct drm_plane *plane;
> +	struct drm_plane __maybe_unused *plane;
>  	struct drm_plane_state *old_plane_state, *new_plane_state;
>  	struct drm_crtc_commit *commit;
>  	int i, ret;
> @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
>   */
>  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
>  {
> -	struct drm_crtc *crtc;
> +	struct drm_crtc __maybe_unused *crtc;
>  	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct drm_crtc_commit *commit;
>  	int i;
> @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
>  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  				     struct drm_atomic_state *state)
>  {
> -	struct drm_connector *connector;
> +	struct drm_connector __maybe_unused *connector;
>  	struct drm_connector_state *new_conn_state;
>  	struct drm_plane *plane;
>  	struct drm_plane_state *new_plane_state;
> @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
>  {
>  	struct drm_atomic_state *state;
>  	struct drm_connector_state *conn_state;
> -	struct drm_connector *conn;
> +	struct drm_connector __maybe_unused *conn;
>  	struct drm_plane_state *plane_state;
> -	struct drm_plane *plane;
> +	struct drm_plane __maybe_unused *plane;
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_crtc *crtc;
>  	int ret, i;
> @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
>  {
>  	int i, ret;
>  	struct drm_plane *plane;
> -	struct drm_plane_state *new_plane_state;
> +	struct drm_plane_state __maybe_unused *new_plane_state;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *new_conn_state;
> +	struct drm_connector_state __maybe_unused *new_conn_state;
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *new_crtc_state;
> +	struct drm_crtc_state __maybe_unused *new_crtc_state;
>  
>  	state->acquire_ctx = ctx;
>  
> -- 
> 2.15.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-03 14:27   ` Ville Syrjälä
@ 2019-10-03 14:46     ` Benjamin Gaignard
  2019-10-03 15:05       ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Benjamin Gaignard @ 2019-10-03 14:46 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Benjamin Gaignard, David Airlie, Daniel Vetter,
	Linux Kernel Mailing List, ML dri-devel

Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
<ville.syrjala@linux.intel.com> a écrit :
>
> On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > Fix warnings with W=1.
> > Few for_each macro set variables that are never used later.
> > Prevent warning by marking these variables as __maybe_unused.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> >  1 file changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index aa16ea17ff9b..b69d17b0b9bd 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> >             struct drm_encoder *encoder)
> >  {
> >       struct drm_crtc_state *crtc_state;
> > -     struct drm_connector *connector;
> > +     struct drm_connector __maybe_unused *connector;
>
> Rather ugly. IMO would be nicer if we could hide something inside
> the iterator macros to suppress the warning.

Ok but how ?
connector is assigned in the macros but not used later and we can't
set "__maybe_unused"
in the macro.
Does another keyword exist for that ?

>
> >       struct drm_connector_state *old_connector_state, *new_connector_state;
> >       int i;
> >
> > @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
> >  {
> >       struct drm_crtc *crtc;
> >       struct drm_crtc_state *new_crtc_state;
> > -     struct drm_connector *connector;
> > +     struct drm_connector __maybe_unused *connector;
> >       struct drm_connector_state *new_conn_state;
> >       int i;
> >       int ret;
> > @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> >  {
> >       struct drm_crtc *crtc;
> >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > -     struct drm_connector *connector;
> > +     struct drm_connector __maybe_unused *connector;
> >       struct drm_connector_state *old_connector_state, *new_connector_state;
> >       int i, ret;
> >       unsigned connectors_mask = 0;
> > @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
> >  static void
> >  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> >  {
> > -     struct drm_connector *connector;
> > +     struct drm_connector __maybe_unused *connector;
> >       struct drm_connector_state *old_conn_state, *new_conn_state;
> >       struct drm_crtc *crtc;
> >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
> >  {
> >       struct drm_crtc *crtc;
> >       struct drm_crtc_state *new_crtc_state;
> > -     struct drm_connector *connector;
> > +     struct drm_connector __maybe_unused *connector;
> >       struct drm_connector_state *new_conn_state;
> >       int i;
> >
> > @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> >       struct drm_crtc *crtc;
> >       struct drm_crtc_state *old_crtc_state;
> >       struct drm_crtc_state *new_crtc_state;
> > -     struct drm_connector *connector;
> > +     struct drm_connector __maybe_unused *connector;
> >       struct drm_connector_state *new_conn_state;
> >       int i;
> >
> > @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> >                                     struct drm_atomic_state *state,
> >                                     bool pre_swap)
> >  {
> > -     struct drm_plane *plane;
> > +     struct drm_plane __maybe_unused *plane;
> >       struct drm_plane_state *new_plane_state;
> >       int i, ret;
> >
> > @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
> >               struct drm_atomic_state *old_state)
> >  {
> >       struct drm_crtc *crtc;
> > -     struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > +     struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
> >       int i, ret;
> >       unsigned crtc_mask = 0;
> >
> > @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
> >  int drm_atomic_helper_async_check(struct drm_device *dev,
> >                                  struct drm_atomic_state *state)
> >  {
> > -     struct drm_crtc *crtc;
> > +     struct drm_crtc __maybe_unused *crtc;
> >       struct drm_crtc_state *crtc_state;
> >       struct drm_plane *plane = NULL;
> >       struct drm_plane_state *old_plane_state = NULL;
> > @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> >  {
> >       struct drm_crtc *crtc;
> >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > -     struct drm_connector *conn;
> > +     struct drm_connector __maybe_unused *conn;
> >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > -     struct drm_plane *plane;
> > +     struct drm_plane __maybe_unused *plane;
> >       struct drm_plane_state *old_plane_state, *new_plane_state;
> >       struct drm_crtc_commit *commit;
> >       int i, ret;
> > @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
> >   */
> >  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
> >  {
> > -     struct drm_crtc *crtc;
> > +     struct drm_crtc __maybe_unused *crtc;
> >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> >       struct drm_crtc_commit *commit;
> >       int i;
> > @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
> >  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> >                                    struct drm_atomic_state *state)
> >  {
> > -     struct drm_connector *connector;
> > +     struct drm_connector __maybe_unused *connector;
> >       struct drm_connector_state *new_conn_state;
> >       struct drm_plane *plane;
> >       struct drm_plane_state *new_plane_state;
> > @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
> >  {
> >       struct drm_atomic_state *state;
> >       struct drm_connector_state *conn_state;
> > -     struct drm_connector *conn;
> > +     struct drm_connector __maybe_unused *conn;
> >       struct drm_plane_state *plane_state;
> > -     struct drm_plane *plane;
> > +     struct drm_plane __maybe_unused *plane;
> >       struct drm_crtc_state *crtc_state;
> >       struct drm_crtc *crtc;
> >       int ret, i;
> > @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> >  {
> >       int i, ret;
> >       struct drm_plane *plane;
> > -     struct drm_plane_state *new_plane_state;
> > +     struct drm_plane_state __maybe_unused *new_plane_state;
> >       struct drm_connector *connector;
> > -     struct drm_connector_state *new_conn_state;
> > +     struct drm_connector_state __maybe_unused *new_conn_state;
> >       struct drm_crtc *crtc;
> > -     struct drm_crtc_state *new_crtc_state;
> > +     struct drm_crtc_state __maybe_unused *new_crtc_state;
> >
> >       state->acquire_ctx = ctx;
> >
> > --
> > 2.15.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-03 14:46     ` Benjamin Gaignard
@ 2019-10-03 15:05       ` Ville Syrjälä
  2019-10-03 15:37         ` Benjamin Gaignard
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-10-03 15:05 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Benjamin Gaignard, David Airlie, Daniel Vetter,
	Linux Kernel Mailing List, ML dri-devel

On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> <ville.syrjala@linux.intel.com> a écrit :
> >
> > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > Fix warnings with W=1.
> > > Few for_each macro set variables that are never used later.
> > > Prevent warning by marking these variables as __maybe_unused.
> > >
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > >             struct drm_encoder *encoder)
> > >  {
> > >       struct drm_crtc_state *crtc_state;
> > > -     struct drm_connector *connector;
> > > +     struct drm_connector __maybe_unused *connector;
> >
> > Rather ugly. IMO would be nicer if we could hide something inside
> > the iterator macros to suppress the warning.
> 
> Ok but how ?
> connector is assigned in the macros but not used later and we can't
> set "__maybe_unused"
> in the macro.
> Does another keyword exist for that ?

Stick a (void)(connector) into the macro?

Another (arguably cleaner) idea would be to remove the connector/crtc/plane
argument from the iterators entirely since it's redundant, and instead just
extract it from the appropriate new/old state as needed.

We could then also add a for_each_connector_in_state()/etc. which omit
s the state arguments and just has the connector argument, for cases where
you don't care about the states when iterating.

> 
> >
> > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > >       int i;
> > >
> > > @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
> > >  {
> > >       struct drm_crtc *crtc;
> > >       struct drm_crtc_state *new_crtc_state;
> > > -     struct drm_connector *connector;
> > > +     struct drm_connector __maybe_unused *connector;
> > >       struct drm_connector_state *new_conn_state;
> > >       int i;
> > >       int ret;
> > > @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > >  {
> > >       struct drm_crtc *crtc;
> > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > -     struct drm_connector *connector;
> > > +     struct drm_connector __maybe_unused *connector;
> > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > >       int i, ret;
> > >       unsigned connectors_mask = 0;
> > > @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
> > >  static void
> > >  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> > >  {
> > > -     struct drm_connector *connector;
> > > +     struct drm_connector __maybe_unused *connector;
> > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > >       struct drm_crtc *crtc;
> > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
> > >  {
> > >       struct drm_crtc *crtc;
> > >       struct drm_crtc_state *new_crtc_state;
> > > -     struct drm_connector *connector;
> > > +     struct drm_connector __maybe_unused *connector;
> > >       struct drm_connector_state *new_conn_state;
> > >       int i;
> > >
> > > @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> > >       struct drm_crtc *crtc;
> > >       struct drm_crtc_state *old_crtc_state;
> > >       struct drm_crtc_state *new_crtc_state;
> > > -     struct drm_connector *connector;
> > > +     struct drm_connector __maybe_unused *connector;
> > >       struct drm_connector_state *new_conn_state;
> > >       int i;
> > >
> > > @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> > >                                     struct drm_atomic_state *state,
> > >                                     bool pre_swap)
> > >  {
> > > -     struct drm_plane *plane;
> > > +     struct drm_plane __maybe_unused *plane;
> > >       struct drm_plane_state *new_plane_state;
> > >       int i, ret;
> > >
> > > @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
> > >               struct drm_atomic_state *old_state)
> > >  {
> > >       struct drm_crtc *crtc;
> > > -     struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > +     struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
> > >       int i, ret;
> > >       unsigned crtc_mask = 0;
> > >
> > > @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
> > >  int drm_atomic_helper_async_check(struct drm_device *dev,
> > >                                  struct drm_atomic_state *state)
> > >  {
> > > -     struct drm_crtc *crtc;
> > > +     struct drm_crtc __maybe_unused *crtc;
> > >       struct drm_crtc_state *crtc_state;
> > >       struct drm_plane *plane = NULL;
> > >       struct drm_plane_state *old_plane_state = NULL;
> > > @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> > >  {
> > >       struct drm_crtc *crtc;
> > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > -     struct drm_connector *conn;
> > > +     struct drm_connector __maybe_unused *conn;
> > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > -     struct drm_plane *plane;
> > > +     struct drm_plane __maybe_unused *plane;
> > >       struct drm_plane_state *old_plane_state, *new_plane_state;
> > >       struct drm_crtc_commit *commit;
> > >       int i, ret;
> > > @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
> > >   */
> > >  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
> > >  {
> > > -     struct drm_crtc *crtc;
> > > +     struct drm_crtc __maybe_unused *crtc;
> > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > >       struct drm_crtc_commit *commit;
> > >       int i;
> > > @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
> > >  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> > >                                    struct drm_atomic_state *state)
> > >  {
> > > -     struct drm_connector *connector;
> > > +     struct drm_connector __maybe_unused *connector;
> > >       struct drm_connector_state *new_conn_state;
> > >       struct drm_plane *plane;
> > >       struct drm_plane_state *new_plane_state;
> > > @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
> > >  {
> > >       struct drm_atomic_state *state;
> > >       struct drm_connector_state *conn_state;
> > > -     struct drm_connector *conn;
> > > +     struct drm_connector __maybe_unused *conn;
> > >       struct drm_plane_state *plane_state;
> > > -     struct drm_plane *plane;
> > > +     struct drm_plane __maybe_unused *plane;
> > >       struct drm_crtc_state *crtc_state;
> > >       struct drm_crtc *crtc;
> > >       int ret, i;
> > > @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> > >  {
> > >       int i, ret;
> > >       struct drm_plane *plane;
> > > -     struct drm_plane_state *new_plane_state;
> > > +     struct drm_plane_state __maybe_unused *new_plane_state;
> > >       struct drm_connector *connector;
> > > -     struct drm_connector_state *new_conn_state;
> > > +     struct drm_connector_state __maybe_unused *new_conn_state;
> > >       struct drm_crtc *crtc;
> > > -     struct drm_crtc_state *new_crtc_state;
> > > +     struct drm_crtc_state __maybe_unused *new_crtc_state;
> > >
> > >       state->acquire_ctx = ctx;
> > >
> > > --
> > > 2.15.0
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-03 15:05       ` Ville Syrjälä
@ 2019-10-03 15:37         ` Benjamin Gaignard
  2019-10-03 15:46           ` Ville Syrjälä
  2019-10-08  9:55           ` Daniel Vetter
  0 siblings, 2 replies; 16+ messages in thread
From: Benjamin Gaignard @ 2019-10-03 15:37 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Benjamin Gaignard, David Airlie, Daniel Vetter,
	Linux Kernel Mailing List, ML dri-devel

Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
<ville.syrjala@linux.intel.com> a écrit :
>
> On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> > Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> a écrit :
> > >
> > > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > > Fix warnings with W=1.
> > > > Few for_each macro set variables that are never used later.
> > > > Prevent warning by marking these variables as __maybe_unused.
> > > >
> > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> > > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > > >             struct drm_encoder *encoder)
> > > >  {
> > > >       struct drm_crtc_state *crtc_state;
> > > > -     struct drm_connector *connector;
> > > > +     struct drm_connector __maybe_unused *connector;
> > >
> > > Rather ugly. IMO would be nicer if we could hide something inside
> > > the iterator macros to suppress the warning.
> >
> > Ok but how ?
> > connector is assigned in the macros but not used later and we can't
> > set "__maybe_unused"
> > in the macro.
> > Does another keyword exist for that ?
>
> Stick a (void)(connector) into the macro?

That could work but it will look strange inside the macro.

>
> Another (arguably cleaner) idea would be to remove the connector/crtc/plane
> argument from the iterators entirely since it's redundant, and instead just
> extract it from the appropriate new/old state as needed.
>
> We could then also add a for_each_connector_in_state()/etc. which omit
> s the state arguments and just has the connector argument, for cases where
> you don't care about the states when iterating.

That may lead to get a macro for each possible combination of used variables.

>
> >
> > >
> > > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > > >       int i;
> > > >
> > > > @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
> > > >  {
> > > >       struct drm_crtc *crtc;
> > > >       struct drm_crtc_state *new_crtc_state;
> > > > -     struct drm_connector *connector;
> > > > +     struct drm_connector __maybe_unused *connector;
> > > >       struct drm_connector_state *new_conn_state;
> > > >       int i;
> > > >       int ret;
> > > > @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > > >  {
> > > >       struct drm_crtc *crtc;
> > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > -     struct drm_connector *connector;
> > > > +     struct drm_connector __maybe_unused *connector;
> > > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > > >       int i, ret;
> > > >       unsigned connectors_mask = 0;
> > > > @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
> > > >  static void
> > > >  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> > > >  {
> > > > -     struct drm_connector *connector;
> > > > +     struct drm_connector __maybe_unused *connector;
> > > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > >       struct drm_crtc *crtc;
> > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
> > > >  {
> > > >       struct drm_crtc *crtc;
> > > >       struct drm_crtc_state *new_crtc_state;
> > > > -     struct drm_connector *connector;
> > > > +     struct drm_connector __maybe_unused *connector;
> > > >       struct drm_connector_state *new_conn_state;
> > > >       int i;
> > > >
> > > > @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> > > >       struct drm_crtc *crtc;
> > > >       struct drm_crtc_state *old_crtc_state;
> > > >       struct drm_crtc_state *new_crtc_state;
> > > > -     struct drm_connector *connector;
> > > > +     struct drm_connector __maybe_unused *connector;
> > > >       struct drm_connector_state *new_conn_state;
> > > >       int i;
> > > >
> > > > @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> > > >                                     struct drm_atomic_state *state,
> > > >                                     bool pre_swap)
> > > >  {
> > > > -     struct drm_plane *plane;
> > > > +     struct drm_plane __maybe_unused *plane;
> > > >       struct drm_plane_state *new_plane_state;
> > > >       int i, ret;
> > > >
> > > > @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
> > > >               struct drm_atomic_state *old_state)
> > > >  {
> > > >       struct drm_crtc *crtc;
> > > > -     struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > +     struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
> > > >       int i, ret;
> > > >       unsigned crtc_mask = 0;
> > > >
> > > > @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
> > > >  int drm_atomic_helper_async_check(struct drm_device *dev,
> > > >                                  struct drm_atomic_state *state)
> > > >  {
> > > > -     struct drm_crtc *crtc;
> > > > +     struct drm_crtc __maybe_unused *crtc;
> > > >       struct drm_crtc_state *crtc_state;
> > > >       struct drm_plane *plane = NULL;
> > > >       struct drm_plane_state *old_plane_state = NULL;
> > > > @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> > > >  {
> > > >       struct drm_crtc *crtc;
> > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > -     struct drm_connector *conn;
> > > > +     struct drm_connector __maybe_unused *conn;
> > > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > > -     struct drm_plane *plane;
> > > > +     struct drm_plane __maybe_unused *plane;
> > > >       struct drm_plane_state *old_plane_state, *new_plane_state;
> > > >       struct drm_crtc_commit *commit;
> > > >       int i, ret;
> > > > @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
> > > >   */
> > > >  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
> > > >  {
> > > > -     struct drm_crtc *crtc;
> > > > +     struct drm_crtc __maybe_unused *crtc;
> > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > >       struct drm_crtc_commit *commit;
> > > >       int i;
> > > > @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
> > > >  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> > > >                                    struct drm_atomic_state *state)
> > > >  {
> > > > -     struct drm_connector *connector;
> > > > +     struct drm_connector __maybe_unused *connector;
> > > >       struct drm_connector_state *new_conn_state;
> > > >       struct drm_plane *plane;
> > > >       struct drm_plane_state *new_plane_state;
> > > > @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
> > > >  {
> > > >       struct drm_atomic_state *state;
> > > >       struct drm_connector_state *conn_state;
> > > > -     struct drm_connector *conn;
> > > > +     struct drm_connector __maybe_unused *conn;
> > > >       struct drm_plane_state *plane_state;
> > > > -     struct drm_plane *plane;
> > > > +     struct drm_plane __maybe_unused *plane;
> > > >       struct drm_crtc_state *crtc_state;
> > > >       struct drm_crtc *crtc;
> > > >       int ret, i;
> > > > @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> > > >  {
> > > >       int i, ret;
> > > >       struct drm_plane *plane;
> > > > -     struct drm_plane_state *new_plane_state;
> > > > +     struct drm_plane_state __maybe_unused *new_plane_state;
> > > >       struct drm_connector *connector;
> > > > -     struct drm_connector_state *new_conn_state;
> > > > +     struct drm_connector_state __maybe_unused *new_conn_state;
> > > >       struct drm_crtc *crtc;
> > > > -     struct drm_crtc_state *new_crtc_state;
> > > > +     struct drm_crtc_state __maybe_unused *new_crtc_state;
> > > >
> > > >       state->acquire_ctx = ctx;
> > > >
> > > > --
> > > > 2.15.0
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
>
> --
> Ville Syrjälä
> Intel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-03 15:37         ` Benjamin Gaignard
@ 2019-10-03 15:46           ` Ville Syrjälä
  2019-10-04 10:48             ` Benjamin Gaignard
  2019-10-08  9:55           ` Daniel Vetter
  1 sibling, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-10-03 15:46 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Benjamin Gaignard, David Airlie, Daniel Vetter,
	Linux Kernel Mailing List, ML dri-devel

On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> <ville.syrjala@linux.intel.com> a écrit :
> >
> > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> > > Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> > > <ville.syrjala@linux.intel.com> a écrit :
> > > >
> > > > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > > > Fix warnings with W=1.
> > > > > Few for_each macro set variables that are never used later.
> > > > > Prevent warning by marking these variables as __maybe_unused.
> > > > >
> > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> > > > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > > > >             struct drm_encoder *encoder)
> > > > >  {
> > > > >       struct drm_crtc_state *crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > >
> > > > Rather ugly. IMO would be nicer if we could hide something inside
> > > > the iterator macros to suppress the warning.
> > >
> > > Ok but how ?
> > > connector is assigned in the macros but not used later and we can't
> > > set "__maybe_unused"
> > > in the macro.
> > > Does another keyword exist for that ?
> >
> > Stick a (void)(connector) into the macro?
> 
> That could work but it will look strange inside the macro.
> 
> >
> > Another (arguably cleaner) idea would be to remove the connector/crtc/plane
> > argument from the iterators entirely since it's redundant, and instead just
> > extract it from the appropriate new/old state as needed.
> >
> > We could then also add a for_each_connector_in_state()/etc. which omit
> > s the state arguments and just has the connector argument, for cases where
> > you don't care about the states when iterating.
> 
> That may lead to get a macro for each possible combination of used variables.

We already have new/old/oldnew, so would "just" add one more.

> 
> >
> > >
> > > >
> > > > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > > > >       int i;
> > > > >
> > > > > @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *new_conn_state;
> > > > >       int i;
> > > > >       int ret;
> > > > > @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > > > >       int i, ret;
> > > > >       unsigned connectors_mask = 0;
> > > > > @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
> > > > >  static void
> > > > >  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> > > > >  {
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *new_conn_state;
> > > > >       int i;
> > > > >
> > > > > @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state;
> > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *new_conn_state;
> > > > >       int i;
> > > > >
> > > > > @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> > > > >                                     struct drm_atomic_state *state,
> > > > >                                     bool pre_swap)
> > > > >  {
> > > > > -     struct drm_plane *plane;
> > > > > +     struct drm_plane __maybe_unused *plane;
> > > > >       struct drm_plane_state *new_plane_state;
> > > > >       int i, ret;
> > > > >
> > > > > @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
> > > > >               struct drm_atomic_state *old_state)
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > > -     struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > +     struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
> > > > >       int i, ret;
> > > > >       unsigned crtc_mask = 0;
> > > > >
> > > > > @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
> > > > >  int drm_atomic_helper_async_check(struct drm_device *dev,
> > > > >                                  struct drm_atomic_state *state)
> > > > >  {
> > > > > -     struct drm_crtc *crtc;
> > > > > +     struct drm_crtc __maybe_unused *crtc;
> > > > >       struct drm_crtc_state *crtc_state;
> > > > >       struct drm_plane *plane = NULL;
> > > > >       struct drm_plane_state *old_plane_state = NULL;
> > > > > @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > -     struct drm_connector *conn;
> > > > > +     struct drm_connector __maybe_unused *conn;
> > > > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > > > -     struct drm_plane *plane;
> > > > > +     struct drm_plane __maybe_unused *plane;
> > > > >       struct drm_plane_state *old_plane_state, *new_plane_state;
> > > > >       struct drm_crtc_commit *commit;
> > > > >       int i, ret;
> > > > > @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
> > > > >   */
> > > > >  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
> > > > >  {
> > > > > -     struct drm_crtc *crtc;
> > > > > +     struct drm_crtc __maybe_unused *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > >       struct drm_crtc_commit *commit;
> > > > >       int i;
> > > > > @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
> > > > >  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> > > > >                                    struct drm_atomic_state *state)
> > > > >  {
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *new_conn_state;
> > > > >       struct drm_plane *plane;
> > > > >       struct drm_plane_state *new_plane_state;
> > > > > @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
> > > > >  {
> > > > >       struct drm_atomic_state *state;
> > > > >       struct drm_connector_state *conn_state;
> > > > > -     struct drm_connector *conn;
> > > > > +     struct drm_connector __maybe_unused *conn;
> > > > >       struct drm_plane_state *plane_state;
> > > > > -     struct drm_plane *plane;
> > > > > +     struct drm_plane __maybe_unused *plane;
> > > > >       struct drm_crtc_state *crtc_state;
> > > > >       struct drm_crtc *crtc;
> > > > >       int ret, i;
> > > > > @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> > > > >  {
> > > > >       int i, ret;
> > > > >       struct drm_plane *plane;
> > > > > -     struct drm_plane_state *new_plane_state;
> > > > > +     struct drm_plane_state __maybe_unused *new_plane_state;
> > > > >       struct drm_connector *connector;
> > > > > -     struct drm_connector_state *new_conn_state;
> > > > > +     struct drm_connector_state __maybe_unused *new_conn_state;
> > > > >       struct drm_crtc *crtc;
> > > > > -     struct drm_crtc_state *new_crtc_state;
> > > > > +     struct drm_crtc_state __maybe_unused *new_crtc_state;
> > > > >
> > > > >       state->acquire_ctx = ctx;
> > > > >
> > > > > --
> > > > > 2.15.0
> > > > >
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > >
> > > > --
> > > > Ville Syrjälä
> > > > Intel
> >
> > --
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-03 15:46           ` Ville Syrjälä
@ 2019-10-04 10:48             ` Benjamin Gaignard
  2019-10-04 12:27               ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Benjamin Gaignard @ 2019-10-04 10:48 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Benjamin Gaignard, David Airlie, Daniel Vetter,
	Linux Kernel Mailing List, ML dri-devel

Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
<ville.syrjala@linux.intel.com> a écrit :
>
> On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> > Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> a écrit :
> > >
> > > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> > > > Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> > > > <ville.syrjala@linux.intel.com> a écrit :
> > > > >
> > > > > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > > > > Fix warnings with W=1.
> > > > > > Few for_each macro set variables that are never used later.
> > > > > > Prevent warning by marking these variables as __maybe_unused.
> > > > > >
> > > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> > > > > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > > > > >             struct drm_encoder *encoder)
> > > > > >  {
> > > > > >       struct drm_crtc_state *crtc_state;
> > > > > > -     struct drm_connector *connector;
> > > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >
> > > > > Rather ugly. IMO would be nicer if we could hide something inside
> > > > > the iterator macros to suppress the warning.
> > > >
> > > > Ok but how ?
> > > > connector is assigned in the macros but not used later and we can't
> > > > set "__maybe_unused"
> > > > in the macro.
> > > > Does another keyword exist for that ?
> > >
> > > Stick a (void)(connector) into the macro?
> >
> > That could work but it will look strange inside the macro.
> >
> > >
> > > Another (arguably cleaner) idea would be to remove the connector/crtc/plane
> > > argument from the iterators entirely since it's redundant, and instead just
> > > extract it from the appropriate new/old state as needed.
> > >
> > > We could then also add a for_each_connector_in_state()/etc. which omit
> > > s the state arguments and just has the connector argument, for cases where
> > > you don't care about the states when iterating.
> >
> > That may lead to get a macro for each possible combination of used variables.
>
> We already have new/old/oldnew, so would "just" add one more.

Not just one, it will be one each new/old/oldnew macro to be able to distinguish
when connector is used or not.
And it will be the same for the for_each macros...

>
> >
> > >
> > > >
> > > > >
> > > > > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > > > > >       int i;
> > > > > >
> > > > > > @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
> > > > > >  {
> > > > > >       struct drm_crtc *crtc;
> > > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > > -     struct drm_connector *connector;
> > > > > > +     struct drm_connector __maybe_unused *connector;
> > > > > >       struct drm_connector_state *new_conn_state;
> > > > > >       int i;
> > > > > >       int ret;
> > > > > > @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > > > > >  {
> > > > > >       struct drm_crtc *crtc;
> > > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > > -     struct drm_connector *connector;
> > > > > > +     struct drm_connector __maybe_unused *connector;
> > > > > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > > > > >       int i, ret;
> > > > > >       unsigned connectors_mask = 0;
> > > > > > @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
> > > > > >  static void
> > > > > >  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> > > > > >  {
> > > > > > -     struct drm_connector *connector;
> > > > > > +     struct drm_connector __maybe_unused *connector;
> > > > > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > > > >       struct drm_crtc *crtc;
> > > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > > @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
> > > > > >  {
> > > > > >       struct drm_crtc *crtc;
> > > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > > -     struct drm_connector *connector;
> > > > > > +     struct drm_connector __maybe_unused *connector;
> > > > > >       struct drm_connector_state *new_conn_state;
> > > > > >       int i;
> > > > > >
> > > > > > @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> > > > > >       struct drm_crtc *crtc;
> > > > > >       struct drm_crtc_state *old_crtc_state;
> > > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > > -     struct drm_connector *connector;
> > > > > > +     struct drm_connector __maybe_unused *connector;
> > > > > >       struct drm_connector_state *new_conn_state;
> > > > > >       int i;
> > > > > >
> > > > > > @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> > > > > >                                     struct drm_atomic_state *state,
> > > > > >                                     bool pre_swap)
> > > > > >  {
> > > > > > -     struct drm_plane *plane;
> > > > > > +     struct drm_plane __maybe_unused *plane;
> > > > > >       struct drm_plane_state *new_plane_state;
> > > > > >       int i, ret;
> > > > > >
> > > > > > @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
> > > > > >               struct drm_atomic_state *old_state)
> > > > > >  {
> > > > > >       struct drm_crtc *crtc;
> > > > > > -     struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > > +     struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
> > > > > >       int i, ret;
> > > > > >       unsigned crtc_mask = 0;
> > > > > >
> > > > > > @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
> > > > > >  int drm_atomic_helper_async_check(struct drm_device *dev,
> > > > > >                                  struct drm_atomic_state *state)
> > > > > >  {
> > > > > > -     struct drm_crtc *crtc;
> > > > > > +     struct drm_crtc __maybe_unused *crtc;
> > > > > >       struct drm_crtc_state *crtc_state;
> > > > > >       struct drm_plane *plane = NULL;
> > > > > >       struct drm_plane_state *old_plane_state = NULL;
> > > > > > @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> > > > > >  {
> > > > > >       struct drm_crtc *crtc;
> > > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > > -     struct drm_connector *conn;
> > > > > > +     struct drm_connector __maybe_unused *conn;
> > > > > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > > > > -     struct drm_plane *plane;
> > > > > > +     struct drm_plane __maybe_unused *plane;
> > > > > >       struct drm_plane_state *old_plane_state, *new_plane_state;
> > > > > >       struct drm_crtc_commit *commit;
> > > > > >       int i, ret;
> > > > > > @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
> > > > > >   */
> > > > > >  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
> > > > > >  {
> > > > > > -     struct drm_crtc *crtc;
> > > > > > +     struct drm_crtc __maybe_unused *crtc;
> > > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > >       struct drm_crtc_commit *commit;
> > > > > >       int i;
> > > > > > @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
> > > > > >  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> > > > > >                                    struct drm_atomic_state *state)
> > > > > >  {
> > > > > > -     struct drm_connector *connector;
> > > > > > +     struct drm_connector __maybe_unused *connector;
> > > > > >       struct drm_connector_state *new_conn_state;
> > > > > >       struct drm_plane *plane;
> > > > > >       struct drm_plane_state *new_plane_state;
> > > > > > @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
> > > > > >  {
> > > > > >       struct drm_atomic_state *state;
> > > > > >       struct drm_connector_state *conn_state;
> > > > > > -     struct drm_connector *conn;
> > > > > > +     struct drm_connector __maybe_unused *conn;
> > > > > >       struct drm_plane_state *plane_state;
> > > > > > -     struct drm_plane *plane;
> > > > > > +     struct drm_plane __maybe_unused *plane;
> > > > > >       struct drm_crtc_state *crtc_state;
> > > > > >       struct drm_crtc *crtc;
> > > > > >       int ret, i;
> > > > > > @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> > > > > >  {
> > > > > >       int i, ret;
> > > > > >       struct drm_plane *plane;
> > > > > > -     struct drm_plane_state *new_plane_state;
> > > > > > +     struct drm_plane_state __maybe_unused *new_plane_state;
> > > > > >       struct drm_connector *connector;
> > > > > > -     struct drm_connector_state *new_conn_state;
> > > > > > +     struct drm_connector_state __maybe_unused *new_conn_state;
> > > > > >       struct drm_crtc *crtc;
> > > > > > -     struct drm_crtc_state *new_crtc_state;
> > > > > > +     struct drm_crtc_state __maybe_unused *new_crtc_state;
> > > > > >
> > > > > >       state->acquire_ctx = ctx;
> > > > > >
> > > > > > --
> > > > > > 2.15.0
> > > > > >
> > > > > > _______________________________________________
> > > > > > dri-devel mailing list
> > > > > > dri-devel@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > >
> > > > > --
> > > > > Ville Syrjälä
> > > > > Intel
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
>
> --
> Ville Syrjälä
> Intel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-04 10:48             ` Benjamin Gaignard
@ 2019-10-04 12:27               ` Ville Syrjälä
  2019-10-04 12:36                 ` Benjamin GAIGNARD
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-10-04 12:27 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Benjamin Gaignard, David Airlie, Daniel Vetter,
	Linux Kernel Mailing List, ML dri-devel

On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
> <ville.syrjala@linux.intel.com> a écrit :
> >
> > On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> > > Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> > > <ville.syrjala@linux.intel.com> a écrit :
> > > >
> > > > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> > > > > Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> > > > > <ville.syrjala@linux.intel.com> a écrit :
> > > > > >
> > > > > > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > > > > > Fix warnings with W=1.
> > > > > > > Few for_each macro set variables that are never used later.
> > > > > > > Prevent warning by marking these variables as __maybe_unused.
> > > > > > >
> > > > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> > > > > > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > > > > > >             struct drm_encoder *encoder)
> > > > > > >  {
> > > > > > >       struct drm_crtc_state *crtc_state;
> > > > > > > -     struct drm_connector *connector;
> > > > > > > +     struct drm_connector __maybe_unused *connector;
> > > > > >
> > > > > > Rather ugly. IMO would be nicer if we could hide something inside
> > > > > > the iterator macros to suppress the warning.
> > > > >
> > > > > Ok but how ?
> > > > > connector is assigned in the macros but not used later and we can't
> > > > > set "__maybe_unused"
> > > > > in the macro.
> > > > > Does another keyword exist for that ?
> > > >
> > > > Stick a (void)(connector) into the macro?
> > >
> > > That could work but it will look strange inside the macro.
> > >
> > > >
> > > > Another (arguably cleaner) idea would be to remove the connector/crtc/plane
> > > > argument from the iterators entirely since it's redundant, and instead just
> > > > extract it from the appropriate new/old state as needed.
> > > >
> > > > We could then also add a for_each_connector_in_state()/etc. which omit
> > > > s the state arguments and just has the connector argument, for cases where
> > > > you don't care about the states when iterating.
> > >
> > > That may lead to get a macro for each possible combination of used variables.
> >
> > We already have new/old/oldnew, so would "just" add one more.
> 
> Not just one, it will be one each new/old/oldnew macro to be able to distinguish
> when connector is used or not.

What I'm suggesting is this:
for_each_connector_in_state(state, connector, i)
for_each_old_connector_in_state(state, old_conn_state, i)
for_each_new_connector_in_state(state, new_conn_state, i)
for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)

So only four in total for each object type, instead of the current
three.

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-04 12:27               ` Ville Syrjälä
@ 2019-10-04 12:36                 ` Benjamin GAIGNARD
  2019-10-04 13:08                   ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Benjamin GAIGNARD @ 2019-10-04 12:36 UTC (permalink / raw)
  To: Ville Syrjälä, Benjamin Gaignard
  Cc: David Airlie, Daniel Vetter, Linux Kernel Mailing List, ML dri-devel


On 10/4/19 2:27 PM, Ville Syrjälä wrote:
> On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
>> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
>> <ville.syrjala@linux.intel.com> a écrit :
>>> On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
>>>> Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
>>>> <ville.syrjala@linux.intel.com> a écrit :
>>>>> On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
>>>>>> Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
>>>>>> <ville.syrjala@linux.intel.com> a écrit :
>>>>>>> On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
>>>>>>>> Fix warnings with W=1.
>>>>>>>> Few for_each macro set variables that are never used later.
>>>>>>>> Prevent warning by marking these variables as __maybe_unused.
>>>>>>>>
>>>>>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>>>>>> ---
>>>>>>>>   drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
>>>>>>>>   1 file changed, 18 insertions(+), 18 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>>>>>>> index aa16ea17ff9b..b69d17b0b9bd 100644
>>>>>>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>>>>>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>>>>>>> @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
>>>>>>>>              struct drm_encoder *encoder)
>>>>>>>>   {
>>>>>>>>        struct drm_crtc_state *crtc_state;
>>>>>>>> -     struct drm_connector *connector;
>>>>>>>> +     struct drm_connector __maybe_unused *connector;
>>>>>>> Rather ugly. IMO would be nicer if we could hide something inside
>>>>>>> the iterator macros to suppress the warning.
>>>>>> Ok but how ?
>>>>>> connector is assigned in the macros but not used later and we can't
>>>>>> set "__maybe_unused"
>>>>>> in the macro.
>>>>>> Does another keyword exist for that ?
>>>>> Stick a (void)(connector) into the macro?
>>>> That could work but it will look strange inside the macro.
>>>>
>>>>> Another (arguably cleaner) idea would be to remove the connector/crtc/plane
>>>>> argument from the iterators entirely since it's redundant, and instead just
>>>>> extract it from the appropriate new/old state as needed.
>>>>>
>>>>> We could then also add a for_each_connector_in_state()/etc. which omit
>>>>> s the state arguments and just has the connector argument, for cases where
>>>>> you don't care about the states when iterating.
>>>> That may lead to get a macro for each possible combination of used variables.
>>> We already have new/old/oldnew, so would "just" add one more.
>> Not just one, it will be one each new/old/oldnew macro to be able to distinguish
>> when connector is used or not.
> What I'm suggesting is this:
> for_each_connector_in_state(state, connector, i)
> for_each_old_connector_in_state(state, old_conn_state, i)
> for_each_new_connector_in_state(state, new_conn_state, i)
> for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)
>
> So only four in total for each object type, instead of the current
> three.

You are missing these cases: old and connector, new and connector,

old and new and connector are needed together.

>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-04 12:36                 ` Benjamin GAIGNARD
@ 2019-10-04 13:08                   ` Ville Syrjälä
  0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2019-10-04 13:08 UTC (permalink / raw)
  To: Benjamin GAIGNARD
  Cc: Benjamin Gaignard, David Airlie, Daniel Vetter,
	Linux Kernel Mailing List, ML dri-devel

On Fri, Oct 04, 2019 at 12:36:56PM +0000, Benjamin GAIGNARD wrote:
> 
> On 10/4/19 2:27 PM, Ville Syrjälä wrote:
> > On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
> >> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
> >> <ville.syrjala@linux.intel.com> a écrit :
> >>> On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> >>>> Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> >>>> <ville.syrjala@linux.intel.com> a écrit :
> >>>>> On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> >>>>>> Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> >>>>>> <ville.syrjala@linux.intel.com> a écrit :
> >>>>>>> On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> >>>>>>>> Fix warnings with W=1.
> >>>>>>>> Few for_each macro set variables that are never used later.
> >>>>>>>> Prevent warning by marking these variables as __maybe_unused.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >>>>>>>> ---
> >>>>>>>>   drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> >>>>>>>>   1 file changed, 18 insertions(+), 18 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> >>>>>>>> index aa16ea17ff9b..b69d17b0b9bd 100644
> >>>>>>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >>>>>>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >>>>>>>> @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> >>>>>>>>              struct drm_encoder *encoder)
> >>>>>>>>   {
> >>>>>>>>        struct drm_crtc_state *crtc_state;
> >>>>>>>> -     struct drm_connector *connector;
> >>>>>>>> +     struct drm_connector __maybe_unused *connector;
> >>>>>>> Rather ugly. IMO would be nicer if we could hide something inside
> >>>>>>> the iterator macros to suppress the warning.
> >>>>>> Ok but how ?
> >>>>>> connector is assigned in the macros but not used later and we can't
> >>>>>> set "__maybe_unused"
> >>>>>> in the macro.
> >>>>>> Does another keyword exist for that ?
> >>>>> Stick a (void)(connector) into the macro?
> >>>> That could work but it will look strange inside the macro.
> >>>>
> >>>>> Another (arguably cleaner) idea would be to remove the connector/crtc/plane
> >>>>> argument from the iterators entirely since it's redundant, and instead just
> >>>>> extract it from the appropriate new/old state as needed.
> >>>>>
> >>>>> We could then also add a for_each_connector_in_state()/etc. which omit
> >>>>> s the state arguments and just has the connector argument, for cases where
> >>>>> you don't care about the states when iterating.
> >>>> That may lead to get a macro for each possible combination of used variables.
> >>> We already have new/old/oldnew, so would "just" add one more.
> >> Not just one, it will be one each new/old/oldnew macro to be able to distinguish
> >> when connector is used or not.
> > What I'm suggesting is this:
> > for_each_connector_in_state(state, connector, i)
> > for_each_old_connector_in_state(state, old_conn_state, i)
> > for_each_new_connector_in_state(state, new_conn_state, i)
> > for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)
> >
> > So only four in total for each object type, instead of the current
> > three.
> 
> You are missing these cases: old and connector, new and connector,
> 
> old and new and connector are needed together.

No, that's redundant. You can always get the connector from
old/new_conn_state->connector if you need it.

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm: atomic helper: fix W=1 warnings
  2019-10-03 15:37         ` Benjamin Gaignard
  2019-10-03 15:46           ` Ville Syrjälä
@ 2019-10-08  9:55           ` Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2019-10-08  9:55 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Ville Syrjälä,
	Benjamin Gaignard, David Airlie, Daniel Vetter,
	Linux Kernel Mailing List, ML dri-devel

On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> <ville.syrjala@linux.intel.com> a écrit :
> >
> > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> > > Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> > > <ville.syrjala@linux.intel.com> a écrit :
> > > >
> > > > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > > > Fix warnings with W=1.
> > > > > Few for_each macro set variables that are never used later.
> > > > > Prevent warning by marking these variables as __maybe_unused.
> > > > >
> > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++------------------
> > > > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > > > >             struct drm_encoder *encoder)
> > > > >  {
> > > > >       struct drm_crtc_state *crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > >
> > > > Rather ugly. IMO would be nicer if we could hide something inside
> > > > the iterator macros to suppress the warning.
> > >
> > > Ok but how ?
> > > connector is assigned in the macros but not used later and we can't
> > > set "__maybe_unused"
> > > in the macro.
> > > Does another keyword exist for that ?
> >
> > Stick a (void)(connector) into the macro?
> 
> That could work but it will look strange inside the macro.

If this works I think it's fine, maybe together with a FIXME or so ... At
least as an interim solution. Much better than sprinkling all the
maybe_unused annotations around like in this patch.
-Daniel
> 
> >
> > Another (arguably cleaner) idea would be to remove the connector/crtc/plane
> > argument from the iterators entirely since it's redundant, and instead just
> > extract it from the appropriate new/old state as needed.
> >
> > We could then also add a for_each_connector_in_state()/etc. which omit
> > s the state arguments and just has the connector argument, for cases where
> > you don't care about the states when iterating.
> 
> That may lead to get a macro for each possible combination of used variables.
> 
> >
> > >
> > > >
> > > > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > > > >       int i;
> > > > >
> > > > > @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *new_conn_state;
> > > > >       int i;
> > > > >       int ret;
> > > > > @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *old_connector_state, *new_connector_state;
> > > > >       int i, ret;
> > > > >       unsigned connectors_mask = 0;
> > > > > @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
> > > > >  static void
> > > > >  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> > > > >  {
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *new_conn_state;
> > > > >       int i;
> > > > >
> > > > > @@ -1294,7 +1294,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state;
> > > > >       struct drm_crtc_state *new_crtc_state;
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *new_conn_state;
> > > > >       int i;
> > > > >
> > > > > @@ -1384,7 +1384,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> > > > >                                     struct drm_atomic_state *state,
> > > > >                                     bool pre_swap)
> > > > >  {
> > > > > -     struct drm_plane *plane;
> > > > > +     struct drm_plane __maybe_unused *plane;
> > > > >       struct drm_plane_state *new_plane_state;
> > > > >       int i, ret;
> > > > >
> > > > > @@ -1431,7 +1431,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
> > > > >               struct drm_atomic_state *old_state)
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > > -     struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > +     struct drm_crtc_state __maybe_unused *old_crtc_state, *new_crtc_state;
> > > > >       int i, ret;
> > > > >       unsigned crtc_mask = 0;
> > > > >
> > > > > @@ -1621,7 +1621,7 @@ static void commit_work(struct work_struct *work)
> > > > >  int drm_atomic_helper_async_check(struct drm_device *dev,
> > > > >                                  struct drm_atomic_state *state)
> > > > >  {
> > > > > -     struct drm_crtc *crtc;
> > > > > +     struct drm_crtc __maybe_unused *crtc;
> > > > >       struct drm_crtc_state *crtc_state;
> > > > >       struct drm_plane *plane = NULL;
> > > > >       struct drm_plane_state *old_plane_state = NULL;
> > > > > @@ -1982,9 +1982,9 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> > > > >  {
> > > > >       struct drm_crtc *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > -     struct drm_connector *conn;
> > > > > +     struct drm_connector __maybe_unused *conn;
> > > > >       struct drm_connector_state *old_conn_state, *new_conn_state;
> > > > > -     struct drm_plane *plane;
> > > > > +     struct drm_plane __maybe_unused *plane;
> > > > >       struct drm_plane_state *old_plane_state, *new_plane_state;
> > > > >       struct drm_crtc_commit *commit;
> > > > >       int i, ret;
> > > > > @@ -2214,7 +2214,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank);
> > > > >   */
> > > > >  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
> > > > >  {
> > > > > -     struct drm_crtc *crtc;
> > > > > +     struct drm_crtc __maybe_unused *crtc;
> > > > >       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > >       struct drm_crtc_commit *commit;
> > > > >       int i;
> > > > > @@ -2300,7 +2300,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
> > > > >  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> > > > >                                    struct drm_atomic_state *state)
> > > > >  {
> > > > > -     struct drm_connector *connector;
> > > > > +     struct drm_connector __maybe_unused *connector;
> > > > >       struct drm_connector_state *new_conn_state;
> > > > >       struct drm_plane *plane;
> > > > >       struct drm_plane_state *new_plane_state;
> > > > > @@ -2953,9 +2953,9 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
> > > > >  {
> > > > >       struct drm_atomic_state *state;
> > > > >       struct drm_connector_state *conn_state;
> > > > > -     struct drm_connector *conn;
> > > > > +     struct drm_connector __maybe_unused *conn;
> > > > >       struct drm_plane_state *plane_state;
> > > > > -     struct drm_plane *plane;
> > > > > +     struct drm_plane __maybe_unused *plane;
> > > > >       struct drm_crtc_state *crtc_state;
> > > > >       struct drm_crtc *crtc;
> > > > >       int ret, i;
> > > > > @@ -3199,11 +3199,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> > > > >  {
> > > > >       int i, ret;
> > > > >       struct drm_plane *plane;
> > > > > -     struct drm_plane_state *new_plane_state;
> > > > > +     struct drm_plane_state __maybe_unused *new_plane_state;
> > > > >       struct drm_connector *connector;
> > > > > -     struct drm_connector_state *new_conn_state;
> > > > > +     struct drm_connector_state __maybe_unused *new_conn_state;
> > > > >       struct drm_crtc *crtc;
> > > > > -     struct drm_crtc_state *new_crtc_state;
> > > > > +     struct drm_crtc_state __maybe_unused *new_crtc_state;
> > > > >
> > > > >       state->acquire_ctx = ctx;
> > > > >
> > > > > --
> > > > > 2.15.0
> > > > >
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > >
> > > > --
> > > > Ville Syrjälä
> > > > Intel
> >
> > --
> > Ville Syrjälä
> > Intel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-10-08  9:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-09 13:52 [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config Benjamin Gaignard
2019-09-09 13:52 ` [PATCH] drm: atomic helper: fix W=1 warnings Benjamin Gaignard
2019-09-16 13:19   ` Benjamin Gaignard
2019-10-03  9:52     ` Benjamin Gaignard
2019-10-03 14:27   ` Ville Syrjälä
2019-10-03 14:46     ` Benjamin Gaignard
2019-10-03 15:05       ` Ville Syrjälä
2019-10-03 15:37         ` Benjamin Gaignard
2019-10-03 15:46           ` Ville Syrjälä
2019-10-04 10:48             ` Benjamin Gaignard
2019-10-04 12:27               ` Ville Syrjälä
2019-10-04 12:36                 ` Benjamin GAIGNARD
2019-10-04 13:08                   ` Ville Syrjälä
2019-10-08  9:55           ` Daniel Vetter
2019-09-10 12:58 ` [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config Harry Wentland
2019-09-10 17:58   ` Manasi Navare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).