* Re: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
2023-02-02 11:03 [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT Maxime Ripard
@ 2023-02-02 11:31 ` Maíra Canal
2023-02-02 12:36 ` Maxime Ripard
2023-02-02 12:22 ` Thomas Zimmermann
2023-02-02 13:07 ` kernel test robot
2 siblings, 1 reply; 9+ messages in thread
From: Maíra Canal @ 2023-02-02 11:31 UTC (permalink / raw)
To: Maxime Ripard, Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann
Cc: dri-devel
Hi Maxime,
On 2/2/23 08:03, Maxime Ripard wrote:
> Commit 8fc0380f6ba7 ("drm/client: Add some tests for
> drm_connector_pick_cmdline_mode()") was meant to introduce unit tests
> for the static drm_connector_pick_cmdline_mode() function.
>
> In such a case, the kunit documentation recommended to import the tests
> source file directly from the source file with the static function to
> test.
>
> While it was working, it's generally frowned upon. Fortunately, commit
> 9c988fae6f6a ("kunit: add macro to allow conditionally exposing static
> symbols to tests") introduced macros to easily deal with that case. We
> can thus remove our include and use those macros instead.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
> drivers/gpu/drm/drm_client_modeset.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 1b12a3c201a3..f48882941852 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -8,6 +8,9 @@
> */
>
> #include "drm/drm_modeset_lock.h"
> +
> +#include <kunit/visibility.h>
> +
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> @@ -159,7 +162,8 @@ drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int
> return NULL;
> }
>
> -static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_connector *connector)
> +VISIBLE_IF_KUNIT struct drm_display_mode *
> +drm_connector_pick_cmdline_mode(struct drm_connector *connector)
> {
> struct drm_cmdline_mode *cmdline_mode;
> struct drm_display_mode *mode;
> @@ -215,6 +219,7 @@ static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_conne
>
> return NULL;
> }
> +EXPORT_SYMBOL_IF_KUNIT(drm_connector_pick_cmdline_mode);
>
> static bool drm_connector_enabled(struct drm_connector *connector, bool strict)
> {
> @@ -1233,7 +1238,3 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
> return ret;
> }
> EXPORT_SYMBOL(drm_client_modeset_dpms);
> -
> -#ifdef CONFIG_DRM_KUNIT_TEST
> -#include "tests/drm_client_modeset_test.c"
> -#endif
As you removed this guard, you need to add drm_client_modeset_test.o
to tests/Makefile, otherwise, drm_client_modeset_test won't compile.
Best Regards,
- Maíra Canal
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
2023-02-02 11:31 ` Maíra Canal
@ 2023-02-02 12:36 ` Maxime Ripard
0 siblings, 0 replies; 9+ messages in thread
From: Maxime Ripard @ 2023-02-02 12:36 UTC (permalink / raw)
To: Maíra Canal
Cc: David Airlie, Daniel Vetter, dri-devel, Thomas Zimmermann
[-- Attachment #1: Type: text/plain, Size: 2670 bytes --]
On Thu, Feb 02, 2023 at 08:31:27AM -0300, Maíra Canal wrote:
> Hi Maxime,
>
> On 2/2/23 08:03, Maxime Ripard wrote:
> > Commit 8fc0380f6ba7 ("drm/client: Add some tests for
> > drm_connector_pick_cmdline_mode()") was meant to introduce unit tests
> > for the static drm_connector_pick_cmdline_mode() function.
> >
> > In such a case, the kunit documentation recommended to import the tests
> > source file directly from the source file with the static function to
> > test.
> >
> > While it was working, it's generally frowned upon. Fortunately, commit
> > 9c988fae6f6a ("kunit: add macro to allow conditionally exposing static
> > symbols to tests") introduced macros to easily deal with that case. We
> > can thus remove our include and use those macros instead.
> >
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> > drivers/gpu/drm/drm_client_modeset.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> > index 1b12a3c201a3..f48882941852 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -8,6 +8,9 @@
> > */
> > #include "drm/drm_modeset_lock.h"
> > +
> > +#include <kunit/visibility.h>
> > +
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > #include <linux/slab.h>
> > @@ -159,7 +162,8 @@ drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int
> > return NULL;
> > }
> > -static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_connector *connector)
> > +VISIBLE_IF_KUNIT struct drm_display_mode *
> > +drm_connector_pick_cmdline_mode(struct drm_connector *connector)
> > {
> > struct drm_cmdline_mode *cmdline_mode;
> > struct drm_display_mode *mode;
> > @@ -215,6 +219,7 @@ static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_conne
> > return NULL;
> > }
> > +EXPORT_SYMBOL_IF_KUNIT(drm_connector_pick_cmdline_mode);
> > static bool drm_connector_enabled(struct drm_connector *connector, bool strict)
> > {
> > @@ -1233,7 +1238,3 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
> > return ret;
> > }
> > EXPORT_SYMBOL(drm_client_modeset_dpms);
> > -
> > -#ifdef CONFIG_DRM_KUNIT_TEST
> > -#include "tests/drm_client_modeset_test.c"
> > -#endif
>
> As you removed this guard, you need to add drm_client_modeset_test.o
> to tests/Makefile, otherwise, drm_client_modeset_test won't compile.
Urgh, thanks for noticing this. I'll fix it up in a v2
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
2023-02-02 11:03 [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT Maxime Ripard
2023-02-02 11:31 ` Maíra Canal
@ 2023-02-02 12:22 ` Thomas Zimmermann
2023-02-02 12:35 ` Maxime Ripard
2023-02-02 13:07 ` kernel test robot
2 siblings, 1 reply; 9+ messages in thread
From: Thomas Zimmermann @ 2023-02-02 12:22 UTC (permalink / raw)
To: Maxime Ripard, Daniel Vetter, David Airlie, Maarten Lankhorst; +Cc: dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 3426 bytes --]
Hi
Am 02.02.23 um 12:03 schrieb Maxime Ripard:
> Commit 8fc0380f6ba7 ("drm/client: Add some tests for
> drm_connector_pick_cmdline_mode()") was meant to introduce unit tests
> for the static drm_connector_pick_cmdline_mode() function.
>
> In such a case, the kunit documentation recommended to import the tests
> source file directly from the source file with the static function to
> test.
>
> While it was working, it's generally frowned upon. Fortunately, commit
> 9c988fae6f6a ("kunit: add macro to allow conditionally exposing static
> symbols to tests") introduced macros to easily deal with that case. We
> can thus remove our include and use those macros instead.
I like that this include statements is going away. But changing symbol
visibility for tests is likewise awkward.
Maybe i'm askin gtoo miuch for this simple patch, but can't we have a
helper macro that generates an exported wrapper for Kunit tests?
Something like this:
EXPORT_KUNIT_WRAPPER(struct drm_display_mode *\
drm_connector_pick_cmdline_mode,
struct drm_connector *connector);
which then generates something like this:
struct drm_display_mode * drm_connector_pick_cmdline_mode_kunit(
struct drm_connector *connector)
{
return drm_connector_pick_cmdline_mode(connector);
}
I know that the macro for generating this code is more complex than
illustrated here. But this solution separates Kunit and functions
cleanly. The static functions that are exported for Kunit testing still
need to be declared in a header file. That could also be done via such a
macro.
Best regards
Thomas
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
> drivers/gpu/drm/drm_client_modeset.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 1b12a3c201a3..f48882941852 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -8,6 +8,9 @@
> */
>
> #include "drm/drm_modeset_lock.h"
> +
> +#include <kunit/visibility.h>
> +
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> @@ -159,7 +162,8 @@ drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int
> return NULL;
> }
>
> -static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_connector *connector)
> +VISIBLE_IF_KUNIT struct drm_display_mode *
> +drm_connector_pick_cmdline_mode(struct drm_connector *connector)
> {
> struct drm_cmdline_mode *cmdline_mode;
> struct drm_display_mode *mode;
> @@ -215,6 +219,7 @@ static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_conne
>
> return NULL;
> }
> +EXPORT_SYMBOL_IF_KUNIT(drm_connector_pick_cmdline_mode);
>
> static bool drm_connector_enabled(struct drm_connector *connector, bool strict)
> {
> @@ -1233,7 +1238,3 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
> return ret;
> }
> EXPORT_SYMBOL(drm_client_modeset_dpms);
> -
> -#ifdef CONFIG_DRM_KUNIT_TEST
> -#include "tests/drm_client_modeset_test.c"
> -#endif
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
2023-02-02 12:22 ` Thomas Zimmermann
@ 2023-02-02 12:35 ` Maxime Ripard
2023-02-02 13:05 ` Thomas Zimmermann
0 siblings, 1 reply; 9+ messages in thread
From: Maxime Ripard @ 2023-02-02 12:35 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: David Airlie, Daniel Vetter, dri-devel
[-- Attachment #1: Type: text/plain, Size: 2061 bytes --]
Hi,
On Thu, Feb 02, 2023 at 01:22:01PM +0100, Thomas Zimmermann wrote:
> Am 02.02.23 um 12:03 schrieb Maxime Ripard:
> > Commit 8fc0380f6ba7 ("drm/client: Add some tests for
> > drm_connector_pick_cmdline_mode()") was meant to introduce unit tests
> > for the static drm_connector_pick_cmdline_mode() function.
> >
> > In such a case, the kunit documentation recommended to import the tests
> > source file directly from the source file with the static function to
> > test.
> >
> > While it was working, it's generally frowned upon. Fortunately, commit
> > 9c988fae6f6a ("kunit: add macro to allow conditionally exposing static
> > symbols to tests") introduced macros to easily deal with that case. We
> > can thus remove our include and use those macros instead.
>
> I like that this include statements is going away.
Yeah, when I saw that it was now available, I remembered you really
didn't like it :)
> But changing symbol visibility for tests is likewise awkward.
>
> Maybe i'm askin gtoo miuch for this simple patch, but can't we have a helper
> macro that generates an exported wrapper for Kunit tests? Something like
> this:
>
> EXPORT_KUNIT_WRAPPER(struct drm_display_mode *\
> drm_connector_pick_cmdline_mode,
> struct drm_connector *connector);
>
> which then generates something like this:
>
> struct drm_display_mode * drm_connector_pick_cmdline_mode_kunit(
> struct drm_connector *connector)
> {
> return drm_connector_pick_cmdline_mode(connector);
> }
>
> I know that the macro for generating this code is more complex than
> illustrated here. But this solution separates Kunit and functions cleanly.
> The static functions that are exported for Kunit testing still need to be
> declared in a header file. That could also be done via such a macro.
I mean, I guess we could do that, but what's the point? I don't really
get what that wrapper brings to the table.
Also, this deviates from the existing practice we had for selftests and
EXPORT_SYMBOL_FOR_TESTS_ONLY
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
2023-02-02 12:35 ` Maxime Ripard
@ 2023-02-02 13:05 ` Thomas Zimmermann
2023-02-09 9:30 ` Maxime Ripard
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Zimmermann @ 2023-02-02 13:05 UTC (permalink / raw)
To: Maxime Ripard; +Cc: David Airlie, Daniel Vetter, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2806 bytes --]
Hi
Am 02.02.23 um 13:35 schrieb Maxime Ripard:
> Hi,
>
> On Thu, Feb 02, 2023 at 01:22:01PM +0100, Thomas Zimmermann wrote:
>> Am 02.02.23 um 12:03 schrieb Maxime Ripard:
>>> Commit 8fc0380f6ba7 ("drm/client: Add some tests for
>>> drm_connector_pick_cmdline_mode()") was meant to introduce unit tests
>>> for the static drm_connector_pick_cmdline_mode() function.
>>>
>>> In such a case, the kunit documentation recommended to import the tests
>>> source file directly from the source file with the static function to
>>> test.
>>>
>>> While it was working, it's generally frowned upon. Fortunately, commit
>>> 9c988fae6f6a ("kunit: add macro to allow conditionally exposing static
>>> symbols to tests") introduced macros to easily deal with that case. We
>>> can thus remove our include and use those macros instead.
>>
>> I like that this include statements is going away.
>
> Yeah, when I saw that it was now available, I remembered you really
> didn't like it :)
>
>> But changing symbol visibility for tests is likewise awkward.
>>
>> Maybe i'm askin gtoo miuch for this simple patch, but can't we have a helper
>> macro that generates an exported wrapper for Kunit tests? Something like
>> this:
>>
>> EXPORT_KUNIT_WRAPPER(struct drm_display_mode *\
>> drm_connector_pick_cmdline_mode,
>> struct drm_connector *connector);
>>
>> which then generates something like this:
>>
>> struct drm_display_mode * drm_connector_pick_cmdline_mode_kunit(
>> struct drm_connector *connector)
>> {
>> return drm_connector_pick_cmdline_mode(connector);
>> }
>>
>> I know that the macro for generating this code is more complex than
>> illustrated here. But this solution separates Kunit and functions cleanly.
>> The static functions that are exported for Kunit testing still need to be
>> declared in a header file. That could also be done via such a macro.
>
> I mean, I guess we could do that, but what's the point? I don't really
> get what that wrapper brings to the table.
The big benefit of the kunit wrapper is that we don't change the
visibility or implementation of the tested code. The currently existing
macros invite linker errors because symbol visibility now depends on
whether Kunit it enabled. It's also not clear to me how Kunit knows the
symbol. Is there a function declaration in the Kunit test's source code?
If so, it might diverge from the implementation; with consequences.
Best regards
Thomas
>
> Also, this deviates from the existing practice we had for selftests and
> EXPORT_SYMBOL_FOR_TESTS_ONLY
>
> Maxime
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
2023-02-02 13:05 ` Thomas Zimmermann
@ 2023-02-09 9:30 ` Maxime Ripard
0 siblings, 0 replies; 9+ messages in thread
From: Maxime Ripard @ 2023-02-09 9:30 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: David Airlie, Daniel Vetter, dri-devel
[-- Attachment #1: Type: text/plain, Size: 3568 bytes --]
On Thu, Feb 02, 2023 at 02:05:14PM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 02.02.23 um 13:35 schrieb Maxime Ripard:
> > Hi,
> >
> > On Thu, Feb 02, 2023 at 01:22:01PM +0100, Thomas Zimmermann wrote:
> > > Am 02.02.23 um 12:03 schrieb Maxime Ripard:
> > > > Commit 8fc0380f6ba7 ("drm/client: Add some tests for
> > > > drm_connector_pick_cmdline_mode()") was meant to introduce unit tests
> > > > for the static drm_connector_pick_cmdline_mode() function.
> > > >
> > > > In such a case, the kunit documentation recommended to import the tests
> > > > source file directly from the source file with the static function to
> > > > test.
> > > >
> > > > While it was working, it's generally frowned upon. Fortunately, commit
> > > > 9c988fae6f6a ("kunit: add macro to allow conditionally exposing static
> > > > symbols to tests") introduced macros to easily deal with that case. We
> > > > can thus remove our include and use those macros instead.
> > >
> > > I like that this include statements is going away.
> >
> > Yeah, when I saw that it was now available, I remembered you really
> > didn't like it :)
> >
> > > But changing symbol visibility for tests is likewise awkward.
> > >
> > > Maybe i'm askin gtoo miuch for this simple patch, but can't we have a helper
> > > macro that generates an exported wrapper for Kunit tests? Something like
> > > this:
> > >
> > > EXPORT_KUNIT_WRAPPER(struct drm_display_mode *\
> > > drm_connector_pick_cmdline_mode,
> > > struct drm_connector *connector);
> > >
> > > which then generates something like this:
> > >
> > > struct drm_display_mode * drm_connector_pick_cmdline_mode_kunit(
> > > struct drm_connector *connector)
> > > {
> > > return drm_connector_pick_cmdline_mode(connector);
> > > }
> > >
> > > I know that the macro for generating this code is more complex than
> > > illustrated here. But this solution separates Kunit and functions cleanly.
> > > The static functions that are exported for Kunit testing still need to be
> > > declared in a header file. That could also be done via such a macro.
> >
> > I mean, I guess we could do that, but what's the point? I don't really
> > get what that wrapper brings to the table.
>
> The big benefit of the kunit wrapper is that we don't change the visibility
> or implementation of the tested code. The currently existing macros invite
> linker errors because symbol visibility now depends on whether Kunit it
> enabled.
Sure, it can happen, but saying that it encourages them is a stretch.
And fortunately, we have build bots to detect that.
The huge downside of the wrapper approach is that you're no longer
testing the function you want to test but something else, and you have
to trust that it's exactly the same thing.
And it's far from obvious that we're supposed to do that in the first
place, especially when everyone else is doing something else, and we're
doing it ourselves in a similar situation.
It's still fairly new in kunit, but if it was indeed creating any kind
of friction, I think EXPORT_SYMBOL_FOR_TESTS_ONLY would have been
reworked some time in the last 4 years.
> It's also not clear to me how Kunit knows the symbol. Is there a
> function declaration in the Kunit test's source code? If so, it might
> diverge from the implementation; with consequences.
apparmor has been using an internal header included in both the test
file and the implementation. That way, the compiler will make sure
there's no inconsistencies.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
2023-02-02 11:03 [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT Maxime Ripard
@ 2023-02-02 13:07 ` kernel test robot
2023-02-02 12:22 ` Thomas Zimmermann
2023-02-02 13:07 ` kernel test robot
2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-02-02 13:07 UTC (permalink / raw)
To: Maxime Ripard, Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann
Cc: oe-kbuild-all, dri-devel
Hi Maxime,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.2-rc6 next-20230202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Maxime-Ripard/drm-client-Convert-to-VISIBLE_IF_KUNIT/20230202-190453
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20230202110312.808607-1-maxime%40cerno.tech
patch subject: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230202/202302022027.LWmJQ4lL-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.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
# https://github.com/intel-lab-lkp/linux/commit/dd1e4c34178f4049f33e639350a6ef66ae9b5fd3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Maxime-Ripard/drm-client-Convert-to-VISIBLE_IF_KUNIT/20230202-190453
git checkout dd1e4c34178f4049f33e639350a6ef66ae9b5fd3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_client_modeset.c:166:1: warning: no previous prototype for 'drm_connector_pick_cmdline_mode' [-Wmissing-prototypes]
166 | drm_connector_pick_cmdline_mode(struct drm_connector *connector)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/drm_connector_pick_cmdline_mode +166 drivers/gpu/drm/drm_client_modeset.c
164
165 VISIBLE_IF_KUNIT struct drm_display_mode *
> 166 drm_connector_pick_cmdline_mode(struct drm_connector *connector)
167 {
168 struct drm_cmdline_mode *cmdline_mode;
169 struct drm_display_mode *mode;
170 bool prefer_non_interlace;
171
172 /*
173 * Find a user-defined mode. If the user gave us a valid
174 * mode on the kernel command line, it will show up in this
175 * list.
176 */
177
178 list_for_each_entry(mode, &connector->modes, head) {
179 if (mode->type & DRM_MODE_TYPE_USERDEF)
180 return mode;
181 }
182
183 cmdline_mode = &connector->cmdline_mode;
184 if (cmdline_mode->specified == false)
185 return NULL;
186
187 /*
188 * Attempt to find a matching mode in the list of modes we
189 * have gotten so far.
190 */
191
192 prefer_non_interlace = !cmdline_mode->interlace;
193 again:
194 list_for_each_entry(mode, &connector->modes, head) {
195 /* check width/height */
196 if (mode->hdisplay != cmdline_mode->xres ||
197 mode->vdisplay != cmdline_mode->yres)
198 continue;
199
200 if (cmdline_mode->refresh_specified) {
201 if (drm_mode_vrefresh(mode) != cmdline_mode->refresh)
202 continue;
203 }
204
205 if (cmdline_mode->interlace) {
206 if (!(mode->flags & DRM_MODE_FLAG_INTERLACE))
207 continue;
208 } else if (prefer_non_interlace) {
209 if (mode->flags & DRM_MODE_FLAG_INTERLACE)
210 continue;
211 }
212 return mode;
213 }
214
215 if (prefer_non_interlace) {
216 prefer_non_interlace = false;
217 goto again;
218 }
219
220 return NULL;
221 }
222 EXPORT_SYMBOL_IF_KUNIT(drm_connector_pick_cmdline_mode);
223
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
@ 2023-02-02 13:07 ` kernel test robot
0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-02-02 13:07 UTC (permalink / raw)
To: Maxime Ripard, Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann
Cc: dri-devel, oe-kbuild-all
Hi Maxime,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.2-rc6 next-20230202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Maxime-Ripard/drm-client-Convert-to-VISIBLE_IF_KUNIT/20230202-190453
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20230202110312.808607-1-maxime%40cerno.tech
patch subject: [PATCH] drm/client: Convert to VISIBLE_IF_KUNIT
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230202/202302022027.LWmJQ4lL-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.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
# https://github.com/intel-lab-lkp/linux/commit/dd1e4c34178f4049f33e639350a6ef66ae9b5fd3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Maxime-Ripard/drm-client-Convert-to-VISIBLE_IF_KUNIT/20230202-190453
git checkout dd1e4c34178f4049f33e639350a6ef66ae9b5fd3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_client_modeset.c:166:1: warning: no previous prototype for 'drm_connector_pick_cmdline_mode' [-Wmissing-prototypes]
166 | drm_connector_pick_cmdline_mode(struct drm_connector *connector)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/drm_connector_pick_cmdline_mode +166 drivers/gpu/drm/drm_client_modeset.c
164
165 VISIBLE_IF_KUNIT struct drm_display_mode *
> 166 drm_connector_pick_cmdline_mode(struct drm_connector *connector)
167 {
168 struct drm_cmdline_mode *cmdline_mode;
169 struct drm_display_mode *mode;
170 bool prefer_non_interlace;
171
172 /*
173 * Find a user-defined mode. If the user gave us a valid
174 * mode on the kernel command line, it will show up in this
175 * list.
176 */
177
178 list_for_each_entry(mode, &connector->modes, head) {
179 if (mode->type & DRM_MODE_TYPE_USERDEF)
180 return mode;
181 }
182
183 cmdline_mode = &connector->cmdline_mode;
184 if (cmdline_mode->specified == false)
185 return NULL;
186
187 /*
188 * Attempt to find a matching mode in the list of modes we
189 * have gotten so far.
190 */
191
192 prefer_non_interlace = !cmdline_mode->interlace;
193 again:
194 list_for_each_entry(mode, &connector->modes, head) {
195 /* check width/height */
196 if (mode->hdisplay != cmdline_mode->xres ||
197 mode->vdisplay != cmdline_mode->yres)
198 continue;
199
200 if (cmdline_mode->refresh_specified) {
201 if (drm_mode_vrefresh(mode) != cmdline_mode->refresh)
202 continue;
203 }
204
205 if (cmdline_mode->interlace) {
206 if (!(mode->flags & DRM_MODE_FLAG_INTERLACE))
207 continue;
208 } else if (prefer_non_interlace) {
209 if (mode->flags & DRM_MODE_FLAG_INTERLACE)
210 continue;
211 }
212 return mode;
213 }
214
215 if (prefer_non_interlace) {
216 prefer_non_interlace = false;
217 goto again;
218 }
219
220 return NULL;
221 }
222 EXPORT_SYMBOL_IF_KUNIT(drm_connector_pick_cmdline_mode);
223
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 9+ messages in thread