Re: [cip-dev] [linux-cip:linux-4.19.y-cip 7/17] drivers/gpu/drm/drm_atomic.c:1264:1: warning: no previous prototype for 'drm_atomic_get_old_private_obj_state'

Biju Das
2020-08-11T13:58:50Z

[-- Attachment #1: Type: text/plain, Size: 6010 bytes --]

Hi All,

I have received a warning message mail from Kernel test robot. I have investigated this issue and found there were lots of warnings with architecture by following the steps mentioned in this mail.

Then I investigated this particular issue and found that issue is caused by bad commit done by me while backporting. The below backport commit has
extra functions from patch1(0001-drm-Add-drm_atomic_get_old-new_private_obj_state.patch), but the corresponding header is missing.

cip commit: cacc2a81b9872aa8830353110b1252021f124ae8
upstream commit: 1b27fbdde1df172dba604855c45078d741f8c858

I have attached 3 patches, that looks like fixes the issue mentioned by kernel robot.

How do we proceed here?
drop cip commit: cacc2a81b9872aa8830353110b1252021f124ae8 and apply the 3 patches to fix this issue?

Please share your views.

Regards,
Biju

> -----Original Message-----
> From: kernel test robot <lkp@intel.com>
> Sent: 09 August 2020 15:13
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: kbuild-all@lists.01.org; Pavel Machek <pavel@denx.de>; Daniel Vetter
> <daniel@ffwll.ch>; Sean Paul <seanpaul@chromium.org>; Biju Das
> <biju.das.jz@bp.renesas.com>
> Subject: [linux-cip:linux-4.19.y-cip 7/17]
> drivers/gpu/drm/drm_atomic.c:1264:1: warning: no previous prototype for
> 'drm_atomic_get_old_private_obj_state'
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git linux-
> 4.19.y-cip
> head: 72750517bc5f1718ce3cb3fa75d4cadc05d27eee
> commit: 60b4667275b60d92c760d48df95f5b80bbcfde6e [7/17] drm: Add
> drm_atomic_get_(old|new)_connector_for_encoder() helpers
> config: riscv-randconfig-r034-20200809 (attached as .config)
> compiler: riscv32-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-
> tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git checkout 60b4667275b60d92c760d48df95f5b80bbcfde6e
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0
> make.cross ARCH=riscv
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/gpu/drm/drm_atomic.c:1264:1: warning: no previous prototype
> >> for 'drm_atomic_get_old_private_obj_state' [-Wmissing-prototypes]
> 1264 | drm_atomic_get_old_private_obj_state(struct drm_atomic_state
> *state,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> drivers/gpu/drm/drm_atomic.c:1286:1: warning: no previous prototype
> >> for 'drm_atomic_get_new_private_obj_state' [-Wmissing-prototypes]
> 1286 | drm_atomic_get_new_private_obj_state(struct drm_atomic_state
> *state,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/drm_atomic.c: In function 'drm_atomic_print_state':
> drivers/gpu/drm/drm_atomic.c:2184:24: warning: variable 'connector' set
> but not used [-Wunused-but-set-variable]
> 2184 | struct drm_connector *connector;
> | ^~~~~~~~~
> drivers/gpu/drm/drm_atomic.c:2182:19: warning: variable 'crtc' set but not
> used [-Wunused-but-set-variable]
> 2182 | struct drm_crtc *crtc;
> | ^~~~
> drivers/gpu/drm/drm_atomic.c:2180:20: warning: variable 'plane' set but
> not used [-Wunused-but-set-variable]
> 2180 | struct drm_plane *plane;
> | ^~~~~
> drivers/gpu/drm/drm_atomic.c: In function 'complete_signaling':
> drivers/gpu/drm/drm_atomic.c:2624:19: warning: variable 'crtc' set but not
> used [-Wunused-but-set-variable]
> 2624 | struct drm_crtc *crtc;
> | ^~~~
>
> vim +/drm_atomic_get_old_private_obj_state +1264
> drivers/gpu/drm/drm_atomic.c
>
> 1254
> 1255/**
> 1256 * drm_atomic_get_old_private_obj_state
> 1257 * @state: global atomic state object
> 1258 * @obj: private_obj to grab
> 1259 *
> 1260 * This function returns the old private object state for the given
> private_obj,
> 1261 * or NULL if the private_obj is not part of the global atomic state.
> 1262 */
> 1263struct drm_private_state *
> > 1264drm_atomic_get_old_private_obj_state(struct drm_atomic_state
> *state,
> 1265 struct drm_private_obj *obj)
> 1266{
> 1267int i;
> 1268
> 1269for (i = 0; i < state->num_private_objs; i++)
> 1270if (obj == state->private_objs[i].ptr)
> 1271return state->private_objs[i].old_state;
> 1272
> 1273return NULL;
> 1274}
> 1275EXPORT_SYMBOL(drm_atomic_get_old_private_obj_state);
> 1276
> 1277/**
> 1278 * drm_atomic_get_new_private_obj_state
> 1279 * @state: global atomic state object
> 1280 * @obj: private_obj to grab
> 1281 *
> 1282 * This function returns the new private object state for the given
> private_obj,
> 1283 * or NULL if the private_obj is not part of the global atomic state.
> 1284 */
> 1285struct drm_private_state *
> > 1286drm_atomic_get_new_private_obj_state(struct drm_atomic_state
> *state,
> 1287 struct drm_private_obj *obj)
> 1288{
> 1289int i;
> 1290
> 1291for (i = 0; i < state->num_private_objs; i++)
> 1292if (obj == state->private_objs[i].ptr)
> 1293return state->private_objs[i].new_state;
> 1294
> 1295return NULL;
> 1296}
> 1297EXPORT_SYMBOL(drm_atomic_get_new_private_obj_state);
> 1298
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

[-- Attachment #2: 0002-drm-Add-drm_atomic_get_-old-new-_connector_for_encod.patch --]
[-- Type: application/octet-stream, Size: 6257 bytes --]

From 735d26c1d92f35a9e0bba029eafb43e98b78803b Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Tue, 11 Jun 2019 16:51:43 -0400
Subject: [PATCH 2/3] drm: Add drm_atomic_get_(old|new)_connector_for_encoder()
helpers

commit 1b27fbdde1df172dba604855c45078d741f8c858 upstream.

Add functions to the atomic core to retrieve the old and new connectors
associated with an encoder in a drm_atomic_state. This is useful for
encoders and bridges that need to access the connector, for instance for
the drm_display_info.

The CRTC associated with the encoder can also be retrieved through the
connector state, and from it, the old and new CRTC states.

Changed in v4:
- Added to the set
Changed in v5:
- Fix up docbook (Daniel & Laurent)
Changed in v6:
- Updated commit subject (Sam)

Link to v4: https://patchwork.freedesktop.org/patch/msgid/20190508160920.144739-3-sean@poorly.run
Link to v5: https://patchwork.freedesktop.org/patch/msgid/20190611160844.257498-3-sean@poorly.run

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Sam Ravnborg <sam@ravnborg.org>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

[seanpaul removed WARNs from helpers and added docs to explain why
returning NULL might be valid]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20190611205147.181298-1-sean@poorly.run
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/gpu/drm/drm_atomic.c | 69 ++++++++++++++++++++++++++++++++++++
include/drm/drm_atomic.h | 7 ++++
include/drm/drm_connector.h | 9 +++++
3 files changed, 85 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5bbf969eb43a..d36742761ada 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1296,6 +1296,75 @@ drm_atomic_get_new_private_obj_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_new_private_obj_state);

+/**
+ * drm_atomic_get_old_connector_for_encoder - Get old connector for an encoder
+ * @state: Atomic state
+ * @encoder: The encoder to fetch the connector state for
+ *
+ * This function finds and returns the connector that was connected to @encoder
+ * as specified by the @state.
+ *
+ * If there is no connector in @state which previously had @encoder connected to
+ * it, this function will return NULL. While this may seem like an invalid use
+ * case, it is sometimes useful to differentiate commits which had no prior
+ * connectors attached to @encoder vs ones that did (and to inspect their
+ * state). This is especially true in enable hooks because the pipeline has
+ * changed.
+ *
+ * Returns: The old connector connected to @encoder, or NULL if the encoder is
+ * not connected.
+ */
+struct drm_connector *
+drm_atomic_get_old_connector_for_encoder(struct drm_atomic_state *state,
+ struct drm_encoder *encoder)
+{
+ struct drm_connector_state *conn_state;
+ struct drm_connector *connector;
+ unsigned int i;
+
+ for_each_old_connector_in_state(state, connector, conn_state, i) {
+ if (conn_state->best_encoder == encoder)
+ return connector;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL(drm_atomic_get_old_connector_for_encoder);
+
+/**
+ * drm_atomic_get_new_connector_for_encoder - Get new connector for an encoder
+ * @state: Atomic state
+ * @encoder: The encoder to fetch the connector state for
+ *
+ * This function finds and returns the connector that will be connected to
+ * @encoder as specified by the @state.
+ *
+ * If there is no connector in @state which will have @encoder connected to it,
+ * this function will return NULL. While this may seem like an invalid use case,
+ * it is sometimes useful to differentiate commits which have no connectors
+ * attached to @encoder vs ones that do (and to inspect their state). This is
+ * especially true in disable hooks because the pipeline will change.
+ *
+ * Returns: The new connector connected to @encoder, or NULL if the encoder is
+ * not connected.
+ */
+struct drm_connector *
+drm_atomic_get_new_connector_for_encoder(struct drm_atomic_state *state,
+ struct drm_encoder *encoder)
+{
+ struct drm_connector_state *conn_state;
+ struct drm_connector *connector;
+ unsigned int i;
+
+ for_each_new_connector_in_state(state, connector, conn_state, i) {
+ if (conn_state->best_encoder == encoder)
+ return connector;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL(drm_atomic_get_new_connector_for_encoder);
+
/**
* drm_atomic_get_connector_state - get connector state
* @state: global atomic state object
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index d2ff456ce72f..6329ed4af310 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -409,6 +409,13 @@ struct drm_private_state *
drm_atomic_get_new_private_obj_state(struct drm_atomic_state *state,
struct drm_private_obj *obj);

+struct drm_connector *
+drm_atomic_get_old_connector_for_encoder(struct drm_atomic_state *state,
+ struct drm_encoder *encoder);
+struct drm_connector *
+drm_atomic_get_new_connector_for_encoder(struct drm_atomic_state *state,
+ struct drm_encoder *encoder);
+
/**
* drm_atomic_get_existing_crtc_state - get crtc state, if it exists
* @state: global atomic state object
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 97ea41dc678f..6f0a05b6581d 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -397,6 +397,15 @@ struct drm_connector_state {
* Used by the atomic helpers to select the encoder, through the
* &drm_connector_helper_funcs.atomic_best_encoder or
* &drm_connector_helper_funcs.best_encoder callbacks.
+ *
+ * This is also used in the atomic helpers to map encoders to their
+ * current and previous connectors, see
+ * &drm_atomic_get_old_connector_for_encoder() and
+ * &drm_atomic_get_new_connector_for_encoder().
+ *
+ * NOTE: Atomic drivers must fill this out (either themselves or through
+ * helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will
+ * not return correct data to userspace.
*/
struct drm_encoder *best_encoder;

--
2.17.1

[-- Attachment #3: 0003-drm-atomic-helper-fix-W-1-warnings.patch --]
[-- Type: application/octet-stream, Size: 4621 bytes --]

From b94e9dd5700b00669aba6e06e1d1bd2629ab3735 Mon Sep 17 00:00:00 2001
From: Benjamin Gaignard <benjamin.gaignard@st.com>
Date: Tue, 8 Oct 2019 14:42:54 +0200
Subject: [PATCH 3/3] drm: atomic helper: fix W=1 warnings

commit bf5d837a0a4ced7cc223befc9e76d4ad30697d27 upstream.

Few for_each macro set variables that are never used later which led
to generate unused-but-set-variable warnings.
Add (void)(foo) inside the macros to remove these warnings

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20191008124254.2144-1-benjamin.gaignard@st.com
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
include/drm/drm_atomic.h | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 6329ed4af310..9042fdae9211 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -662,6 +662,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
(__i)++) \
for_each_if ((__state)->connectors[__i].ptr && \
((connector) = (__state)->connectors[__i].ptr, \
+ (void)(connector) /* Only to avoid unused-but-set-variable warning */, \
(old_connector_state) = (__state)->connectors[__i].old_state, \
(new_connector_state) = (__state)->connectors[__i].new_state, 1))

@@ -683,6 +684,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
(__i)++) \
for_each_if ((__state)->connectors[__i].ptr && \
((connector) = (__state)->connectors[__i].ptr, \
+ (void)(connector) /* Only to avoid unused-but-set-variable warning */, \
(old_connector_state) = (__state)->connectors[__i].old_state, 1))

/**
@@ -703,7 +705,9 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
(__i)++) \
for_each_if ((__state)->connectors[__i].ptr && \
((connector) = (__state)->connectors[__i].ptr, \
- (new_connector_state) = (__state)->connectors[__i].new_state, 1))
+ (void)(connector) /* Only to avoid unused-but-set-variable warning */, \
+ (new_connector_state) = (__state)->connectors[__i].new_state, \
+ (void)(new_connector_state) /* Only to avoid unused-but-set-variable warning */, 1))

/**
* for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update
@@ -723,7 +727,9 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
(__i)++) \
for_each_if ((__state)->crtcs[__i].ptr && \
((crtc) = (__state)->crtcs[__i].ptr, \
+ (void)(crtc) /* Only to avoid unused-but-set-variable warning */, \
(old_crtc_state) = (__state)->crtcs[__i].old_state, \
+ (void)(old_crtc_state) /* Only to avoid unused-but-set-variable warning */, \
(new_crtc_state) = (__state)->crtcs[__i].new_state, 1))

/**
@@ -762,7 +768,9 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
(__i)++) \
for_each_if ((__state)->crtcs[__i].ptr && \
((crtc) = (__state)->crtcs[__i].ptr, \
- (new_crtc_state) = (__state)->crtcs[__i].new_state, 1))
+ (void)(crtc) /* Only to avoid unused-but-set-variable warning */, \
+ (new_crtc_state) = (__state)->crtcs[__i].new_state, \
+ (void)(new_crtc_state) /* Only to avoid unused-but-set-variable warning */, 1))

/**
* for_each_oldnew_plane_in_state - iterate over all planes in an atomic update
@@ -782,6 +790,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
(__i)++) \
for_each_if ((__state)->planes[__i].ptr && \
((plane) = (__state)->planes[__i].ptr, \
+ (void)(plane) /* Only to avoid unused-but-set-variable warning */, \
(old_plane_state) = (__state)->planes[__i].old_state,\
(new_plane_state) = (__state)->planes[__i].new_state, 1))

@@ -842,7 +851,9 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
(__i)++) \
for_each_if ((__state)->planes[__i].ptr && \
((plane) = (__state)->planes[__i].ptr, \
- (new_plane_state) = (__state)->planes[__i].new_state, 1))
+ (void)(plane) /* Only to avoid unused-but-set-variable warning */, \
+ (new_plane_state) = (__state)->planes[__i].new_state, \
+ (void)(new_plane_state) /* Only to avoid unused-but-set-variable warning */, 1))

/**
* for_each_oldnew_private_obj_in_state - iterate over all private objects in an atomic update
--
2.17.1

[-- Attachment #4: 0001-drm-Add-drm_atomic_get_old-new_private_obj_state.patch --]
[-- Type: application/octet-stream, Size: 3338 bytes --]

From 8b9692b8b31b42ded4ce24733fb6f198c197ee87 Mon Sep 17 00:00:00 2001
From: "james qian wang (Arm Technology China)" <james.qian.wang@arm.com>
Date: Tue, 22 Jan 2019 11:05:41 +0000
Subject: [PATCH 1/3] drm: Add drm_atomic_get_old/new_private_obj_state

commit 9801a7eadf4cc29cdc90583a49f12decbb832ced upstream.

This pair of functions return the old/new private object state for the
given private_obj, or NULL if the private_obj is not part of the global
atomic state.

Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/gpu/drm/drm_atomic.c | 44 ++++++++++++++++++++++++++++++++++++
include/drm/drm_atomic.h | 6 +++++
2 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1a4b44923aec..5bbf969eb43a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1252,6 +1252,50 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_private_obj_state);

+/**
+ * drm_atomic_get_old_private_obj_state
+ * @state: global atomic state object
+ * @obj: private_obj to grab
+ *
+ * This function returns the old private object state for the given private_obj,
+ * or NULL if the private_obj is not part of the global atomic state.
+ */
+struct drm_private_state *
+drm_atomic_get_old_private_obj_state(struct drm_atomic_state *state,
+ struct drm_private_obj *obj)
+{
+ int i;
+
+ for (i = 0; i < state->num_private_objs; i++)
+ if (obj == state->private_objs[i].ptr)
+ return state->private_objs[i].old_state;
+
+ return NULL;
+}
+EXPORT_SYMBOL(drm_atomic_get_old_private_obj_state);
+
+/**
+ * drm_atomic_get_new_private_obj_state
+ * @state: global atomic state object
+ * @obj: private_obj to grab
+ *
+ * This function returns the new private object state for the given private_obj,
+ * or NULL if the private_obj is not part of the global atomic state.
+ */
+struct drm_private_state *
+drm_atomic_get_new_private_obj_state(struct drm_atomic_state *state,
+ struct drm_private_obj *obj)
+{
+ int i;
+
+ for (i = 0; i < state->num_private_objs; i++)
+ if (obj == state->private_objs[i].ptr)
+ return state->private_objs[i].new_state;
+
+ return NULL;
+}
+EXPORT_SYMBOL(drm_atomic_get_new_private_obj_state);
+
/**
* drm_atomic_get_connector_state - get connector state
* @state: global atomic state object
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 1e713154f00e..d2ff456ce72f 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -402,6 +402,12 @@ void drm_atomic_private_obj_fini(struct drm_private_obj *obj);
struct drm_private_state * __must_check
drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
struct drm_private_obj *obj);
+struct drm_private_state *
+drm_atomic_get_old_private_obj_state(struct drm_atomic_state *state,
+ struct drm_private_obj *obj);
+struct drm_private_state *
+drm_atomic_get_new_private_obj_state(struct drm_atomic_state *state,
+ struct drm_private_obj *obj);

/**
* drm_atomic_get_existing_crtc_state - get crtc state, if it exists
--
2.17.1

[-- Attachment #5: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5108): https://lists.cip-project.org/g/cip-dev/message/5108