[PATCH V4 2/2] drm/vkms: Add support for virtual hardware mode

[Sumera Priyadarsini]

Add a virtual hardware or vblank-less mode as a module to
enable VKMS to emulate virtual graphic drivers.

Add a new drm_crtc_helper_funcs struct,
vkms_virtual_crtc_helper_funcs() which holds the atomic helpers
for virtual hardware mode. Change the existing
vkms_crtc_helper_funcs struct to vkms_vblank_crtc_helper_funcs
which holds atomic helpers for the vblank mode.
This makes the code flow clearer and easier to test
virtual hardware mode.

The first patch of this patchset added the function vkms_crtc_composer()
for plane composition which is used in case of vblank-less mode and
is directly called in the atomic hook in vkms_crtc_atomic_begin().
However, some crc captures still use vblanks which causes the crc-based
igt tests to crash. Currently, I am unsure about how to approach the
one-shot implementation of crc reads so I am still working on that.

This patchset has been tested with the igt tests- kms_writeback, kms_atomic,
kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking. This patchset must be tested after incorporating the
igt-tests patch:
https://lists.freedesktop.org/archives/igt-dev/2021-February/029355.html .

The patch is based on Rodrigo Siqueira's
patch(https://patchwork.freedesktop.org/patch/316851/?series=48469&rev=3)
and the ensuing review.

Signed-off-by: Sumera Priyadarsini 
---
Changes in V3:
- Refactor patchset(Melissa)
Changes in V2:
- Add atomic helper functions in a separate struct for virtual hardware
mode (Daniel)
- Remove spinlock across 'vkms_output->lock' in vkms_crtc.c(Daniel)
- Add vkms_composer_common() (Daniel)
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 51 +++-
 drivers/gpu/drm/vkms/vkms_drv.c  | 18 +++
 drivers/gpu/drm/vkms/vkms_drv.h  |  1 +
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 57bbd32e9beb..e6286f98d5b6 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -222,20 +222,20 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
return 0;
 }
 
-static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
-   struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_enable(struct drm_crtc *crtc,
+  struct drm_atomic_state *state)
 {
drm_crtc_vblank_on(crtc);
 }
 
-static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
-struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_disable(struct drm_crtc *crtc,
+   struct drm_atomic_state *state)
 {
drm_crtc_vblank_off(crtc);
 }
 
-static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
-  struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_begin(struct drm_crtc *crtc,
+   struct drm_atomic_state *state)
 {
struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
 
@@ -245,8 +245,8 @@ static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
spin_lock_irq(&vkms_output->lock);
 }
 
-static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
-  struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_flush(struct drm_crtc *crtc,
+   struct drm_atomic_state *state)
 {
struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
 
@@ -268,18 +268,38 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
spin_unlock_irq(&vkms_output->lock);
 }
 
-static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
+/*
+ * Crtc functions for virtual hardware/vblankless mode
+ */
+static void vkms_virtual_crtc_atomic_flush(struct drm_crtc *crtc,
+   struct drm_atomic_state *state)
+{
+   struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
+   struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(crtc->state);
+
+   vkms_crtc_composer(vkms_state);
+
+   vkms_output->composer_state = to_vkms_crtc_state(crtc->state);
+}
+
+static const struct drm_crtc_helper_funcs vkms_vblank_crtc_helper_funcs = {
.atomic_check   = vkms_crtc_atomic_check,
-   .atomic_begin   = vkms_crtc_atomic_begin,
-   .atomic_flush   = vkms_crtc_atomic_flush,
-   .atomic_enable  = vkms_crtc_atomic_enable,
-   .atomic_disable = vkms_crtc_atomic_disable,
+   .atomic_begin   = vkms_vblank_crtc_atomic_begin,
+   .atomic_flush   = vkms_vblank_crtc_atomic_flush,
+   .atomic_enable  = vkms_vblank_crtc_atomic_enable,
+   .atomic_disable = vkms_vblank_crtc_atomic_disable,
+};
+
+static const struct drm_crtc_helper_funcs vkms_virtual_crtc_helper_funcs = {
+   .atomic_check   = vkms_crtc_atomic_check,
+   .atomic_flush   = vkms_virtual_crtc_a

[PATCH V4 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode

[Sumera Priyadarsini]

Add two new functions vkms_composer_common() and vkms_crtc_composer().
The actual plane composition work has been moved to the helper function,
vkms_composer_common() which is called by both vkms_composer_worker()
and vkms_crtc_composer(). vkms_crtc_composer() can be used when we
implement virtual_hw mode.

Signed-off-by: Sumera Priyadarsini 
---
Changes in V4:
- Fix warning
Changes in V3:
- Refactor patchset (Melissa)
Change in V2:
- Add vkms_composer_common() (Daniel)
---
 drivers/gpu/drm/vkms/vkms_composer.c | 88 +---
 drivers/gpu/drm/vkms/vkms_drv.h  |  3 +
 2 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
index 66c6842d70db..0d2bad3ff849 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -169,6 +169,44 @@ static int compose_planes(void **vaddr_out,
return 0;
 }
 
+int vkms_composer_common(struct vkms_crtc_state *crtc_state,
+struct vkms_output *out, bool wb_pending, uint32_t 
*crc32)
+{
+   struct vkms_composer *primary_composer = NULL;
+   struct vkms_composer *cursor_composer = NULL;
+   void *vaddr_out = NULL;
+   int ret;
+
+   if (crtc_state->num_active_planes >= 1)
+   primary_composer = crtc_state->active_planes[0]->composer;
+   if (crtc_state->num_active_planes == 2)
+   cursor_composer = crtc_state->active_planes[1]->composer;
+   if (!primary_composer)
+   return -EINVAL;
+   if (wb_pending)
+   vaddr_out = crtc_state->active_writeback;
+
+   ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
+   if (ret) {
+   if (ret == -EINVAL && !wb_pending)
+   kfree(vaddr_out);
+   return -EINVAL;
+   }
+
+   *crc32 = compute_crc(vaddr_out, primary_composer);
+
+   if (wb_pending) {
+   drm_writeback_signal_completion(&out->wb_connector, 0);
+   spin_lock_irq(&out->composer_lock);
+   crtc_state->wb_pending = false;
+   spin_unlock_irq(&out->composer_lock);
+   } else {
+   kfree(vaddr_out);
+   }
+
+   return 0;
+}
+
 /**
  * vkms_composer_worker - ordered work_struct to compute CRC
  *
@@ -185,12 +223,9 @@ void vkms_composer_worker(struct work_struct *work)
composer_work);
struct drm_crtc *crtc = crtc_state->base.crtc;
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
-   struct vkms_composer *primary_composer = NULL;
-   struct vkms_composer *cursor_composer = NULL;
bool crc_pending, wb_pending;
-   void *vaddr_out = NULL;
-   u32 crc32 = 0;
u64 frame_start, frame_end;
+   u32 crc32 = 0;
int ret;
 
spin_lock_irq(&out->composer_lock);
@@ -210,36 +245,9 @@ void vkms_composer_worker(struct work_struct *work)
if (!crc_pending)
return;
 
-   if (crtc_state->num_active_planes >= 1)
-   primary_composer = crtc_state->active_planes[0]->composer;
-
-   if (crtc_state->num_active_planes == 2)
-   cursor_composer = crtc_state->active_planes[1]->composer;
-
-   if (!primary_composer)
-   return;
-
-   if (wb_pending)
-   vaddr_out = crtc_state->active_writeback;
-
-   ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
-   if (ret) {
-   if (ret == -EINVAL && !wb_pending)
-   kfree(vaddr_out);
+   ret = vkms_composer_common(crtc_state, out, wb_pending, &crc32);
+   if (ret == -EINVAL)
return;
-   }
-
-   crc32 = compute_crc(vaddr_out, primary_composer);
-
-   if (wb_pending) {
-   drm_writeback_signal_completion(&out->wb_connector, 0);
-   spin_lock_irq(&out->composer_lock);
-   crtc_state->wb_pending = false;
-   spin_unlock_irq(&out->composer_lock);
-   } else {
-   kfree(vaddr_out);
-   }
-
/*
 * The worker can fall behind the vblank hrtimer, make sure we catch up.
 */
@@ -247,6 +255,20 @@ void vkms_composer_worker(struct work_struct *work)
drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);
 }
 
+void vkms_crtc_composer(struct vkms_crtc_state *crtc_state)
+{
+   struct drm_crtc *crtc = crtc_state->base.crtc;
+   struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+   u32 crc32 = 0;
+   int ret;
+
+   ret = vkms_composer_common(crtc_state, out, crtc_state->wb_pending, 
&crc32);
+   if (ret == -EINVAL)
+   return;
+
+   drm_crtc_add_crc_entry(crtc, true, 0, &crc32);
+}
+
 stat

[PATCH V4 0/2] Add virtual hardware module

[Sumera Priyadarsini]

This patchset adds support for emulating virtual hardware with VKMS.
The virtual hardware mode can be enabled by using the following command
while loading the module:
sudo modprobe vkms enable_virtual_hw=1

The first patch is prep work for adding virtual_hw mode and refactors
the plane composition in vkms by adding a helper function vkms_composer_common()
which can be used for both vblank mode and virtual mode.

The second patch adds virtual hardware support as a module option. It
adds new atomic helper functions for the virtual mode
and modifies the existing atomic helpers for usage by the vblank mode
This gives us two sets of drm_crtc_helper_funcs struct for both modes,
making the code flow cleaner and easier to debug.

This patchset has been tested with the igt tests- kms_writeback, kms_atomic,
kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking. This patchset must be tested after incorporating the
igt-tests patch: 
https://lists.freedesktop.org/archives/igt-dev/2021-February/029355.html

Sumera Priyadarsini (2):
  drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode
  drm/vkms: Add support for virtual hardware mode

 drivers/gpu/drm/vkms/vkms_composer.c | 88 +---
 drivers/gpu/drm/vkms/vkms_crtc.c | 51 +++-
 drivers/gpu/drm/vkms/vkms_drv.c  | 18 --
 drivers/gpu/drm/vkms/vkms_drv.h  |  4 ++
 4 files changed, 109 insertions(+), 52 deletions(-)

-- 
2.25.1

Re: [PATCH V3 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode

[Sumera Priyadarsini]

On Sun, Apr 4, 2021 at 9:19 PM kernel test robot  wrote:
>
> Hi Sumera,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v5.12-rc5 next-20210401]
> [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]
>
> url:    
> https://github.com/0day-ci/linux/commits/Sumera-Priyadarsini/Add-virtual-hardware-module/20210404-211300
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
> 2023a53bdf41b7646b1d384b6816af06309f73a5
> config: mips-randconfig-r025-20210404 (attached as .config)
> compiler: mipsel-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
> # 
> https://github.com/0day-ci/linux/commit/4bd5c27357dd86b6099f3f28db5db722ceeed582
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review 
> Sumera-Priyadarsini/Add-virtual-hardware-module/20210404-211300
> git checkout 4bd5c27357dd86b6099f3f28db5db722ceeed582
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross 
> ARCH=mips
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot 
>
> All warnings (new ones prefixed by >>):
>
>drivers/gpu/drm/vkms/vkms_composer.c: In function 'vkms_composer_worker':
> >> drivers/gpu/drm/vkms/vkms_composer.c:226:20: warning: variable 
> >> 'wb_pending' set but not used [-Wunused-but-set-variable]
>  226 |  bool crc_pending, wb_pending;
>  |^~
>
>
> vim +/wb_pending +226 drivers/gpu/drm/vkms/vkms_composer.c
>
> 4bd5c27357dd86 drivers/gpu/drm/vkms/vkms_composer.c Sumera Priyadarsini 
> 2021-04-04  209
> 0ca33adb91c0a9 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-09-04  210  /**
> a4e7e98e90ebd9 drivers/gpu/drm/vkms/vkms_composer.c Rodrigo Siqueira
> 2019-06-25  211   * vkms_composer_worker - ordered work_struct to compute CRC
> 0ca33adb91c0a9 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-09-04  212   *
> 0ca33adb91c0a9 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-09-04  213   * @work: work_struct
> 0ca33adb91c0a9 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-09-04  214   *
> a4e7e98e90ebd9 drivers/gpu/drm/vkms/vkms_composer.c Rodrigo Siqueira
> 2019-06-25  215   * Work handler for composing and computing CRCs. 
> work_struct scheduled in
> 0ca33adb91c0a9 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-09-04  216   * an ordered workqueue that's periodically scheduled to run 
> by
> 0ca33adb91c0a9 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-09-04  217   * _vblank_handle() and flushed at 
> vkms_atomic_crtc_destroy_state().
> 0ca33adb91c0a9 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-09-04  218   */
> a4e7e98e90ebd9 drivers/gpu/drm/vkms/vkms_composer.c Rodrigo Siqueira
> 2019-06-25  219  void vkms_composer_worker(struct work_struct *work)
> 6c234fe37c5762 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-08-02  220  {
> 6c234fe37c5762 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-08-02  221 struct vkms_crtc_state *crtc_state = 
> container_of(work,
> 6c234fe37c5762 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-08-02  222 struct 
> vkms_crtc_state,
> a4e7e98e90ebd9 drivers/gpu/drm/vkms/vkms_composer.c Rodrigo Siqueira
> 2019-06-25  223 
> composer_work);
> 6c234fe37c5762 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-08-02  224 struct drm_crtc *crtc = crtc_state->base.crtc;
> 6c234fe37c5762 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-08-02  225 struct vkms_output *out = 
> drm_crtc_to_vkms_output(crtc);
> dbd9d80c1b2e03 drivers/gpu/drm/vkms/vkms_composer.c Rodrigo Siqueira
> 2020-08-30 @226 bool crc_pending, wb_pending;
> 0ca33adb91c0a9 drivers/gpu/drm/vkms/vkms_crc.c  Haneen Mohammed 
> 2018-09-04  227 u64 frame_start, frame_end;
> 4bd5c27357dd86 drivers/gpu/drm/vkms/vkms_composer.c Sumera Priyadarsini 
> 2021-04-04  228 u32 crc32 = 0;
> 953025763d1421 drivers/gpu/drm/vkms/v

[PATCH V3 2/2] drm/vkms: Add support for virtual hardware mode

[Sumera Priyadarsini]

Add a virtual hardware or vblank-less mode as a module to
enable VKMS to emulate virtual graphic drivers.

Add a new drm_crtc_helper_funcs struct,
vkms_virtual_crtc_helper_funcs() which holds the atomic helpers
for virtual hardware mode. Change the existing
vkms_crtc_helper_funcs struct to vkms_vblank_crtc_helper_funcs
which holds atomic helpers for the vblank mode.
This makes the code flow clearer and easier to test
virtual hardware mode.

The first patch of this patchset added the function vkms_crtc_composer()
for plane composition which is used in case of vblank-less mode and
is directly called in the atomic hook in vkms_crtc_atomic_begin().
However, some crc captures still use vblanks which causes the crc-based
igt tests to crash. Currently, I am unsure about how to approach the
one-shot implementation of crc reads so I am still working on that.

This patchset has been tested with the igt tests- kms_writeback, kms_atomic,
kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking. This patchset must be tested after incorporating the
igt-tests patch:
https://lists.freedesktop.org/archives/igt-dev/2021-February/029355.html .

The patch is based on Rodrigo Siqueira's
patch(https://patchwork.freedesktop.org/patch/316851/?series=48469&rev=3)
and the ensuing review.

Signed-off-by: Sumera Priyadarsini 
---
Changes in V3:
- Refactor patchset(Melissa)
Changes in V2:
- Add atomic helper functions in a separate struct for virtual hardware
mode (Daniel)
- Remove spinlock across 'vkms_output->lock' in vkms_crtc.c(Daniel)
- Add vkms_composer_common() (Daniel)
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 51 +++-
 drivers/gpu/drm/vkms/vkms_drv.c  | 18 +++
 drivers/gpu/drm/vkms/vkms_drv.h  |  1 +
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 57bbd32e9beb..e6286f98d5b6 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -222,20 +222,20 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
return 0;
 }
 
-static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
-   struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_enable(struct drm_crtc *crtc,
+  struct drm_atomic_state *state)
 {
drm_crtc_vblank_on(crtc);
 }
 
-static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
-struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_disable(struct drm_crtc *crtc,
+   struct drm_atomic_state *state)
 {
drm_crtc_vblank_off(crtc);
 }
 
-static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
-  struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_begin(struct drm_crtc *crtc,
+   struct drm_atomic_state *state)
 {
struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
 
@@ -245,8 +245,8 @@ static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
spin_lock_irq(&vkms_output->lock);
 }
 
-static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
-  struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_flush(struct drm_crtc *crtc,
+   struct drm_atomic_state *state)
 {
struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
 
@@ -268,18 +268,38 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
spin_unlock_irq(&vkms_output->lock);
 }
 
-static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
+/*
+ * Crtc functions for virtual hardware/vblankless mode
+ */
+static void vkms_virtual_crtc_atomic_flush(struct drm_crtc *crtc,
+   struct drm_atomic_state *state)
+{
+   struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
+   struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(crtc->state);
+
+   vkms_crtc_composer(vkms_state);
+
+   vkms_output->composer_state = to_vkms_crtc_state(crtc->state);
+}
+
+static const struct drm_crtc_helper_funcs vkms_vblank_crtc_helper_funcs = {
.atomic_check   = vkms_crtc_atomic_check,
-   .atomic_begin   = vkms_crtc_atomic_begin,
-   .atomic_flush   = vkms_crtc_atomic_flush,
-   .atomic_enable  = vkms_crtc_atomic_enable,
-   .atomic_disable = vkms_crtc_atomic_disable,
+   .atomic_begin   = vkms_vblank_crtc_atomic_begin,
+   .atomic_flush   = vkms_vblank_crtc_atomic_flush,
+   .atomic_enable  = vkms_vblank_crtc_atomic_enable,
+   .atomic_disable = vkms_vblank_crtc_atomic_disable,
+};
+
+static const struct drm_crtc_helper_funcs vkms_virtual_crtc_helper_funcs = {
+   .atomic_check   = vkms_crtc_atomic_check,
+   .atomic_flush   = vkms_virtual_crtc_a

[PATCH V3 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode

[Sumera Priyadarsini]

Add two new functions vkms_composer_common() and vkms_crtc_composer().
The actual plane composition work has been moved to the helper function,
vkms_composer_common() which is called by both vkms_composer_worker()
and vkms_crtc_composer(). vkms_crtc_composer() can be used when we
implement virtual_hw mode.

Signed-off-by: Sumera Priyadarsini 
---

Changes in V3:
- Refactor patchset (Melissa)
Change in V2:
- Add vkms_composer_common() (Daniel)
---
 drivers/gpu/drm/vkms/vkms_composer.c | 88 +---
 drivers/gpu/drm/vkms/vkms_drv.h  |  3 +
 2 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
index 66c6842d70db..91b33c9c2f47 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -169,6 +169,44 @@ static int compose_planes(void **vaddr_out,
return 0;
 }
 
+int vkms_composer_common(struct vkms_crtc_state *crtc_state,
+struct vkms_output *out, bool wb_pending, uint32_t 
*crc32)
+{
+   struct vkms_composer *primary_composer = NULL;
+   struct vkms_composer *cursor_composer = NULL;
+   void *vaddr_out = NULL;
+   int ret;
+
+   if (crtc_state->num_active_planes >= 1)
+   primary_composer = crtc_state->active_planes[0]->composer;
+   if (crtc_state->num_active_planes == 2)
+   cursor_composer = crtc_state->active_planes[1]->composer;
+   if (!primary_composer)
+   return -EINVAL;
+   if (wb_pending)
+   vaddr_out = crtc_state->active_writeback;
+
+   ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
+   if (ret) {
+   if (ret == -EINVAL && !wb_pending)
+   kfree(vaddr_out);
+   return -EINVAL;
+   }
+
+   *crc32 = compute_crc(vaddr_out, primary_composer);
+
+   if (wb_pending) {
+   drm_writeback_signal_completion(&out->wb_connector, 0);
+   spin_lock_irq(&out->composer_lock);
+   crtc_state->wb_pending = false;
+   spin_unlock_irq(&out->composer_lock);
+   } else {
+   kfree(vaddr_out);
+   }
+
+   return 0;
+}
+
 /**
  * vkms_composer_worker - ordered work_struct to compute CRC
  *
@@ -185,12 +223,9 @@ void vkms_composer_worker(struct work_struct *work)
composer_work);
struct drm_crtc *crtc = crtc_state->base.crtc;
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
-   struct vkms_composer *primary_composer = NULL;
-   struct vkms_composer *cursor_composer = NULL;
bool crc_pending, wb_pending;
-   void *vaddr_out = NULL;
-   u32 crc32 = 0;
u64 frame_start, frame_end;
+   u32 crc32 = 0;
int ret;
 
spin_lock_irq(&out->composer_lock);
@@ -210,36 +245,9 @@ void vkms_composer_worker(struct work_struct *work)
if (!crc_pending)
return;
 
-   if (crtc_state->num_active_planes >= 1)
-   primary_composer = crtc_state->active_planes[0]->composer;
-
-   if (crtc_state->num_active_planes == 2)
-   cursor_composer = crtc_state->active_planes[1]->composer;
-
-   if (!primary_composer)
-   return;
-
-   if (wb_pending)
-   vaddr_out = crtc_state->active_writeback;
-
-   ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
-   if (ret) {
-   if (ret == -EINVAL && !wb_pending)
-   kfree(vaddr_out);
+   ret = vkms_composer_common(crtc_state, out, crtc_state->wb_pending, 
&crc32);
+   if (ret == -EINVAL)
return;
-   }
-
-   crc32 = compute_crc(vaddr_out, primary_composer);
-
-   if (wb_pending) {
-   drm_writeback_signal_completion(&out->wb_connector, 0);
-   spin_lock_irq(&out->composer_lock);
-   crtc_state->wb_pending = false;
-   spin_unlock_irq(&out->composer_lock);
-   } else {
-   kfree(vaddr_out);
-   }
-
/*
 * The worker can fall behind the vblank hrtimer, make sure we catch up.
 */
@@ -247,6 +255,20 @@ void vkms_composer_worker(struct work_struct *work)
drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);
 }
 
+void vkms_crtc_composer(struct vkms_crtc_state *crtc_state)
+{
+   struct drm_crtc *crtc = crtc_state->base.crtc;
+   struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+   u32 crc32 = 0;
+   int ret;
+
+   ret = vkms_composer_common(crtc_state, out, crtc_state->wb_pending, 
&crc32);
+   if (ret == -EINVAL)
+   return;
+
+   drm_crtc_add_crc_entry(crtc, true, 0, &crc32);
+}
+
 static const c

[PATCH V3 0/2] Add virtual hardware module

[Sumera Priyadarsini]

This patchset adds support for emulating virtual hardware with VKMS.
The virtual hardware mode can be enabled by using the following command
while loading the module:
sudo modprobe vkms enable_virtual_hw=1

The first patch is prep work for adding virtual_hw mode and refactors
the plane composition in vkms by adding a helper function vkms_composer_common()
which can be used for both vblank mode and virtual mode.

The second patch adds virtual hardware support as a module option. The
second patch adds new atomic helper functions for the virtual mode
and modifies the existing atomic helpers for usage by the vblank mode
This gives us two sets of drm_crtc_helper_funcs struct for both modes,
making the code flow cleaner and easier to debug.

This patchset has been tested with the igt tests, kms_writeback, kms_atomic,
kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking. This patchset must be tested after incorporating the
igt-tests patch: 
https://lists.freedesktop.org/archives/igt-dev/2021-February/029355.html 

Sumera Priyadarsini (2):
  drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode
  drm/vkms: Add support for virtual hardware mode

 drivers/gpu/drm/vkms/vkms_composer.c | 88 +---
 drivers/gpu/drm/vkms/vkms_crtc.c | 51 +++-
 drivers/gpu/drm/vkms/vkms_drv.c  | 18 --
 drivers/gpu/drm/vkms/vkms_drv.h  |  4 ++
 4 files changed, 109 insertions(+), 52 deletions(-)

-- 
2.25.1

[PATCH V2 2/2] drm/vkms: Add crtc atomic helper functions for virtual mode

[Sumera Priyadarsini]

Add a new drm_crtc_helper_funcs struct,
vkms_virtual_crtc_helper_funcs() which holds the atomic helpers
for virtual hardware mode. Change the existing
vkms_crtc_helper_funcs struct to vkms_vblank_crtc_helper_funcs
which holds atomic helpers for the vblank mode.

This patch makes the code flow clearer and easier to test
virtual hardware mode.

Signed-off-by: Sumera Priyadarsini 
---

Changes in V2:
- Add atomic helper functions in a separate struct for virtual hardware
mode (Daniel)
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 69 ++--
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 6cc8dc23bd5d..7d5562ab5ce6 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -221,48 +221,35 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
return 0;
 }
 
-static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
-   struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_enable(struct drm_crtc *crtc,
+struct drm_atomic_state *state)
 {
-   struct vkms_device *vkmsdev = drm_device_to_vkms_device(crtc->dev);
-
-   if (!vkmsdev->config->virtual_hw)
-   drm_crtc_vblank_on(crtc);
+   drm_crtc_vblank_on(crtc);
 }
 
-static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
+static void vkms_vblank_crtc_atomic_disable(struct drm_crtc *crtc,
 struct drm_atomic_state *state)
 {
-   struct vkms_device *vkmsdev = drm_device_to_vkms_device(crtc->dev);
-
-   if (!vkmsdev->config->virtual_hw)
-   drm_crtc_vblank_off(crtc);
+   drm_crtc_vblank_off(crtc);
 }
 
-static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
+static void vkms_vblank_crtc_atomic_begin(struct drm_crtc *crtc,
   struct drm_atomic_state *state)
 {
struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
-   struct vkms_device *vkmsdev = drm_device_to_vkms_device(crtc->dev);
 
/* This lock is held across the atomic commit to block vblank timer
 * from scheduling vkms_composer_worker until the composer is updated
 */
-   if (!vkmsdev->config->virtual_hw)
-   spin_lock_irq(&vkms_output->lock);
+   spin_lock_irq(&vkms_output->lock);
 }
 
-static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
+static void vkms_vblank_crtc_atomic_flush(struct drm_crtc *crtc,
   struct drm_atomic_state *state)
 {
struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
-   struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(crtc->state);
-   struct vkms_device *vkmsdev = drm_device_to_vkms_device(crtc->dev);
-
-   if (vkmsdev->config->virtual_hw)
-   vkms_crtc_composer(vkms_state);
 
-   if (crtc->state->event && !vkmsdev->config->virtual_hw) {
+   if (crtc->state->event) {
spin_lock(&crtc->dev->event_lock);
 
if (drm_crtc_vblank_get(crtc) != 0)
@@ -277,22 +264,41 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
 
vkms_output->composer_state = to_vkms_crtc_state(crtc->state);
 
-   if (!vkmsdev->config->virtual_hw)
-   spin_unlock_irq(&vkms_output->lock);
+   spin_unlock_irq(&vkms_output->lock);
 }
 
-static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
+/*
+ * Crtc functions for virtual hardware/vblankless mode
+ */
+static void vkms_virtual_crtc_atomic_flush(struct drm_crtc *crtc,
+  struct drm_atomic_state *state)
+{
+   struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
+   struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(crtc->state);
+
+   vkms_crtc_composer(vkms_state);
+
+   vkms_output->composer_state = to_vkms_crtc_state(crtc->state);
+}
+
+static const struct drm_crtc_helper_funcs vkms_vblank_crtc_helper_funcs = {
.atomic_check   = vkms_crtc_atomic_check,
-   .atomic_begin   = vkms_crtc_atomic_begin,
-   .atomic_flush   = vkms_crtc_atomic_flush,
-   .atomic_enable  = vkms_crtc_atomic_enable,
-   .atomic_disable = vkms_crtc_atomic_disable,
+   .atomic_begin   = vkms_vblank_crtc_atomic_begin,
+   .atomic_flush   = vkms_vblank_crtc_atomic_flush,
+   .atomic_enable  = vkms_vblank_crtc_atomic_enable,
+   .atomic_disable = vkms_vblank_crtc_atomic_disable,
+};
+
+static const struct drm_crtc_helper_funcs vkms_virtual_crtc_helper_funcs = {
+   .atomic_check   = vkms_crtc_atomic_check,
+   .atomic_flush   = vkms_virtual_crtc_atomic_flush,
 };
 
 int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
   struct drm_plane

[PATCH V2 1/2] drm/vkms: Add support for virtual hardware mode

[Sumera Priyadarsini]

Add a virtual hardware or vblank-less mode as a module to enable
VKMS to emulate virtual graphic drivers.

Two new functions vkms_composer_common() and vkms_crtc_composer() have
been added. The actual plane composition work has been moved to
vkms_composer_common() which is called by both vkms_composer_worker()
and vkms_crtc_composer(). vkms_crtc_composer() is used in case of
vblank mode and is called directly in the atomic hook in
vkms_atomic_begin(). However, some crc captures still use vblanks
which causes the crc-based igt tests to crash. Currently, I am unsure
about how to approach one-shot implementation of crc reads so I am
still working on that.

The patch is based on Rodrigo Siqueira's
patch(https://patchwork.freedesktop.org/patch/316851/?series=48469&rev=3)
and the ensuing review.

Signed-off-by: Sumera Priyadarsini 

Changes in V2:
- Remove spinlock across 'vkms_output->lock' in vkms_crtc.c(Daniel)
- Add vkms_composer_common() (Daniel)
---
 drivers/gpu/drm/vkms/vkms_composer.c | 88 +---
 drivers/gpu/drm/vkms/vkms_crtc.c | 24 ++--
 drivers/gpu/drm/vkms/vkms_drv.c  | 18 --
 drivers/gpu/drm/vkms/vkms_drv.h  |  4 ++
 4 files changed, 91 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
index 66c6842d70db..df5b946f001f 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -169,6 +169,44 @@ static int compose_planes(void **vaddr_out,
return 0;
 }
 
+int vkms_composer_common(struct vkms_crtc_state *crtc_state,
+ struct vkms_output *out, bool wb_pending, uint32_t 
*crc32)
+{
+   struct vkms_composer *primary_composer = NULL;
+   struct vkms_composer *cursor_composer = NULL;
+   void *vaddr_out = NULL;
+   int ret;
+
+   if (crtc_state->num_active_planes >= 1)
+   primary_composer = crtc_state->active_planes[0]->composer;
+   if (crtc_state->num_active_planes == 2)
+   cursor_composer = crtc_state->active_planes[1]->composer;
+   if (!primary_composer)
+   return -EINVAL;
+   if (wb_pending)
+   vaddr_out = crtc_state->active_writeback;
+
+   ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
+   if (ret) {
+   if (ret == -EINVAL && !wb_pending)
+   kfree(vaddr_out);
+   return -EINVAL;
+   }
+
+   *crc32 = compute_crc(vaddr_out, primary_composer);
+
+   if (wb_pending) {
+   drm_writeback_signal_completion(&out->wb_connector, 0);
+   spin_lock_irq(&out->composer_lock);
+   crtc_state->wb_pending = false;
+   spin_unlock_irq(&out->composer_lock);
+   } else {
+   kfree(vaddr_out);
+   }
+
+   return 0;
+}
+
 /**
  * vkms_composer_worker - ordered work_struct to compute CRC
  *
@@ -185,12 +223,9 @@ void vkms_composer_worker(struct work_struct *work)
composer_work);
struct drm_crtc *crtc = crtc_state->base.crtc;
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
-   struct vkms_composer *primary_composer = NULL;
-   struct vkms_composer *cursor_composer = NULL;
bool crc_pending, wb_pending;
-   void *vaddr_out = NULL;
-   u32 crc32 = 0;
u64 frame_start, frame_end;
+   u32 crc32 = 0;
int ret;
 
spin_lock_irq(&out->composer_lock);
@@ -210,36 +245,9 @@ void vkms_composer_worker(struct work_struct *work)
if (!crc_pending)
return;
 
-   if (crtc_state->num_active_planes >= 1)
-   primary_composer = crtc_state->active_planes[0]->composer;
-
-   if (crtc_state->num_active_planes == 2)
-   cursor_composer = crtc_state->active_planes[1]->composer;
-
-   if (!primary_composer)
-   return;
-
-   if (wb_pending)
-   vaddr_out = crtc_state->active_writeback;
-
-   ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
-   if (ret) {
-   if (ret == -EINVAL && !wb_pending)
-   kfree(vaddr_out);
+   ret = vkms_composer_common(crtc_state, out, crtc_state->wb_pending, 
&crc32);
+   if (ret == -EINVAL)
return;
-   }
-
-   crc32 = compute_crc(vaddr_out, primary_composer);
-
-   if (wb_pending) {
-   drm_writeback_signal_completion(&out->wb_connector, 0);
-   spin_lock_irq(&out->composer_lock);
-   crtc_state->wb_pending = false;
-   spin_unlock_irq(&out->composer_lock);
-   } else {
-   kfree(vaddr_out);
-   }
-
/*
 * The worker can fall behind the vblank hrtimer

[PATCH V2 0/2] Add virtual hardware module

[Sumera Priyadarsini]

This patchset adds support for emulating virtual hardware with VKMS.
The virtual hardware mode can be enabled by using the following command
while loading the module:
sudo modprobe vkms enable_virtual_hw=1

The first patch adds virtual hardware support as a module option. The
second patch adds new atomic helper functions for the virtual mode
and modifies the existing atomic helpers for usage by the vblank mode
This gives us two sets of drm_crtc_helper_funcs struct for both modes,
making the code flow cleaner and easier to debug.

This patchset has been tested with the igt tests, kms_writeback, kms_atomic,
kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking. This patchset must be tested after incorporating the
igt-tests patch: 
https://lists.freedesktop.org/archives/igt-dev/2021-February/029355.html .

Sumera Priyadarsini (2):
  drm/vkms: Add support for virtual hardware mode
  drm/vkms: Add crtc atomic helper functions for virtual mode

 drivers/gpu/drm/vkms/vkms_composer.c | 88 +---
 drivers/gpu/drm/vkms/vkms_crtc.c | 45 ++
 drivers/gpu/drm/vkms/vkms_drv.c  | 18 --
 drivers/gpu/drm/vkms/vkms_drv.h  |  4 ++
 4 files changed, 106 insertions(+), 49 deletions(-)

-- 
2.25.1

[RFC PATCH] drm/vkms: Add support for virtual hardware mode

[Sumera Priyadarsini]

Add a virtual hardware or vblank-less mode as a module to enable
VKMS to emulate virtual graphic drivers. This mode can be enabled
by setting enable_virtual_hw=1 at the time of loading VKMS.

A new function vkms_crtc_composer() has been added to bypass the
vblank mode and is called directly in the atomic hook in
vkms_atomic_begin(). However, some crc captures still use vblanks
which causes the crc-based igt tests to crash. Currently, I am unsure
about how to approach one-shot implementation of crc reads so I am
still working on that.

This patch has been tested with the igt tests, kms_writeback, kms_atomic,
kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking. This patch must be tested after incorporating the
igt-tests patch: 
https://lists.freedesktop.org/archives/igt-dev/2021-February/029355.html .

The patch is based on Rodrigo Siqueira's
patch(https://patchwork.freedesktop.org/patch/316851/?series=48469&rev=3)
and the ensuing review.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/vkms/vkms_composer.c | 46 
 drivers/gpu/drm/vkms/vkms_crtc.c | 17 --
 drivers/gpu/drm/vkms/vkms_drv.c  | 18 ---
 drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
index 66c6842d70db..7a8aaf5c 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -247,6 +247,52 @@ void vkms_composer_worker(struct work_struct *work)
drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);
 }
 
+void vkms_crtc_composer(struct vkms_crtc_state *crtc_state)
+{
+   struct drm_crtc *crtc = crtc_state->base.crtc;
+   struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+   struct vkms_composer *primary_composer = NULL;
+   struct vkms_composer *cursor_composer = NULL;
+   void *vaddr_out = NULL;
+   u32 crc32 = 0;
+   int ret;
+   bool wb_pending;
+
+   wb_pending = crtc_state->wb_pending;
+
+   if (crtc_state->num_active_planes >= 1)
+   primary_composer = crtc_state->active_planes[0]->composer;
+
+   if (crtc_state->num_active_planes == 2)
+   cursor_composer = crtc_state->active_planes[1]->composer;
+
+   if (!primary_composer)
+   return;
+
+   if (wb_pending)
+   vaddr_out = crtc_state->active_writeback;
+
+   ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
+   if (ret) {
+   if (ret == -EINVAL && !wb_pending)
+   kfree(vaddr_out);
+   return;
+   }
+
+   crc32 = compute_crc(vaddr_out, primary_composer);
+
+   if (wb_pending) {
+   drm_writeback_signal_completion(&out->wb_connector, 0);
+   spin_lock_irq(&out->composer_lock);
+   crtc_state->wb_pending = false;
+   spin_unlock_irq(&out->composer_lock);
+   } else {
+   kfree(vaddr_out);
+   }
+
+   drm_crtc_add_crc_entry(crtc, true, 0, &crc32);
+}
+
 static const char * const pipe_crc_sources[] = {"auto"};
 
 const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 6164349cdf11..38de791a4882 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -224,13 +224,19 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
 static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_state *state)
 {
-   drm_crtc_vblank_on(crtc);
+   struct vkms_device *vkmsdev = drm_device_to_vkms_device(crtc->dev);
+
+   if (!vkmsdev->config->virtual_hw)
+   drm_crtc_vblank_on(crtc);
 }
 
 static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
 struct drm_atomic_state *state)
 {
-   drm_crtc_vblank_off(crtc);
+   struct vkms_device *vkmsdev = drm_device_to_vkms_device(crtc->dev);
+
+   if (!vkmsdev->config->virtual_hw)
+   drm_crtc_vblank_off(crtc);
 }
 
 static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
@@ -248,8 +254,13 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
   struct drm_atomic_state *state)
 {
struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
+   struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(crtc->state);
+   struct vkms_device *vkmsdev = drm_device_to_vkms_device(crtc->dev);
+
+   if (vkmsdev->config->virtual_hw)
+   vkms_crtc_composer(vkms_state);
 
-   if (crtc->state->event) {
+   

Re: [PATCH][next] drm/vkms: Fix missing kmalloc allocation failure check

[Sumera Priyadarsini]

On Fri, Jan 15, 2021 at 6:39 PM Colin King  wrote:
>
> From: Colin Ian King 
>
> Currently the kmalloc allocation for config is not being null
> checked and could potentially lead to a null pointer dereference.
> Fix this by adding the missing null check.
>
> Addresses-Coverity: ("Dereference null return value")
> Fixes: 2df7af93fdad ("drm/vkms: Add vkms_config type")
> Signed-off-by: Colin Ian King 

Good catch, thank you!

Reviewed-by: Sumera Priyadarsini 
> ---
>  drivers/gpu/drm/vkms/vkms_drv.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 708f7f54001d..2173b82606f6 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -188,7 +188,11 @@ static int vkms_create(struct vkms_config *config)
>
>  static int __init vkms_init(void)
>  {
> -   struct vkms_config *config = kmalloc(sizeof(*config), GFP_KERNEL);
> +   struct vkms_config *config;
> +
> +   config = kmalloc(sizeof(*config), GFP_KERNEL);
> +   if (!config)
> +   return -ENOMEM;
>
> default_config = config;
>
> --
> 2.29.2
>
regards,
Sumera

[PATCH] drm/vblank: Fix typo in docs

[Sumera Priyadarsini]

Fix typo in intro chapter in drm_vblank.c.
Change 'sacn' to 'scan'.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/drm_vblank.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index d30e2f2b8f3c..30912d8f82a5 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -74,7 +74,7 @@
  *||   updates the
  *||   frame as it
  *||   travels down
- *||   ("sacn out")
+ *||   ("scan out")
  *|   Old frame|
  *||
  *||
-- 
2.25.1

[PATCH V5 3/3] drm/vkms: Add information about module options

[Sumera Priyadarsini]

Update vkms documentation to contain usage of `modinfo`
command and steps to load vkms with module options enabled.

Signed-off-by: Sumera Priyadarsini 
---
 Documentation/gpu/vkms.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 9e030c74a82e..2c9b376da5ca 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -35,6 +35,18 @@ Now, to load the driver, use::
 On running the lsmod command now, the VKMS driver will appear listed.
 You can also observe the driver being loaded in the dmesg logs.
 
+The VKMS driver has optional features to simulate different kinds of hardware,
+which are exposed as module options. You can use the `modinfo` command
+to see the module options for vkms::
+
+  modinfo vkms
+
+Module options are helpful when testing, and enabling modules
+can be done while loading vkms. For example, to load vkms with cursor enabled,
+use::
+
+  sudo modprobe vkms enable_cursor=1
+
 To disable the driver, use ::
 
   sudo modprobe -r vkms
-- 
2.25.1

[PATCH V5 2/3] drm/vkms: Add support for writeback module

[Sumera Priyadarsini]

Add enable_writeback feature to vkms_config as a module.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/vkms/vkms_drv.c| 5 +
 drivers/gpu/drm/vkms/vkms_drv.h| 1 +
 drivers/gpu/drm/vkms/vkms_output.c | 9 ++---
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 6b33975a5cb2..708f7f54001d 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -40,6 +40,10 @@ static bool enable_cursor = true;
 module_param_named(enable_cursor, enable_cursor, bool, 0444);
 MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
 
+static bool enable_writeback = true;
+module_param_named(enable_writeback, enable_writeback, bool, 0444);
+MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector 
support");
+
 DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
 
 static void vkms_release(struct drm_device *dev)
@@ -189,6 +193,7 @@ static int __init vkms_init(void)
default_config = config;
 
config->cursor = enable_cursor;
+   config->writeback = enable_writeback;
 
return vkms_create(config);
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 6a27bd8875f2..b9b4e2bc11c0 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -83,6 +83,7 @@ struct vkms_output {
 struct vkms_device;
 
 struct vkms_config {
+   bool writeback;
bool cursor;
/* only set when instantiated */
struct vkms_device *dev;
diff --git a/drivers/gpu/drm/vkms/vkms_output.c 
b/drivers/gpu/drm/vkms/vkms_output.c
index 8f3ffb28b9d1..f5f6f15c362c 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -41,6 +41,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
struct drm_crtc *crtc = &output->crtc;
struct drm_plane *primary, *cursor = NULL;
int ret;
+   int writeback;
 
primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
if (IS_ERR(primary))
@@ -80,9 +81,11 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
goto err_attach;
}
 
-   ret = vkms_enable_writeback_connector(vkmsdev);
-   if (ret)
-   DRM_ERROR("Failed to init writeback connector\n");
+   if (vkmsdev->config->writeback) {
+   writeback = vkms_enable_writeback_connector(vkmsdev);
+   if (writeback)
+   DRM_ERROR("Failed to init writeback connector\n");
+   }
 
drm_mode_config_reset(dev);
 
-- 
2.25.1

[PATCH V5 1/3] drm/vkms: Add vkms_config type

[Sumera Priyadarsini]

Currently, data for the device instance is held by vkms_device.
Add a separate type, vkms_config to contain configuration details
for the device and various modes to be later used by configfs.
This config data stays constant once the device is created.

Accordingly, add vkms_create and vkms_destroy to initialize/destroy
device through configfs. Currently, they are being called from vkms_init
and vkms_exit, but will be evoked from configfs later on. When configfs
is added, device configuration will be tracked by configfs and only vkms
device lifetime will be handled by vkms_init and vkms_exit functions.

Modify usage of enable_cursor feature to reflect the changes in
relevant files.

Co-developed-by: Daniel Vetter 
Signed-off-by: Daniel Vetter 
Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/vkms/vkms_drv.c| 40 --
 drivers/gpu/drm/vkms/vkms_drv.h| 12 +++--
 drivers/gpu/drm/vkms/vkms_output.c |  4 +--
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index aef29393b811..6b33975a5cb2 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -34,9 +34,9 @@
 #define DRIVER_MAJOR   1
 #define DRIVER_MINOR   0
 
-static struct vkms_device *vkms_device;
+static struct vkms_config *default_config;
 
-bool enable_cursor = true;
+static bool enable_cursor = true;
 module_param_named(enable_cursor, enable_cursor, bool, 0444);
 MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
 
@@ -122,10 +122,11 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
return vkms_output_init(vkmsdev, 0);
 }
 
-static int __init vkms_init(void)
+static int vkms_create(struct vkms_config *config)
 {
int ret;
struct platform_device *pdev;
+   struct vkms_device *vkms_device;
 
pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
if (IS_ERR(pdev))
@@ -143,6 +144,8 @@ static int __init vkms_init(void)
goto out_devres;
}
vkms_device->platform = pdev;
+   vkms_device->config = config;
+   config->dev = vkms_device;
 
ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
   DMA_BIT_MASK(64));
@@ -179,21 +182,42 @@ static int __init vkms_init(void)
return ret;
 }
 
-static void __exit vkms_exit(void)
+static int __init vkms_init(void)
+{
+   struct vkms_config *config = kmalloc(sizeof(*config), GFP_KERNEL);
+
+   default_config = config;
+
+   config->cursor = enable_cursor;
+
+   return vkms_create(config);
+}
+
+static void vkms_destroy(struct vkms_config *config)
 {
struct platform_device *pdev;
 
-   if (!vkms_device) {
+   if (!config->dev) {
DRM_INFO("vkms_device is NULL.\n");
return;
}
 
-   pdev = vkms_device->platform;
+   pdev = config->dev->platform;
 
-   drm_dev_unregister(&vkms_device->drm);
-   drm_atomic_helper_shutdown(&vkms_device->drm);
+   drm_dev_unregister(&config->dev->drm);
+   drm_atomic_helper_shutdown(&config->dev->drm);
devres_release_group(&pdev->dev, NULL);
platform_device_unregister(pdev);
+
+   config->dev = NULL;
+}
+
+static void __exit vkms_exit(void)
+{
+   if (default_config->dev)
+   vkms_destroy(default_config);
+
+   kfree(default_config);
 }
 
 module_init(vkms_init);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 5ed91ff08cb3..6a27bd8875f2 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -19,8 +19,6 @@
 #define XRES_MAX  8192
 #define YRES_MAX  8192
 
-extern bool enable_cursor;
-
 struct vkms_composer {
struct drm_framebuffer fb;
struct drm_rect src, dst;
@@ -82,10 +80,19 @@ struct vkms_output {
spinlock_t composer_lock;
 };
 
+struct vkms_device;
+
+struct vkms_config {
+   bool cursor;
+   /* only set when instantiated */
+   struct vkms_device *dev;
+};
+
 struct vkms_device {
struct drm_device drm;
struct platform_device *platform;
struct vkms_output output;
+   const struct vkms_config *config;
 };
 
 #define drm_crtc_to_vkms_output(target) \
@@ -124,3 +131,4 @@ void vkms_set_composer(struct vkms_output *out, bool 
enabled);
 int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
 
 #endif /* _VKMS_DRV_H_ */
+
diff --git a/drivers/gpu/drm/vkms/vkms_output.c 
b/drivers/gpu/drm/vkms/vkms_output.c
index 4a1848b0318f..8f3ffb28b9d1 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -46,7 +46,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
if (IS_ERR(primary))
return PTR_ERR(primary);
 
-   if (enable_cur

[PATCH V5 0/3] Decouple config data for configfs

[Sumera Priyadarsini]

This patchset aims to lay down some prep work before configfs can be
implemented for the vkms driver. The first patch in the series adds a
new type vkms_config to track device configuration. The second and third
patch add module testing support for writeback operations.

The first patch is developed by Daniel Vetter.

Daniel Vetter (1):
  drm/vkms: Add vkms_config type

Sumera Priyadarsini (3):
  drm/vkms: Add vkms_config type
  drm/vkms: Add support for writeback module
  drm/vkms: Add information about module options

 Documentation/gpu/vkms.rst | 12 
 drivers/gpu/drm/vkms/vkms_drv.c| 45 --
 drivers/gpu/drm/vkms/vkms_drv.h| 13 +++--
 drivers/gpu/drm/vkms/vkms_output.c | 13 +
 4 files changed, 68 insertions(+), 15 deletions(-)

---
Changes in v2:
 - add Co-developed-by tag
 
Changes in v3:
 - correct usage of Co-developed by tag(Melissa)
 - add enable_writeback_feature(Melissa)
 - modify commit message(Melissa)

Changes in v4:
 - split previous patch into patchset(Melissa)
 - fix checkpatch issue(Melissa)
 - update docs(Daniel)

Changes in v5:
 - modify docs patch(Daniel)
-- 
2.25.1

[PATCH V4 3/3] drm/vkms: Add information about module options

[Sumera Priyadarsini]

Update vkms documentation to contain usage of `modinfo`
command and steps to load vkms with module options enabled.

Signed-off-by: Sumera Priyadarsini 
---
 Documentation/gpu/vkms.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 9e030c74a82e..45fe02f643a8 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -35,6 +35,16 @@ Now, to load the driver, use::
 On running the lsmod command now, the VKMS driver will appear listed.
 You can also observe the driver being loaded in the dmesg logs.
 
+You can use the `modinfo` command to see module options for vkms::
+
+  modinfo vkms
+
+Module options are helpful when testing, and enabling modules
+can be done while loading vkms. For example, to load vkms with cursor enabled,
+use::
+
+  sudo modprobe vkms enable_cursor=1
+
 To disable the driver, use ::
 
   sudo modprobe -r vkms
-- 
2.25.1

[PATCH V4 2/3] drm/vkms: Add support for writeback module

[Sumera Priyadarsini]

Add enable_writeback feature to vkms_config as a module.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/vkms/vkms_drv.c| 5 +
 drivers/gpu/drm/vkms/vkms_drv.h| 1 +
 drivers/gpu/drm/vkms/vkms_output.c | 9 ++---
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 6b33975a5cb2..708f7f54001d 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -40,6 +40,10 @@ static bool enable_cursor = true;
 module_param_named(enable_cursor, enable_cursor, bool, 0444);
 MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
 
+static bool enable_writeback = true;
+module_param_named(enable_writeback, enable_writeback, bool, 0444);
+MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector 
support");
+
 DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
 
 static void vkms_release(struct drm_device *dev)
@@ -189,6 +193,7 @@ static int __init vkms_init(void)
default_config = config;
 
config->cursor = enable_cursor;
+   config->writeback = enable_writeback;
 
return vkms_create(config);
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 6a27bd8875f2..b9b4e2bc11c0 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -83,6 +83,7 @@ struct vkms_output {
 struct vkms_device;
 
 struct vkms_config {
+   bool writeback;
bool cursor;
/* only set when instantiated */
struct vkms_device *dev;
diff --git a/drivers/gpu/drm/vkms/vkms_output.c 
b/drivers/gpu/drm/vkms/vkms_output.c
index 8f3ffb28b9d1..f5f6f15c362c 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -41,6 +41,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
struct drm_crtc *crtc = &output->crtc;
struct drm_plane *primary, *cursor = NULL;
int ret;
+   int writeback;
 
primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
if (IS_ERR(primary))
@@ -80,9 +81,11 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
goto err_attach;
}
 
-   ret = vkms_enable_writeback_connector(vkmsdev);
-   if (ret)
-   DRM_ERROR("Failed to init writeback connector\n");
+   if (vkmsdev->config->writeback) {
+   writeback = vkms_enable_writeback_connector(vkmsdev);
+   if (writeback)
+   DRM_ERROR("Failed to init writeback connector\n");
+   }
 
drm_mode_config_reset(dev);
 
-- 
2.25.1

[PATCH V4 1/3] drm/vkms: Add vkms_config type

[Sumera Priyadarsini]

Currently, data for the device instance is held by vkms_device.
Add a separate type, vkms_config to contain configuration details
for the device and various modes to be later used by configfs.
This config data stays constant once the device is created.

Accordingly, add vkms_create and vkms_destroy to initialize/destroy
device through configfs. Currently, they are being called from vkms_init
and vkms_exit, but will be evoked from configfs later on. When configfs
is added, device configuration will be tracked by configfs and only vkms
device lifetime will be handled by vkms_init and vkms_exit functions.

Modify usage of enable_cursor feature to reflect the changes in
relevant files.

Co-developed-by: Daniel Vetter 
Signed-off-by: Daniel Vetter 
Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/vkms/vkms_drv.c| 40 --
 drivers/gpu/drm/vkms/vkms_drv.h| 12 +++--
 drivers/gpu/drm/vkms/vkms_output.c |  4 +--
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index aef29393b811..6b33975a5cb2 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -34,9 +34,9 @@
 #define DRIVER_MAJOR   1
 #define DRIVER_MINOR   0
 
-static struct vkms_device *vkms_device;
+static struct vkms_config *default_config;
 
-bool enable_cursor = true;
+static bool enable_cursor = true;
 module_param_named(enable_cursor, enable_cursor, bool, 0444);
 MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
 
@@ -122,10 +122,11 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
return vkms_output_init(vkmsdev, 0);
 }
 
-static int __init vkms_init(void)
+static int vkms_create(struct vkms_config *config)
 {
int ret;
struct platform_device *pdev;
+   struct vkms_device *vkms_device;
 
pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
if (IS_ERR(pdev))
@@ -143,6 +144,8 @@ static int __init vkms_init(void)
goto out_devres;
}
vkms_device->platform = pdev;
+   vkms_device->config = config;
+   config->dev = vkms_device;
 
ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
   DMA_BIT_MASK(64));
@@ -179,21 +182,42 @@ static int __init vkms_init(void)
return ret;
 }
 
-static void __exit vkms_exit(void)
+static int __init vkms_init(void)
+{
+   struct vkms_config *config = kmalloc(sizeof(*config), GFP_KERNEL);
+
+   default_config = config;
+
+   config->cursor = enable_cursor;
+
+   return vkms_create(config);
+}
+
+static void vkms_destroy(struct vkms_config *config)
 {
struct platform_device *pdev;
 
-   if (!vkms_device) {
+   if (!config->dev) {
DRM_INFO("vkms_device is NULL.\n");
return;
}
 
-   pdev = vkms_device->platform;
+   pdev = config->dev->platform;
 
-   drm_dev_unregister(&vkms_device->drm);
-   drm_atomic_helper_shutdown(&vkms_device->drm);
+   drm_dev_unregister(&config->dev->drm);
+   drm_atomic_helper_shutdown(&config->dev->drm);
devres_release_group(&pdev->dev, NULL);
platform_device_unregister(pdev);
+
+   config->dev = NULL;
+}
+
+static void __exit vkms_exit(void)
+{
+   if (default_config->dev)
+   vkms_destroy(default_config);
+
+   kfree(default_config);
 }
 
 module_init(vkms_init);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 5ed91ff08cb3..6a27bd8875f2 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -19,8 +19,6 @@
 #define XRES_MAX  8192
 #define YRES_MAX  8192
 
-extern bool enable_cursor;
-
 struct vkms_composer {
struct drm_framebuffer fb;
struct drm_rect src, dst;
@@ -82,10 +80,19 @@ struct vkms_output {
spinlock_t composer_lock;
 };
 
+struct vkms_device;
+
+struct vkms_config {
+   bool cursor;
+   /* only set when instantiated */
+   struct vkms_device *dev;
+};
+
 struct vkms_device {
struct drm_device drm;
struct platform_device *platform;
struct vkms_output output;
+   const struct vkms_config *config;
 };
 
 #define drm_crtc_to_vkms_output(target) \
@@ -124,3 +131,4 @@ void vkms_set_composer(struct vkms_output *out, bool 
enabled);
 int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
 
 #endif /* _VKMS_DRV_H_ */
+
diff --git a/drivers/gpu/drm/vkms/vkms_output.c 
b/drivers/gpu/drm/vkms/vkms_output.c
index 4a1848b0318f..8f3ffb28b9d1 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -46,7 +46,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
if (IS_ERR(primary))
return PTR_ERR(primary);
 
-   if (enable_cur

[PATCH V4 0/3] Decouple config data for configfs

[Sumera Priyadarsini]

This patchset aims to lay down some prep work before configfs can be
implemented for the vkms driver. The first patch in the series adds a
new type vkms_config to track device configuration. The second and third
patch add module testing support for writeback operations.

The first patch is developed by Daniel Vetter.

Daniel Vetter (1):
  drm/vkms: Add vkms_config type

Sumera Priyadarsini (3):
  drm/vkms: Add vkms_config type
  drm/vkms: Add support for writeback module
  drm/vkms: Add information about module options

 Documentation/gpu/vkms.rst | 10 +++
 drivers/gpu/drm/vkms/vkms_drv.c| 45 --
 drivers/gpu/drm/vkms/vkms_drv.h| 13 +++--
 drivers/gpu/drm/vkms/vkms_output.c | 13 +
 4 files changed, 66 insertions(+), 15 deletions(-)

---
Changes in v2:
 - add Co-developed-by tag
 
Changes in v3:
 - correct usage of Co-developed by tag(Melissa)
 - add enable_writeback_feature(Melissa)
 - modify commit message(Melissa)

Changes in v4:
 - split previous patch into patchset(Melissa)
 - fix checkpatch issue(Melissa)
 - update docs(Daniel)
-- 
2.25.1

[PATCH V3] drm/vkms: Decouple config data for configfs

[Sumera Priyadarsini]

Currently, data for the device instance is held by vkms_device.
Add a separate type, vkms_config to contain configuration details
for the device and various modes to be later used by configfs.
This config data stays constant once the device is created.

Accordingly, add vkms_create and vkms_destroy to initialize/destroy
device through configfs. Currently, they are being called from vkms_init
and vkms_exit, but will be evoked from configfs later on. When configfs
is added, device configuration will be tracked by configfs and only vkms
device lifetime will be handled by vkms_init and vkms_exit functions.

Modify usage of enable_cursor feature to reflect the changes in
relevant files.

Add enable_writeback_connector feature to vkms_config type.

Co-developed-by: Daniel Vetter 
Signed-off-by: Daniel Vetter 
Signed-off-by: Sumera Priyadarsini 

---
Changes in v2:
- add Co-developed-by tag

Changes in v3:
- correct usage of Co-developed by tag(Melissa)
- add enable_writeback_feature(Melissa)
- modify commit message(Melissa)
---
 drivers/gpu/drm/vkms/vkms_drv.c| 45 --
 drivers/gpu/drm/vkms/vkms_drv.h| 12 ++--
 drivers/gpu/drm/vkms/vkms_output.c | 13 +
 3 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index aef29393b811..fab964900dce 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -34,12 +34,16 @@
 #define DRIVER_MAJOR   1
 #define DRIVER_MINOR   0
 
-static struct vkms_device *vkms_device;
+static struct vkms_config *default_config;
 
-bool enable_cursor = true;
+static bool enable_cursor = true;
 module_param_named(enable_cursor, enable_cursor, bool, 0444);
 MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
 
+static bool enable_writeback_connector = true;
+module_param_named(enable_writeback_connector, enable_writeback_connector, 
bool, 0444);
+MODULE_PARM_DESC(enable_writeback_connector, "Enable/Disable writeback 
connector support");
+
 DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
 
 static void vkms_release(struct drm_device *dev)
@@ -122,10 +126,11 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
return vkms_output_init(vkmsdev, 0);
 }
 
-static int __init vkms_init(void)
+static int vkms_create(struct vkms_config *config)
 {
int ret;
struct platform_device *pdev;
+   struct vkms_device *vkms_device;
 
pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
if (IS_ERR(pdev))
@@ -143,6 +148,8 @@ static int __init vkms_init(void)
goto out_devres;
}
vkms_device->platform = pdev;
+   vkms_device->config = config;
+   config->dev = vkms_device;
 
ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
   DMA_BIT_MASK(64));
@@ -179,21 +186,43 @@ static int __init vkms_init(void)
return ret;
 }
 
-static void __exit vkms_exit(void)
+static int __init vkms_init(void)
+{
+   struct vkms_config *config = kmalloc(sizeof(*config), GFP_KERNEL);
+
+   default_config = config;
+
+   config->cursor = enable_cursor;
+   config->writeback = enable_writeback_connector;
+
+   return vkms_create(config);
+}
+
+static void vkms_destroy(struct vkms_config *config)
 {
struct platform_device *pdev;
 
-   if (!vkms_device) {
+   if (!config->dev) {
DRM_INFO("vkms_device is NULL.\n");
return;
}
 
-   pdev = vkms_device->platform;
+   pdev = config->dev->platform;
 
-   drm_dev_unregister(&vkms_device->drm);
-   drm_atomic_helper_shutdown(&vkms_device->drm);
+   drm_dev_unregister(&config->dev->drm);
+   drm_atomic_helper_shutdown(&config->dev->drm);
devres_release_group(&pdev->dev, NULL);
platform_device_unregister(pdev);
+
+   config->dev = NULL;
+}
+
+static void __exit vkms_exit(void)
+{
+   if (default_config->dev)
+   vkms_destroy(default_config);
+
+   kfree(default_config);
 }
 
 module_init(vkms_init);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 5ed91ff08cb3..caa1fafb6ca7 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -19,8 +19,6 @@
 #define XRES_MAX  8192
 #define YRES_MAX  8192
 
-extern bool enable_cursor;
-
 struct vkms_composer {
struct drm_framebuffer fb;
struct drm_rect src, dst;
@@ -82,10 +80,18 @@ struct vkms_output {
spinlock_t composer_lock;
 };
 
+struct vkms_device;
+struct vkms_config {
+   bool writeback;
+   bool cursor;
+   /* only set when instantiated */
+   struct vkms_device *dev;
+};
 struct vkms_device {
struct drm_device drm;
struct platform_device *platform;
struct vkms_output o

[PATCH V2] drm/vkms: Decouple config data for configfs

[Sumera Priyadarsini]

Currently, data for the device instance is held by vkms_device.
Add a separate type, vkms_config to contain configuration details
for the device and various modes to be later used by configfs.
This config data stays constant once the device is created.

Accordingly, add vkms_create and vkms_destroy to initialize/destroy
device through configfs. Currently, they are being called from vkms_init
and vkms_exit, but will be evoked from configfs later on. When configfs
is added, device configuration- enabling/disabling features will
be tracked by configfs and only vkms device lifetime will be kept track of
by vkms_init and vkms_exit functions.

Modify usage of enable_cursor feature to reflect the changes in
relevant files.

Signed-off-by: Sumera Priyadarsini 
Co-developed-by: Daniel Vetter 
---
Changes in v2:
- add Co-developed-by tag

---
 drivers/gpu/drm/vkms/vkms_drv.c| 40 --
 drivers/gpu/drm/vkms/vkms_drv.h| 11 +---
 drivers/gpu/drm/vkms/vkms_output.c |  4 +--
 3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index aef29393b811..6b33975a5cb2 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -34,9 +34,9 @@
 #define DRIVER_MAJOR   1
 #define DRIVER_MINOR   0
 
-static struct vkms_device *vkms_device;
+static struct vkms_config *default_config;
 
-bool enable_cursor = true;
+static bool enable_cursor = true;
 module_param_named(enable_cursor, enable_cursor, bool, 0444);
 MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
 
@@ -122,10 +122,11 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
return vkms_output_init(vkmsdev, 0);
 }
 
-static int __init vkms_init(void)
+static int vkms_create(struct vkms_config *config)
 {
int ret;
struct platform_device *pdev;
+   struct vkms_device *vkms_device;
 
pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
if (IS_ERR(pdev))
@@ -143,6 +144,8 @@ static int __init vkms_init(void)
goto out_devres;
}
vkms_device->platform = pdev;
+   vkms_device->config = config;
+   config->dev = vkms_device;
 
ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
   DMA_BIT_MASK(64));
@@ -179,21 +182,42 @@ static int __init vkms_init(void)
return ret;
 }
 
-static void __exit vkms_exit(void)
+static int __init vkms_init(void)
+{
+   struct vkms_config *config = kmalloc(sizeof(*config), GFP_KERNEL);
+
+   default_config = config;
+
+   config->cursor = enable_cursor;
+
+   return vkms_create(config);
+}
+
+static void vkms_destroy(struct vkms_config *config)
 {
struct platform_device *pdev;
 
-   if (!vkms_device) {
+   if (!config->dev) {
DRM_INFO("vkms_device is NULL.\n");
return;
}
 
-   pdev = vkms_device->platform;
+   pdev = config->dev->platform;
 
-   drm_dev_unregister(&vkms_device->drm);
-   drm_atomic_helper_shutdown(&vkms_device->drm);
+   drm_dev_unregister(&config->dev->drm);
+   drm_atomic_helper_shutdown(&config->dev->drm);
devres_release_group(&pdev->dev, NULL);
platform_device_unregister(pdev);
+
+   config->dev = NULL;
+}
+
+static void __exit vkms_exit(void)
+{
+   if (default_config->dev)
+   vkms_destroy(default_config);
+
+   kfree(default_config);
 }
 
 module_init(vkms_init);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 5ed91ff08cb3..2fa0c52f1dd8 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -19,8 +19,6 @@
 #define XRES_MAX  8192
 #define YRES_MAX  8192
 
-extern bool enable_cursor;
-
 struct vkms_composer {
struct drm_framebuffer fb;
struct drm_rect src, dst;
@@ -82,10 +80,17 @@ struct vkms_output {
spinlock_t composer_lock;
 };
 
+struct vkms_device;
+struct vkms_config {
+   bool cursor;
+   /* only set when instantiated */
+   struct vkms_device *dev;
+};
 struct vkms_device {
struct drm_device drm;
struct platform_device *platform;
struct vkms_output output;
+   const struct vkms_config *config;
 };
 
 #define drm_crtc_to_vkms_output(target) \
@@ -123,4 +128,4 @@ void vkms_set_composer(struct vkms_output *out, bool 
enabled);
 /* Writeback */
 int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
 
-#endif /* _VKMS_DRV_H_ */
+#endif /* _VKMS_DRV_H_ */
\ No newline at end of file
diff --git a/drivers/gpu/drm/vkms/vkms_output.c 
b/drivers/gpu/drm/vkms/vkms_output.c
index 4a1848b0318f..8f3ffb28b9d1 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -46,7 +46,7 @@ int vkms_output_init(struct vkms_d

[PATCH] drm/vkms: Decouple config data for configfs

[Sumera Priyadarsini]

Currently, data for the device instance is held by vkms_device.
Add a separate type, vkms_config to contain configuration details
for the device and various modes to be later used by configfs.
This config data stays constant once the device is created.

Accordingly, add vkms_create and vkms_destroy to initialize/destroy
device through configfs. Currently, they are being called from vkms_init
and vkms_exit, but will be evoked from configfs later on. When configfs
is added, device configuration- enabling/disabling features will
be tracked by configfs and only vkms device lifetime will be kept track of
by vkms_init and vkms_exit functions.

Modify usage of enable_cursor feature to reflect the changes in
relevant files.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/vkms/vkms_drv.c| 40 --
 drivers/gpu/drm/vkms/vkms_drv.h| 11 +---
 drivers/gpu/drm/vkms/vkms_output.c |  4 +--
 3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index aef29393b811..6b33975a5cb2 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -34,9 +34,9 @@
 #define DRIVER_MAJOR   1
 #define DRIVER_MINOR   0
 
-static struct vkms_device *vkms_device;
+static struct vkms_config *default_config;
 
-bool enable_cursor = true;
+static bool enable_cursor = true;
 module_param_named(enable_cursor, enable_cursor, bool, 0444);
 MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
 
@@ -122,10 +122,11 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
return vkms_output_init(vkmsdev, 0);
 }
 
-static int __init vkms_init(void)
+static int vkms_create(struct vkms_config *config)
 {
int ret;
struct platform_device *pdev;
+   struct vkms_device *vkms_device;
 
pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
if (IS_ERR(pdev))
@@ -143,6 +144,8 @@ static int __init vkms_init(void)
goto out_devres;
}
vkms_device->platform = pdev;
+   vkms_device->config = config;
+   config->dev = vkms_device;
 
ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
   DMA_BIT_MASK(64));
@@ -179,21 +182,42 @@ static int __init vkms_init(void)
return ret;
 }
 
-static void __exit vkms_exit(void)
+static int __init vkms_init(void)
+{
+   struct vkms_config *config = kmalloc(sizeof(*config), GFP_KERNEL);
+
+   default_config = config;
+
+   config->cursor = enable_cursor;
+
+   return vkms_create(config);
+}
+
+static void vkms_destroy(struct vkms_config *config)
 {
struct platform_device *pdev;
 
-   if (!vkms_device) {
+   if (!config->dev) {
DRM_INFO("vkms_device is NULL.\n");
return;
}
 
-   pdev = vkms_device->platform;
+   pdev = config->dev->platform;
 
-   drm_dev_unregister(&vkms_device->drm);
-   drm_atomic_helper_shutdown(&vkms_device->drm);
+   drm_dev_unregister(&config->dev->drm);
+   drm_atomic_helper_shutdown(&config->dev->drm);
devres_release_group(&pdev->dev, NULL);
platform_device_unregister(pdev);
+
+   config->dev = NULL;
+}
+
+static void __exit vkms_exit(void)
+{
+   if (default_config->dev)
+   vkms_destroy(default_config);
+
+   kfree(default_config);
 }
 
 module_init(vkms_init);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 5ed91ff08cb3..2fa0c52f1dd8 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -19,8 +19,6 @@
 #define XRES_MAX  8192
 #define YRES_MAX  8192
 
-extern bool enable_cursor;
-
 struct vkms_composer {
struct drm_framebuffer fb;
struct drm_rect src, dst;
@@ -82,10 +80,17 @@ struct vkms_output {
spinlock_t composer_lock;
 };
 
+struct vkms_device;
+struct vkms_config {
+   bool cursor;
+   /* only set when instantiated */
+   struct vkms_device *dev;
+};
 struct vkms_device {
struct drm_device drm;
struct platform_device *platform;
struct vkms_output output;
+   const struct vkms_config *config;
 };
 
 #define drm_crtc_to_vkms_output(target) \
@@ -123,4 +128,4 @@ void vkms_set_composer(struct vkms_output *out, bool 
enabled);
 /* Writeback */
 int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
 
-#endif /* _VKMS_DRV_H_ */
+#endif /* _VKMS_DRV_H_ */
\ No newline at end of file
diff --git a/drivers/gpu/drm/vkms/vkms_output.c 
b/drivers/gpu/drm/vkms/vkms_output.c
index 4a1848b0318f..8f3ffb28b9d1 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -46,7 +46,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
if (IS_ERR(primary))
 

[PATCH v3] drm/vkms: Add setup and testing information

[Sumera Priyadarsini]

Update the vkms documentation to contain steps to:

 - setup the vkms driver
 - run tests using igt

Signed-off-by: Sumera Priyadarsini 
___
Changes in v2:
 - Change heading to title case (Daniel)
 - Add examples to run tests directly (Daniel)
 - Add examples to run subtests (Melissa)

Changes in v3:
 - Add example using run-tests.sh script(Daniel)
---
 Documentation/gpu/vkms.rst | 70 ++
 1 file changed, 70 insertions(+)

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 13bab1d93bb3..9e030c74a82e 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -7,6 +7,76 @@
 .. kernel-doc:: drivers/gpu/drm/vkms/vkms_drv.c
:doc: vkms (Virtual Kernel Modesetting)
 
+Setup
+=
+
+The VKMS driver can be setup with the following steps:
+
+To check if VKMS is loaded, run::
+
+  lsmod | grep vkms
+
+This should list the VKMS driver. If no output is obtained, then
+you need to enable and/or load the VKMS driver.
+Ensure that the VKMS driver has been set as a loadable module in your
+kernel config file. Do::
+
+  make nconfig
+
+  Go to `Device Drivers> Graphics support`
+
+  Enable `Virtual KMS (EXPERIMENTAL)`
+
+Compile and build the kernel for the changes to get reflected.
+Now, to load the driver, use::
+
+  sudo modprobe vkms
+
+On running the lsmod command now, the VKMS driver will appear listed.
+You can also observe the driver being loaded in the dmesg logs.
+
+To disable the driver, use ::
+
+  sudo modprobe -r vkms
+
+Testing With IGT
+
+
+The IGT GPU Tools is a test suite used specifically for debugging and
+development of the DRM drivers.
+The IGT Tools can be installed from
+`here <https://gitlab.freedesktop.org/drm/igt-gpu-tools>`_ .
+
+The tests need to be run without a compositor, so you need to switch to text
+only mode. You can do this by::
+
+  sudo systemctl isolate multi-user.target
+
+To return to graphical mode, do::
+
+  sudo systemctl isolate graphical.target
+
+Once you are in text only mode, you can run tests using the --device switch
+or IGT_DEVICE variable to specify the device filter for the driver we want
+to test. IGT_DEVICE can also be used with the run-test.sh script to run the
+tests for a specific driver::
+
+  sudo ./build/tests/ --device "sys:/sys/devices/platform/vkms"
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./scripts/run-tests.sh -t 

+
+For example, to test the functionality of the writeback library,
+we can run the kms_writeback test::
+
+  sudo ./build/tests/kms_writeback --device "sys:/sys/devices/platform/vkms"
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_writeback
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./scripts/run-tests.sh -t 
kms_writeback
+
+You can also run subtests if you do not want to run the entire test::
+
+  sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device 
"sys:/sys/devices/platform/vkms"
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip 
--run-subtest basic-plain-flip
+
 TODO
 
 
-- 
2.25.1

Re: [PATCH V2] drm/vkms: Add setup and testing information

[Sumera Priyadarsini]

On Wed, Dec 9, 2020 at 6:24 AM Daniel Vetter  wrote:
>
> On Wed, Dec 09, 2020 at 02:07:35AM +0530, Sumera Priyadarsini wrote:
> > Update the vkms documentation to contain steps to:
> >
> >  - setup the vkms driver
> >  - run tests using igt
> >
> > Signed-off-by: Sumera Priyadarsini 
> > ___
> > Changes in v2:
> >  - Change heading to title case (Daniel)
> >  - Add examples to run tests directly (Daniel)
> >  - Add examples to run subtests (Melissa)
> > ---
> >  Documentation/gpu/vkms.rst | 67 ++
> >  1 file changed, 67 insertions(+)
> >
> > diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> > index 13bab1d93bb3..d6739fbbe503 100644
> > --- a/Documentation/gpu/vkms.rst
> > +++ b/Documentation/gpu/vkms.rst
> > @@ -7,6 +7,73 @@
> >  .. kernel-doc:: drivers/gpu/drm/vkms/vkms_drv.c
> > :doc: vkms (Virtual Kernel Modesetting)
> >
> > +Setup
> > +=
> > +
> > +The VKMS driver can be setup with the following steps:
> > +
> > +To check if VKMS is loaded, run::
> > +
> > +  lsmod | grep vkms
> > +
> > +This should list the VKMS driver. If no output is obtained, then
> > +you need to enable and/or load the VKMS driver.
> > +Ensure that the VKMS driver has been set as a loadable module in your
> > +kernel config file. Do::
> > +
> > +  make nconfig
> > +
> > +  Go to `Device Drivers> Graphics support`
> > +
> > +  Enable `Virtual KMS (EXPERIMENTAL)`
> > +
> > +Compile and build the kernel for the changes to get reflected.
> > +Now, to load the driver, use::
> > +
> > +  sudo modprobe vkms
> > +
> > +On running the lsmod command now, the VKMS driver will appear listed.
> > +You can also observe the driver being loaded in the dmesg logs.
> > +
> > +To disable the driver, use ::
> > +
> > +  sudo modprobe -r vkms
> > +
> > +Testing With IGT
> > +
> > +
> > +The IGT GPU Tools is a test suite used specifically for debugging and
> > +development of the DRM drivers.
> > +The IGT Tools can be installed from
> > +`here <https://gitlab.freedesktop.org/drm/igt-gpu-tools>`_ .
> > +
> > +The tests need to be run without a compositor, so you need to switch to 
> > text
> > +only mode. You can do this by::
> > +
> > +  sudo systemctl isolate multi-user.target
> > +
> > +To return to graphical mode, do::
> > +
> > +  sudo systemctl isolate graphical.target
> > +
> > +Once you are in text only mode, you can run tests using the --device switch
> > +or IGT_DEVICE variable to specify the device filter for the driver we want
> > +to test::
> > +
> > +  sudo ./build/tests/ --device 
> > "sys:/sys/devices/platform/vkms"
> > +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/ > test>
> > +
> > +For example, to test the functionality of the writeback library,
> > +we can run the kms_writeback test::
> > +
> > +  sudo ./build/tests/kms_writeback --device 
> > "sys:/sys/devices/platform/vkms"
> > +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" 
> > ./build/tests/kms_writeback
> > +
> > +You can also run subtests if you do not want to run the entire test::
> > +
> > +  sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device 
> > "sys:/sys/devices/platform/vkms"
> > +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip 
> > --run-subtest basic-plain-flip
>
> Does IGT_DEVICE also work with run-tests.sh? Aside from my curious
> question, patch looks good to me, thanks a lot.

Good catch, it does.

Melissa, IGT_FORCE_DRIVER also works. I think I was used test/kms_flip
earlier instead of
./build/test/kms_flip hence the fluke.

Should I add these also to the docs, was wondering if it will get too
confusing

>
> Reviewed-by: Daniel Vetter 
>
> > +
> >  TODO
> >  
> >
> > --
> > 2.25.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
`sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./scripts/run-tests.sh -t kms_writeback`

[9445.317993] [1/4] kms_writeback (writeback-pixel-formats)
[9445.623499] [2/4] kms_writeback (writeback-invalid-parameters)
[9447.076263] [3/4] kms_writeback (writeback-fb-id)
[9447.371402] [4/4] kms_writeback (writeback-check-output)
Done.



`sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_writeback`

[PATCH V2] drm/vkms: Add setup and testing information

[Sumera Priyadarsini]

Update the vkms documentation to contain steps to:

 - setup the vkms driver
 - run tests using igt

Signed-off-by: Sumera Priyadarsini 
___
Changes in v2:
 - Change heading to title case (Daniel)
 - Add examples to run tests directly (Daniel)
 - Add examples to run subtests (Melissa)
---
 Documentation/gpu/vkms.rst | 67 ++
 1 file changed, 67 insertions(+)

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 13bab1d93bb3..d6739fbbe503 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -7,6 +7,73 @@
 .. kernel-doc:: drivers/gpu/drm/vkms/vkms_drv.c
:doc: vkms (Virtual Kernel Modesetting)
 
+Setup
+=
+
+The VKMS driver can be setup with the following steps:
+
+To check if VKMS is loaded, run::
+
+  lsmod | grep vkms
+
+This should list the VKMS driver. If no output is obtained, then
+you need to enable and/or load the VKMS driver.
+Ensure that the VKMS driver has been set as a loadable module in your
+kernel config file. Do::
+
+  make nconfig
+
+  Go to `Device Drivers> Graphics support`
+
+  Enable `Virtual KMS (EXPERIMENTAL)`
+
+Compile and build the kernel for the changes to get reflected.
+Now, to load the driver, use::
+
+  sudo modprobe vkms
+
+On running the lsmod command now, the VKMS driver will appear listed.
+You can also observe the driver being loaded in the dmesg logs.
+
+To disable the driver, use ::
+
+  sudo modprobe -r vkms
+
+Testing With IGT
+
+
+The IGT GPU Tools is a test suite used specifically for debugging and
+development of the DRM drivers.
+The IGT Tools can be installed from
+`here <https://gitlab.freedesktop.org/drm/igt-gpu-tools>`_ .
+
+The tests need to be run without a compositor, so you need to switch to text
+only mode. You can do this by::
+
+  sudo systemctl isolate multi-user.target
+
+To return to graphical mode, do::
+
+  sudo systemctl isolate graphical.target
+
+Once you are in text only mode, you can run tests using the --device switch
+or IGT_DEVICE variable to specify the device filter for the driver we want
+to test::
+
+  sudo ./build/tests/ --device "sys:/sys/devices/platform/vkms"
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/
+
+For example, to test the functionality of the writeback library,
+we can run the kms_writeback test::
+
+  sudo ./build/tests/kms_writeback --device "sys:/sys/devices/platform/vkms"
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_writeback
+
+You can also run subtests if you do not want to run the entire test::
+
+  sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device 
"sys:/sys/devices/platform/vkms"
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip 
--run-subtest basic-plain-flip
+
 TODO
 
 
-- 
2.25.1

[PATCH] drm/vkms: Add setup and testing information

[Sumera Priyadarsini]

Update the vkms documentation to contain steps to:

 - setup the vkms driver
 - run tests using igt

Signed-off-by: Sumera Priyadarsini 
---
 Documentation/gpu/vkms.rst | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 13bab1d93bb3..d6782174d23f 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -7,6 +7,53 @@
 .. kernel-doc:: drivers/gpu/drm/vkms/vkms_drv.c
:doc: vkms (Virtual Kernel Modesetting)
 
+SETUP
+=
+
+The VKMS driver can be setup with the following steps:
+
+To check if VKMS is loaded, run::
+
+  lsmod | grep vkms
+
+This should list the VKMS driver. If no output is obtained, then
+you need to enable and/or load the VKMS driver.
+Ensure that the VKMS driver has been set as a loadable module in your
+kernel config file. The following line should be present in the .config
+file in your kernel root::
+
+  CONFIG_DRM_VKMS=m
+
+Compile and build the kernel for the changes to get reflected.
+If your existing config already has VKMS available as a loadable module,
+then there is no need to build the kernel again.
+Now, to load the driver, use::
+
+  sudo modprobe vkms
+
+On running the lsmod command now, the VKMS driver will appear listed.
+You can also observe the driver being loaded in the dmesg logs.
+
+To disable the driver, use ::
+
+  sudo modprobe -r vkms
+
+TESTING WITH IGT
+
+
+The IGT GPU Tools is a test suite used specifically for debugging and
+development of the DRM drivers.
+The IGT Tools can be installed from
+`here <https://gitlab.freedesktop.org/drm/igt-gpu-tools>`_ .
+Once you have installed IGT, you can run tests using::
+
+  ./scripts/run-tests.sh -t 
+
+For example, to test the functionality of the igt_draw library,
+we can run the kms_draw_crc test::
+
+  ./scripts/run-tests.sh -t kms_draw_crc
+
 TODO
 
 
-- 
2.25.1

[PATCH v2] Documentation: Coccinelle: Improve command example for debugging patches

[Sumera Priyadarsini]

Modify Coccinelle documentation to clarify usage of make command to
run coccicheck on a folder.

Changes in v2:
- Give example of folder instead of file
- Add note

Signed-off-by: Sumera Priyadarsini 
---
 Documentation/dev-tools/coccinelle.rst | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/dev-tools/coccinelle.rst 
b/Documentation/dev-tools/coccinelle.rst
index 74c5e6aeeff5..9c454de5a7f7 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -224,14 +224,21 @@ you may want to use::
 
 rm -f err.log
 export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci
-make coccicheck DEBUG_FILE="err.log" MODE=report SPFLAGS="--profile 
--show-trying" M=./drivers/mfd/arizona-irq.c
+make coccicheck DEBUG_FILE="err.log" MODE=report SPFLAGS="--profile 
--show-trying" M=./drivers/mfd
 
 err.log will now have the profiling information, while stdout will
 provide some progress information as Coccinelle moves forward with
 work.
 
+NOTE:
+
 DEBUG_FILE support is only supported when using coccinelle >= 1.0.2.
 
+Currently, DEBUG_FILE support is only available to check folders, and
+not single files. This is because checking a single file requires spatch
+to be called twice leading to DEBUG_FILE being set both times to the same 
value,
+giving rise to an error.
+
 .cocciconfig support
 
 
-- 
2.25.1

[PATCH v3] scripts: coccicheck: Correct usage of make coccicheck

[Sumera Priyadarsini]

The command "make coccicheck C=1 CHECK=scripts/coccicheck" results in the
error:
./scripts/coccicheck: line 65: -1: shift count out of range

This happens because every time the C variable is specified,
the shell arguments need to be "shifted" in order to take only
the last argument, which is the C file to test. These shell arguments
mostly comprise flags that have been set in the Makefile. However,
when coccicheck is specified in the make command as a rule, the
number of shell arguments is zero, thus passing the invalid value -1
to the shift command, resulting in an error.

Modify coccicheck to print correct usage of make coccicheck so as to
avoid the error.

Signed-off-by: Sumera Priyadarsini 
---
Changes in v2:
- Move test to only display error message

Changes in v3:
- Update example with latest file
---
 scripts/coccicheck | 12 
 1 file changed, 12 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index 209bb0427b43..d1aaa1dc0a69 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -61,6 +61,18 @@ COCCIINCLUDE=${COCCIINCLUDE// -include/ --include}
 if [ "$C" = "1" -o "$C" = "2" ]; then
 ONLINE=1
 
+if [[ $# -le 0 ]]; then
+   echo ''
+   echo 'Specifying both the variable "C" and rule "coccicheck" in the 
make
+command results in a shift count error.'
+   echo ''
+   echo 'Try specifying "scripts/coccicheck" as a value for the CHECK 
variable instead.'
+   echo ''
+   echo 'Example:  make C=2 CHECK=scripts/coccicheck 
drivers/net/ethernet/ethoc.o'
+   echo ''
+   exit 1
+fi
+
 # Take only the last argument, which is the C file to test
 shift $(( $# - 1 ))
 OPTIONS="$COCCIINCLUDE $1"
-- 
2.25.1

[PATCH] Documentation: Coccinelle: Improve command example for debugging patches

[Sumera Priyadarsini]

Modify Coccinelle documentation to clarify usage of make command to
run coccicheck on a single file.

Signed-off-by: Sumera Priyadarsini 
---
 Documentation/dev-tools/coccinelle.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/dev-tools/coccinelle.rst 
b/Documentation/dev-tools/coccinelle.rst
index 74c5e6aeeff5..9e60cf175fd6 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -224,7 +224,7 @@ you may want to use::
 
 rm -f err.log
 export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci
-make coccicheck DEBUG_FILE="err.log" MODE=report SPFLAGS="--profile 
--show-trying" M=./drivers/mfd/arizona-irq.c
+make C=2 CHECK=scripts/coccicheck DEBUG_FILE="err.log" MODE=report 
SPFLAGS="--profile --show-trying" ./drivers/mfd/arizona-irq.c
 
 err.log will now have the profiling information, while stdout will
 provide some progress information as Coccinelle moves forward with
-- 
2.25.1

[PATCH v2] scripts: coccicheck: Correct usage of make coccicheck

[Sumera Priyadarsini]

The command "make coccicheck C=1 CHECK=scripts/coccicheck" results in the
error:
./scripts/coccicheck: line 65: -1: shift count out of range

This happens because every time the C variable is specified,
the shell arguments need to be "shifted" in order to take only
the last argument, which is the C file to test. These shell arguments
mostly comprise flags that have been set in the Makefile. However,
when coccicheck is specified in the make command as a rule, the
number of shell arguments is zero, thus passing the invalid value -1
to the shift command, resulting in an error.

Modify coccicheck to print correct usage of make coccicheck so as to
avoid the error.

Signed-off-by: Sumera Priyadarsini 
---
Changes in v2:
- Move test to only display error message
---
 scripts/coccicheck | 12 
 1 file changed, 12 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index 209bb0427b43..f3b8bf505c5f 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -61,6 +61,18 @@ COCCIINCLUDE=${COCCIINCLUDE// -include/ --include}
 if [ "$C" = "1" -o "$C" = "2" ]; then
 ONLINE=1
 
+if [[ $# -le 0 ]]; then
+   echo ''
+   echo 'Specifying both the variable "C" and rule "coccicheck" in the 
make
+command results in a shift count error.'
+   echo ''
+   echo 'Try specifying "scripts/coccicheck" as a value for the CHECK 
variable instead.'
+   echo ''
+   echo 'Example:  make C=2 CHECK=scripts/coccicheck 
drivers/staging/wfx/hi_t.o'
+   echo ''
+   exit 1
+fi
+
 # Take only the last argument, which is the C file to test
 shift $(( $# - 1 ))
 OPTIONS="$COCCIINCLUDE $1"
-- 
2.25.1

[PATCH v2] coccinelle: locks: Add balancedlock.cocci script

[Sumera Priyadarsini]

When acquiring locks under certain conditions, they must be released
under the same conditions as well. However, sometimes, there may be
missing unlocks which may lead to a potential deadlock.

Add this script to detect such code segments and avoid potential
deadlock situations.

Signed-off-by: Sumera Priyadarsini 
---
Changes in v2(as suggested by Markus):
- Modify usage of position variable
- Modify comments
- Add dependencies for rules
---
 scripts/coccinelle/locks/balancedlock.cocci | 164 
 1 file changed, 164 insertions(+)
 create mode 100644 scripts/coccinelle/locks/balancedlock.cocci

diff --git a/scripts/coccinelle/locks/balancedlock.cocci 
b/scripts/coccinelle/locks/balancedlock.cocci
new file mode 100644
index ..9684a9920f79
--- /dev/null
+++ b/scripts/coccinelle/locks/balancedlock.cocci
@@ -0,0 +1,164 @@
+/// Sometimes, locks that are acquired under certain conditions may have
+/// missing unlocks leading to a potential deadlock situation. This
+/// semantic patch detects such cases.
+//# False positives may be generated due to locks released within a nested
+//# function call or a goto block.
+///
+// Confidence: Moderate
+// Copyright: (C) 2020 Julia Lawall INRIA/LIP6
+// Copyright: (C) 2020 Sumera Priyadarsini
+
+virtual context
+virtual org
+virtual report
+
+
+@prelocked@
+expression E;
+position p;
+@@
+
+(
+mutex_lock@p(E);
+|
+read_lock@p(E);
+|
+write_lock@p(E);
+|
+spin_lock@p(E);
+|
+spin_lock_bh@p(E);
+|
+spin_lock_irqsave@p(E, ...);
+|
+read_lock_irqsave@p(E, ...);
+|
+write_lock_irqsave@p(E, ...);
+|
+raw_spin_lock@p(E);
+|
+raw_spin_lock_irq@p(E);
+|
+raw_spin_lock_bh@p(E);
+|
+local_lock@p(E);
+|
+local_lock_irq@p(E);
+|
+local_lock_irqsave@p(E, ...);
+|
+read_lock_irq@p(E);
+|
+read_lock_bh@p(E);
+|
+write_lock_bh@p(E);
+)
+
+@balanced@ depends on context || org || report@
+position prelocked.p;
+position pif;
+expression e,prelocked.E;
+statement S1,S2;
+identifier lock;
+identifier unlock={mutex_unlock,
+   spin_unlock,
+   spin_unlock_bh,
+   spin_unlock_irqrestore,
+   read_unlock_irqrestore,
+   write_unlock_irqrestore,
+   raw_spin_unlock,
+   raw_spin_unlock_irq,
+   raw_spin_unlock_bh,
+   local_unlock,
+   local_unlock_irq,
+   local_unlock_irqrestore,
+   read_unlock_irq,
+   read_unlock_bh,
+   write_unlock_bh
+   };
+@@
+
+if (e) {
+ ... when any
+lock@p(E, ...)
+ ... when != E
+ when any
+} else S1
+... when != E
+when any
+if@pif (e) {
+ ... when != E
+ when any
+ unlock(E, ...);
+ ... when any
+} else S2
+...  when != E
+ when any
+
+// 
+
+@balanced2 depends on context || org || report@
+identifier lock, unlock = {mutex_unlock,
+   spin_unlock,
+   spin_unlock_bh,
+   spin_unlock_irqrestore,
+   read_unlock_irqrestore,
+   write_unlock_irqrestore,
+   raw_spin_unlock,
+   raw_spin_unlock_irq,
+   raw_spin_unlock_bh,
+   local_unlock,
+   local_unlock_irq,
+   local_unlock_irqrestore,
+   read_unlock_irq,
+   read_unlock_bh,
+   write_unlock_bh
+   };
+expression E, f, x;
+statement S1, S2, S3, S4;
+position prelocked.p, balanced.pif;
+position j0, j1, j2, j3;
+@@
+
+* lock@j0@p(E, ...);
+... when != E;
+when != if@pif (...) S1 else S2
+when any
+x@j1 = f(...);
+* if (<+...x...+>)
+{
+  ... when != E;
+  when forall
+  when != if@pif (...) S3 else S4
+*  return@j2 ...;
+}
+... when any
+* unlock@j3(E, ...);
+
+// 
+
+@script:python balanced2_org depends on org@
+j0 << balanced2.j0;
+j1 << balanced2.j1;
+j2 << balanced2.j2;
+j3 << balanced2.j3;
+@@
+
+msg = "This code segment might have an unbalanced lock."
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+coccilib.org.print_link(j2[0], "")
+coccilib.org.print_link(j3[0], "")
+
+// 
+
+@script:python balanced2_report depends on report@
+j0 << balanced2.j0;
+j1 << balanced2.j1;
+j2 << balanced2.j2;
+j3 << balanced2.j3;
+@@
+
+msg = "This code segment might have an unbalanced lock around lines %s,%s,%s." 
% (j1[0].line,j2[0].line,j3[0].line)
+coccilib.report.print_report(j0[0], msg)
+
-- 
2.25.1

[PATCH] scripts: coccicheck: Correct usage of make coccicheck

[Sumera Priyadarsini]

The command "make coccicheck C=1 CHECK=scripts/coccicheck" results in the
error:
./scripts/coccicheck: line 65: -1: shift count out of range

This happens because every time the C variable is specified,
the shell arguments need to be "shifted" in order to take only
the last argument, which is the C file to test. These shell arguments
mostly comprise flags that have been set in the Makefile. However,
when coccicheck is specified in the make command as a rule, the
number of shell arguments is zero, thus passing the invalid value -1
to the shift command, resulting in an error.

Modify coccicheck to print correct usage of make coccicheck so as to
avoid the error.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 12 
 1 file changed, 12 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index 209bb0427b43..b990c8a60a94 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -63,6 +63,18 @@ if [ "$C" = "1" -o "$C" = "2" ]; then
 
 # Take only the last argument, which is the C file to test
 shift $(( $# - 1 ))
+err=$?
+if [[ $err -ne 0 ]]; then
+   echo ''
+   echo 'Specifying both the variable "C" and rule "coccicheck" in the 
make
+command results in a shift count error.'
+   echo ''
+   echo 'Try specifying "scripts/coccicheck" as a value for the CHECK 
variable instead.'
+   echo ''
+   echo 'Example:  make C=2 CHECK=scripts/coccicheck 
drivers/staging/wfx/hi_t.o'
+   echo ''
+   exit 1
+fi
 OPTIONS="$COCCIINCLUDE $1"
 
 # No need to parallelize Coccinelle since this mode takes one input file.
-- 
2.25.1

[PATCH] coccinelle: locks: Add balancedlock.cocci script

[Sumera Priyadarsini]

When acquiring locks under certain conditions, they must be released
under the same conditions as well. However, sometimes, there may be
missing unlocks which may lead to a potential deadlock.

Add this script to detect such code segments and avoid potential
deadlock situations.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccinelle/locks/balancedlock.cocci | 162 
 1 file changed, 162 insertions(+)
 create mode 100644 scripts/coccinelle/locks/balancedlock.cocci

diff --git a/scripts/coccinelle/locks/balancedlock.cocci 
b/scripts/coccinelle/locks/balancedlock.cocci
new file mode 100644
index ..fe7bc2dfeb29
--- /dev/null
+++ b/scripts/coccinelle/locks/balancedlock.cocci
@@ -0,0 +1,162 @@
+/// Sometimes, locks that are acquired under certain conditions may have 
missing unlocks
+/// leading to a potential deadlock situation. This patch detects such cases.
+//# False positives may be generated due to locks released within a nested
+//# function call or a goto block.
+///
+// Confidence: Moderate
+// Copyright: (C) 2020 Julia Lawall INRIA/LIP6
+
+virtual context
+virtual org
+virtual report
+
+
+@prelocked@
+expression E;
+position p;
+@@
+
+(
+mutex_lock(E@p);
+|
+read_lock(E@p);
+|
+write_lock(E@p);
+|
+spin_lock(E@p);
+|
+spin_lock_bh(E@p);
+|
+spin_lock_irqsave(E@p, ...);
+|
+read_lock_irqsave(E@p, ...);
+|
+write_lock_irqsave(E@p, ...);
+|
+raw_spin_lock(E@p);
+|
+raw_spin_lock_irq(E@p);
+|
+raw_spin_lock_bh(E@p);
+|
+local_lock(E@p);
+|
+local_lock_irq(E@p);
+|
+local_lock_irqsave(E@p, ...);
+|
+read_lock_irq(E@p);
+|
+read_lock_bh(E@p);
+|
+write_lock_bh(E@p);
+)
+
+@balanced@
+position prelocked.p;
+position pif;
+expression e,prelocked.E;
+statement S1,S2;
+identifier lock;
+identifier unlock={mutex_unlock,
+   spin_unlock,
+   spin_unlock_bh,
+   spin_unlock_irqrestore,
+   read_unlock_irqrestore,
+   write_unlock_irqrestore,
+   raw_spin_unlock,
+   raw_spin_unlock_irq,
+   raw_spin_unlock_bh,
+   local_unlock,
+   local_unlock_irq,
+   local_unlock_irqrestore,
+   read_unlock_irq,
+   read_unlock_bh,
+   write_unlock_bh
+   };
+@@
+
+if (e) {
+ ... when any
+lock(E@p, ...)
+ ... when != E
+ when any
+} else S1
+... when != E
+when any
+if@pif (e) {
+ ... when != E
+ when any
+ unlock(E, ...);
+ ... when any
+} else S2
+...  when != E
+ when any
+
+// 
+
+@balanced2 depends on context || org || report@
+identifier lock, unlock = {mutex_unlock,
+   spin_unlock,
+   spin_unlock_bh,
+   spin_unlock_irqrestore,
+   read_unlock_irqrestore,
+   write_unlock_irqrestore,
+   raw_spin_unlock,
+   raw_spin_unlock_irq,
+   raw_spin_unlock_bh,
+   local_unlock,
+   local_unlock_irq,
+   local_unlock_irqrestore,
+   read_unlock_irq,
+   read_unlock_bh,
+   write_unlock_bh
+   };
+expression E, f, x;
+statement S1, S2, S3, S4;
+position prelocked.p, balanced.pif;
+position j0, j1, j2, j3;
+@@
+
+* lock@j0(E@p, ...);
+... when != E;
+when != if@pif (...) S1 else S2
+when any
+x@j1 = f(...);
+* if (<+...x...+>)
+{
+  ... when != E;
+  when forall
+  when != if@pif (...) S3 else S4
+*  return@j2 ...;
+}
+... when any
+* unlock@j3(E, ...);
+
+// 
+
+@script:python balanced2_org depends on org@
+j0 << balanced2.j0;
+j1 << balanced2.j1;
+j2 << balanced2.j2;
+j3 << balanced2.j3;
+@@
+
+msg = "This code segment might have an unbalanced lock."
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+coccilib.org.print_link(j2[0], "")
+coccilib.org.print_link(j3[0], "")
+
+// 
+
+@script:python balanced2_report depends on report@
+j0 << balanced2.j0;
+j1 << balanced2.j1;
+j2 << balanced2.j2;
+j3 << balanced2.j3;
+@@
+
+msg = "This code segment might have an unbalanced lock around lines %s,%s,%s." 
% (j1[0].line,j2[0].line,j3[0].line)
+coccilib.report.print_report(j0[0], msg)
+
-- 
2.25.1

[Outreachy][PATCH] drm/amdgpu: use true and false for bool initialisations

[Sumera Priyadarsini]

Bool initialisation should use 'true' and 'false' values instead of 0
and 1.

Modify amdgpu_amdkfd_gpuvm.c to initialise variable is_imported
to false instead of 0.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 64d4b5ff95d6..ba4bd06bfcc5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1288,7 +1288,7 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
struct ttm_validate_buffer *bo_list_entry;
unsigned int mapped_to_gpu_memory;
int ret;
-   bool is_imported = 0;
+   bool is_imported = false;
 
mutex_lock(&mem->lock);
mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
-- 
2.25.1

[PATCH 5/5] gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()

[Sumera Priyadarsini]

Using snprintf() for show() methods holds the risk of buffer overrun
as snprintf() does not know the PAGE_SIZE maximum of the temporary
buffer used to output sysfs content.

Modify amdgpu_ras.c to use sysfs_emit() instead which knows the
size of the temporary buffer.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index e5ea14774c0c..6d9901e1b4b0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -429,13 +429,13 @@ static ssize_t amdgpu_ras_sysfs_read(struct device *dev,
};
 
if (!amdgpu_ras_get_error_query_ready(obj->adev))
-   return snprintf(buf, PAGE_SIZE,
+   return sysfs_emit(buf, PAGE_SIZE,
"Query currently inaccessible\n");
 
if (amdgpu_ras_error_query(obj->adev, &info))
return -EINVAL;
 
-   return snprintf(buf, PAGE_SIZE, "%s: %lu\n%s: %lu\n",
+   return sysfs_emit(buf, PAGE_SIZE, "%s: %lu\n%s: %lu\n",
"ue", info.ue_count,
"ce", info.ce_count);
 }
-- 
2.25.1

[PATCH 4/5] gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()

[Sumera Priyadarsini]

Using snprintf() for show() methods holds the risk of buffer overrun
as snprintf() does not know the PAGE_SIZE maximum of the temporary
buffer used to output sysfs content.

Modify amdgpu_psp.c to use sysfs_emit() instead which knows the
size of the temporary buffer.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index d6c38e24f130..4d1d1e1b005d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -2621,7 +2621,7 @@ static ssize_t psp_usbc_pd_fw_sysfs_read(struct device 
*dev,
return ret;
}
 
-   return snprintf(buf, PAGE_SIZE, "%x\n", fw_ver);
+   return sysfs_emit(buf, PAGE_SIZE, "%x\n", fw_ver);
 }
 
 static ssize_t psp_usbc_pd_fw_sysfs_write(struct device *dev,
-- 
2.25.1

[PATCH 2/5] gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()

[Sumera Priyadarsini]

Using snprintf() for show() methods holds the risk of buffer overrun
as snprintf() does not know the PAGE_SIZE maximum of the temporary
buffer used to output sysfs content.

Modify amdgpu_device.c to use sysfs_emit() instead which knows the
size of the temporary buffer.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f7307af76452..7eef6b20578f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -135,7 +135,7 @@ static ssize_t amdgpu_device_get_pcie_replay_count(struct 
device *dev,
struct amdgpu_device *adev = drm_to_adev(ddev);
uint64_t cnt = amdgpu_asic_get_pcie_replay_count(adev);
 
-   return snprintf(buf, PAGE_SIZE, "%llu\n", cnt);
+   return sysfs_emit(buf, PAGE_SIZE, "%llu\n", cnt);
 }
 
 static DEVICE_ATTR(pcie_replay_count, S_IRUGO,
@@ -159,7 +159,7 @@ static ssize_t amdgpu_device_get_product_name(struct device 
*dev,
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
 
-   return snprintf(buf, PAGE_SIZE, "%s\n", adev->product_name);
+   return sysfs_emit(buf, PAGE_SIZE, "%s\n", adev->product_name);
 }
 
 static DEVICE_ATTR(product_name, S_IRUGO,
@@ -181,7 +181,7 @@ static ssize_t amdgpu_device_get_product_number(struct 
device *dev,
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
 
-   return snprintf(buf, PAGE_SIZE, "%s\n", adev->product_number);
+   return sysfs_emit(buf, PAGE_SIZE, "%s\n", adev->product_number);
 }
 
 static DEVICE_ATTR(product_number, S_IRUGO,
@@ -203,7 +203,7 @@ static ssize_t amdgpu_device_get_serial_number(struct 
device *dev,
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
 
-   return snprintf(buf, PAGE_SIZE, "%s\n", adev->serial);
+   return sysfs_emit(buf, PAGE_SIZE, "%s\n", adev->serial);
 }
 
 static DEVICE_ATTR(serial_number, S_IRUGO,
-- 
2.25.1

[PATCH 1/5] gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()

[Sumera Priyadarsini]

Using snprintf() for show() methods holds the risk of buffer overrun
as snprintf() does not know the PAGE_SIZE maximum of the temporary
buffer used to output sysfs content.

Modify amdgpu_atombios.c to use sysfs_emit() instead which knows the
size of the temporary buffer.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index 469352e2d6ec..3c19862c94c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -1947,7 +1947,7 @@ static ssize_t amdgpu_atombios_get_vbios_version(struct 
device *dev,
struct amdgpu_device *adev = drm_to_adev(ddev);
struct atom_context *ctx = adev->mode_info.atom_context;
 
-   return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
+   return sysfs_emit(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
 }
 
 static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
-- 
2.25.1

[Outreachy kernel][PATCH 0/5] drm/amdgpu: Replace snprintf() with sysfs_emit

[Sumera Priyadarsini]

Using snprintf() for show() methods holds the risk of buffer overrun
as snprintf() does not know the PAGE_SIZE maximum of the temporary
buffer used to output sysfs content.

This patchset is a series of Coccinelle cleanups across the staging
directory to convert snprintf with scnprintf in the relevant files.

Sumera Priyadarsini (5):
  gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()
  gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()
  gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()
  gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()
  gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()

 drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 8 
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c  | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.25.1

[Outreachy kernel][PATCH] gpu: amd: Return boolean types instead of integer values

[Sumera Priyadarsini]

Return statements for functions returning bool should use truth
and false instead of 1 and 0 respectively.

Modify cik_event_interrupt.c to return false instead of 0.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c 
b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
index 24b471734117..8e64c01565ac 100644
--- a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
+++ b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
@@ -66,12 +66,12 @@ static bool cik_event_interrupt_isr(struct kfd_dev *dev,
vmid  = (ihre->ring_id & 0xff00) >> 8;
if (vmid < dev->vm_info.first_vmid_kfd ||
vmid > dev->vm_info.last_vmid_kfd)
-   return 0;
+   return false;
 
/* If there is no valid PASID, it's likely a firmware bug */
pasid = (ihre->ring_id & 0x) >> 16;
if (WARN_ONCE(pasid == 0, "FW bug: No PASID in KFD interrupt"))
-   return 0;
+   return false;
 
/* Interrupt types we care about: various signals and faults.
 * They will be forwarded to a work queue (see below).
-- 
2.25.1

[PATCH V3] [Cocci] coccinelle: iterators: Add for_each_child.cocci script

[Sumera Priyadarsini]

While iterating over child nodes with the for_each functions, if
control is transferred from the middle of the loop, as in the case
of a break or return or goto, there is no decrement in the
reference counter thus ultimately resulting in a memory leak.

Add this script to detect potential memory leaks caused by
the absence of of_node_put() before break, goto, or, return
statements which transfer control outside the loop.

Signed-off-by: Sumera Priyadarsini 


Changes in V2:
- Add options --include-headers and --no-includes
- Add 'when forall` to rules for break and goto

Changes in V3:
- Add return case
---
 .../coccinelle/iterators/for_each_child.cocci | 358 ++
 1 file changed, 358 insertions(+)
 create mode 100644 scripts/coccinelle/iterators/for_each_child.cocci

diff --git a/scripts/coccinelle/iterators/for_each_child.cocci 
b/scripts/coccinelle/iterators/for_each_child.cocci
new file mode 100644
index ..bc394615948e
--- /dev/null
+++ b/scripts/coccinelle/iterators/for_each_child.cocci
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Adds missing of_node_put() before return/break/goto statement within a 
for_each iterator for child nodes.
+//# False positives can be due to function calls within the for_each
+//# loop that may encapsulate an of_node_put.
+///
+// Confidence: High
+// Copyright: (C) 2020 Sumera Priyadarsini
+// URL: http://coccinelle.lip6.fr
+// Options: --no-includes --include-headers
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r@
+local idexpression n;
+expression e1,e2;
+iterator name for_each_node_by_name, for_each_node_by_type,
+for_each_compatible_node, for_each_matching_node,
+for_each_matching_node_and_match, for_each_child_of_node,
+for_each_available_child_of_node, for_each_node_with_property;
+iterator i;
+statement S;
+expression list [n1] es;
+@@
+
+(
+(
+for_each_node_by_name(n,e1) S
+|
+for_each_node_by_type(n,e1) S
+|
+for_each_compatible_node(n,e1,e2) S
+|
+for_each_matching_node(n,e1) S
+|
+for_each_matching_node_and_match(n,e1,e2) S
+|
+for_each_child_of_node(e1,n) S
+|
+for_each_available_child_of_node(e1,n) S
+|
+for_each_node_with_property(n,e1) S
+)
+&
+i(es,n,...) S
+)
+
+@ruleone depends on patch && !context && !org && !report@
+
+local idexpression r.n;
+iterator r.i,i1;
+expression e;
+expression list [r.n1] es;
+statement S;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   return n;
+|
+   i1(...,n,...) S
+|
+- return of_node_get(n);
++ return n;
+|
++  of_node_put(n);
+?  return ...;
+)
+   ... when any
+ }
+
+@ruletwo depends on patch && !context && !org && !report@
+
+local idexpression r.n;
+iterator r.i,i1,i2;
+expression e,e1;
+expression list [r.n1] es;
+statement S,S2;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
++  of_node_put(n);
+?  break;
+)
+   ... when any
+ }
+... when != n
+when strict
+when forall
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+@rulethree depends on patch && !context && !org && !report exists@
+
+local idexpression r.n;
+iterator r.i,i1,i2;
+expression e,e1;
+identifier l;
+expression list [r.n1] es;
+statement S,S2;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
++  of_node_put(n);
+?  goto l;
+)
+   ... when any
+ }
+... when exists
+l: ... when != n
+   when strict
+   when forall
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+// 
+
+@ruleone_context depends on !patch && (context || org || report) exists@
+statement S;
+expression e;
+expression list[r.n1] es;
+iterator r.i, i1;
+local idexpression r.n;
+position j0, j1;
+@@
+
+ i@j0(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   return n;
+|
+   i1(...,n,...) S
+|
+  return @j1 ...;
+)
+   ... when any
+ }
+
+@ruleone_disj depends on !patch && (context || org || report)@
+expression list[r.n1] es;
+iterator r.i;
+local idexpression r.n;
+position ruleone_context.j0, ruleone_context.j1;
+@@
+
+*  i@j0(es,n,...) {
+   ...
+*return  @j1...;
+   ... when any
+ }
+
+@ruletwo_context depends on !patch && (context || org || report) exists@
+statement S, S2;
+expression e, e1;
+expression list[r.n1] es;
+iterator r.i, i1, i2;
+local idexpression r.n;
+position j0, j2;
+@@
+
+ i@j0(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
+  break@j2;
+)
+   ... when any
+ }
+... when != n
+when strict
+when forall
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+@ruletwo_disj depends on !patch && (context || org || report)@
+statement S2;
+expression e1;
+expression list[r.n1] es;
+iterator r.i, i2;
+local idexpression r.n;
+position ruletwo_context.j0, ruletwo_context.j2;
+@@
+
+*  i@j0(es,n,...) {
+   ...
+*break @j2;
+   ... when any
+ }
+... when != n
+when s

[PATCH V2] [Cocci] coccinelle: iterators: Add for_each_child.cocci script

[Sumera Priyadarsini]

While iterating over child nodes with the for_each functions, if
control is transferred from the middle of the loop, as in the case
of a break or return or goto, there is no decrement in the
reference counter thus ultimately resulting in a memory leak.

Add this script to detect potential memory leaks caused by
the absence of of_node_put() before break, goto, or, return
statements which transfer control outside the loop.

Signed-off-by: Sumera Priyadarsini 


Changes in V2:
- Add options --include-headers and --no-includes
- Add 'when forall` to rules for break and goto
---
 .../coccinelle/iterators/for_each_child.cocci | 355 ++
 1 file changed, 355 insertions(+)
 create mode 100644 scripts/coccinelle/iterators/for_each_child.cocci

diff --git a/scripts/coccinelle/iterators/for_each_child.cocci 
b/scripts/coccinelle/iterators/for_each_child.cocci
new file mode 100644
index ..57caf9b8d7a5
--- /dev/null
+++ b/scripts/coccinelle/iterators/for_each_child.cocci
@@ -0,0 +1,355 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Adds missing of_node_put() before return/break/goto statement within a 
for_each iterator for child nodes.
+//# False positives can be due to function calls within the for_each
+//# loop that may encapsulate an of_node_put.
+///
+// Confidence: High
+// Copyright: (C) 2020 Sumera Priyadarsini
+// URL: http://coccinelle.lip6.fr
+// Options: --no-includes --include-headers
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r@
+local idexpression n;
+expression e1,e2;
+iterator name for_each_node_by_name, for_each_node_by_type,
+for_each_compatible_node, for_each_matching_node,
+for_each_matching_node_and_match, for_each_child_of_node,
+for_each_available_child_of_node, for_each_node_with_property;
+iterator i;
+statement S;
+expression list [n1] es;
+@@
+
+(
+(
+for_each_node_by_name(n,e1) S
+|
+for_each_node_by_type(n,e1) S
+|
+for_each_compatible_node(n,e1,e2) S
+|
+for_each_matching_node(n,e1) S
+|
+for_each_matching_node_and_match(n,e1,e2) S
+|
+for_each_child_of_node(e1,n) S
+|
+for_each_available_child_of_node(e1,n) S
+|
+for_each_node_with_property(n,e1) S
+)
+&
+i(es,n,...) S
+)
+
+@ruleone depends on patch && !context && !org && !report@
+
+local idexpression r.n;
+iterator r.i,i1;
+expression e;
+expression list [r.n1] es;
+statement S;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   return n;
+|
+   i1(...,n,...) S
+|
++  of_node_put(n);
+?  return ...;
+)
+   ... when any
+ }
+
+@ruletwo depends on patch && !context && !org && !report@
+
+local idexpression r.n;
+iterator r.i,i1,i2;
+expression e,e1;
+expression list [r.n1] es;
+statement S,S2;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
++  of_node_put(n);
+?  break;
+)
+   ... when any
+ }
+... when != n
+when strict
+when forall
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+@rulethree depends on patch && !context && !org && !report exists@
+
+local idexpression r.n;
+iterator r.i,i1,i2;
+expression e,e1;
+identifier l;
+expression list [r.n1] es;
+statement S,S2;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
++  of_node_put(n);
+?  goto l;
+)
+   ... when any
+ }
+... when exists
+l: ... when != n
+   when strict
+   when forall
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+// 
+
+@ruleone_context depends on !patch && (context || org || report) exists@
+statement S;
+expression e;
+expression list[r.n1] es;
+iterator r.i, i1;
+local idexpression r.n;
+position j0, j1;
+@@
+
+ i@j0(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   return n;
+|
+   i1(...,n,...) S
+|
+  return @j1 ...;
+)
+   ... when any
+ }
+
+@ruleone_disj depends on !patch && (context || org || report)@
+expression list[r.n1] es;
+iterator r.i;
+local idexpression r.n;
+position ruleone_context.j0, ruleone_context.j1;
+@@
+
+*  i@j0(es,n,...) {
+   ...
+*return  @j1...;
+   ... when any
+ }
+
+@ruletwo_context depends on !patch && (context || org || report) exists@
+statement S, S2;
+expression e, e1;
+expression list[r.n1] es;
+iterator r.i, i1, i2;
+local idexpression r.n;
+position j0, j2;
+@@
+
+ i@j0(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
+  break@j2;
+)
+   ... when any
+ }
+... when != n
+when strict
+when forall
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+@ruletwo_disj depends on !patch && (context || org || report)@
+statement S2;
+expression e1;
+expression list[r.n1] es;
+iterator r.i, i2;
+local idexpression r.n;
+position ruletwo_context.j0, ruletwo_context.j2;
+@@
+
+*  i@j0(es,n,...) {
+   ...
+*break @j2;
+   ... when any
+ }
+... when != n
+when strict
+when forall
+(
+  n = e1;
+|
+?i2(...,n,...) S2
+)
+
+@rulethree_

[PATCH v4 3/3] Documentation: Coccinelle: Modify Parallelisation information in docs

[Sumera Priyadarsini]

This patchset modifies coccicheck to use at most one thread per core by
default in machines with more than 4 hyperthreads for optimal performance.
Modify documentation in coccinelle.rst to reflect the same.

Signed-off-by: Sumera Priyadarsini 
---
 Documentation/dev-tools/coccinelle.rst | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/dev-tools/coccinelle.rst 
b/Documentation/dev-tools/coccinelle.rst
index 74c5e6aeeff5..530d8d313601 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -130,8 +130,10 @@ To enable verbose messages set the V= variable, for 
example::
 Coccinelle parallelization
 --
 
-By default, coccicheck tries to run as parallel as possible. To change
-the parallelism, set the J= variable. For example, to run across 4 CPUs::
+By default, coccicheck uses at most 1 thread per core in a machine
+with more than 4 hyperthreads. In a machine with upto 4 threads,
+all threads are used. To change the parallelism, set the J= variable.
+For example, to run across 4 CPUs::
 
make coccicheck MODE=report J=4
 
-- 
2.25.1

[PATCH v4 2/3] scripts: coccicheck: Change default condition for parallelism

[Sumera Priyadarsini]

Currently, Coccinelle uses at most one thread per core by default in
machines with more than 2 hyperthreads. However, for systems with only 4
hyperthreads, this does not improve performance.

Modify coccicheck to use all available threads in machines with
upto 4 hyperthreads.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index d67907b8a38b..209bb0427b43 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -79,7 +79,7 @@ else
 THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd 
"[:digit:]")
 if [ -z "$J" ]; then
 NPROC=$(getconf _NPROCESSORS_ONLN)
-   if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 2 ] ; then
+   if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 4 ] ; then
NPROC=$((NPROC/2))
fi
 else
-- 
2.25.1

[PATCH v4 1/3] scripts: coccicheck: Add quotes to improve portability

[Sumera Priyadarsini]

While fetching the number of threads per core with lscpu,
the [:digit:] set is used for translation of digits from 0-9.
However, using [:digit:] instead of "[:digit:]" does not seem
to work uniformly for some shell types and configurations
(such as zsh).

Therefore, modify coccicheck to use double quotes around the
[:digit:] set for uniformity and better portability.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index 6789751607f5..d67907b8a38b 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -76,7 +76,7 @@ else
 fi
 
 # Use only one thread per core by default if hyperthreading is enabled
-THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd [:digit:])
+THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd 
"[:digit:]")
 if [ -z "$J" ]; then
 NPROC=$(getconf _NPROCESSORS_ONLN)
if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 2 ] ; then
-- 
2.25.1

[PATCH v4 0/3] Improve Coccinelle Parallelisation

[Sumera Priyadarsini]

Presently, Coccinelle uses at most one thread per core to improve
performance in machines with more than 2 hyperthreads. Modify
coccicheck to use all available threads in machines upto 4 hyperthreads.
Further, modify the coccicheck script to improve portability.

Modify documentation to reflect the same. 

Sumera Priyadarsini (3):
  scripts: coccicheck: Add quotes to improve portability
  scripts: coccicheck: Change default condition for parallelism
  Documentation: Coccinelle: Modify Parallelisation information in docs

 Documentation/dev-tools/coccinelle.rst | 6 --
 scripts/coccicheck | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH 2/2 V3] Documentation: Coccinelle: Modify parallelisation information in docs

[Sumera Priyadarsini]

This patchset modifies coccicheck to use at most one thread per core by
default in machines with more than 4 hyperthreads for optimal performance.
Modify documentation in coccinelle.rst to reflect the same.

Signed-off-by: Sumera Priyadarsini 
---
Changes in V2:
Update scripts/coccicheck to use all available threads
in machines with upto 4 hyperthreads.
---
 Documentation/dev-tools/coccinelle.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/dev-tools/coccinelle.rst 
b/Documentation/dev-tools/coccinelle.rst
index 74c5e6aeeff5..6fdc462689d5 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -130,8 +130,9 @@ To enable verbose messages set the V= variable, for 
example::
 Coccinelle parallelization
 --
 
-By default, coccicheck tries to run as parallel as possible. To change
-the parallelism, set the J= variable. For example, to run across 4 CPUs::
+By default, coccicheck uses at most 1 thread per core in a machine with more
+than 4 hyperthreads. In a machine with upto 4 threads, all threads are used.
+To change the parallelism, set the J= variable. For example, to run across 4 
CPUs::
 
make coccicheck MODE=report J=4
 
-- 
2.25.1

[PATCH 1/2 V3] scripts: coccicheck: Change default value for parallelism

[Sumera Priyadarsini]

By default, coccicheck utilizes all available threads to implement
parallelisation. However, when all available threads are used,
a decrease in performance is noted. The elapsed time is  minimum
when at most one thread per core is used.

For example, on benchmarking the semantic patch kfree.cocci for
usb/serial using hyperfine, the outputs obtained for J=5 and J=2
are 1.32 and 1.90 times faster than those for J=10 and J=9
respectively for two separate runs. For the larger drivers/staging
directory, minimium elapsed time is obtained for J=3 which is 1.86
times faster than that for J=12. The optimal J value does not
exceed 6 in any of the test runs. The benchmarks are run on a machine
with 6 cores, with 2 threads per core, i.e, 12 hyperthreads in all.

To improve performance, modify coccicheck to use at most only
one thread per core by default in machines with more than 4
hyperthreads.

Signed-off-by: Sumera Priyadarsini 

---
Changes in V2:
- Change commit message as suggested by Julia Lawall
Changes in V3:
- Use J/2 as optimal value for machines with more
than 8 hyperthreads as well.
Changes in V4:
- Use J as optimal value for machines with less than or
equal to 4 hyperthreads.
---
 scripts/coccicheck | 5 +
 1 file changed, 5 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..bafc55141a73 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -75,8 +75,13 @@ else
 OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
 fi
 
+# Use only one thread per core by default if hyperthreading is enabled
+THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd 
"[:digit:]")
 if [ -z "$J" ]; then
 NPROC=$(getconf _NPROCESSORS_ONLN)
+   if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 4 ] ; then
+   NPROC=$((NPROC/2))
+   fi
 else
 NPROC="$J"
 fi
-- 
2.25.1

[PATCH 0/2 V3] Improve Coccinelle Parallelisation

[Sumera Priyadarsini]

Coccinelle utilises all available threads to implement parallelisation.
However, this results in a decrease in performance.

This patchset aims to improve performance by modifying cocciccheck to
use at most one thread per core by default in machines with more than 4
hyperthreads.

Sumera Priyadarsini (2):
  scripts: coccicheck: Change default value for parallelism
  Documentation: Coccinelle: Modify parallelisation information in docs

 Documentation/dev-tools/coccinelle.rst | 5 +++--
 scripts/coccicheck | 5 +
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 2/2 V2] Documentation: Coccinelle: Modify parallelisation information in docs

[Sumera Priyadarsini]

This patchset modifies coccicheck to use at most one thread per core by
default for optimal performance. Modify documentation in coccinelle.rst
to reflect the same.

Signed-off-by: Sumera Priyadarsini 
---
 Documentation/dev-tools/coccinelle.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/dev-tools/coccinelle.rst 
b/Documentation/dev-tools/coccinelle.rst
index 74c5e6aeeff5..a27a4867018c 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -130,8 +130,8 @@ To enable verbose messages set the V= variable, for 
example::
 Coccinelle parallelization
 --
 
-By default, coccicheck tries to run as parallel as possible. To change
-the parallelism, set the J= variable. For example, to run across 4 CPUs::
+By default, coccicheck uses at most only one thread per core of the system.
+To change the parallelism, set the J= variable. For example, to run across 4 
CPUs::
 
make coccicheck MODE=report J=4
 
-- 
2.25.1

[PATCH 1/2 V2] scripts: coccicheck: Change default value for parallelism

[Sumera Priyadarsini]

By default, coccicheck utilizes all available threads to implement
parallelisation. However, when all available threads are used,
a decrease in performance is noted. The elapsed time is  minimum
when at most one thread per core is used.

For example, on benchmarking the semantic patch kfree.cocci for
usb/serial using hyperfine, the outputs obtained for J=5 and J=2
are 1.32 and 1.90 times faster than those for J=10 and J=9
respectively for two separate runs. For the larger drivers/staging
directory, minimium elapsed time is obtained for J=3 which is 1.86
times faster than that for J=12. The optimal J value does not
exceed 6 in any of the test runs. The benchmarks are run on a machine
with 6 cores, with 2 threads per core, i.e, 12 hyperthreads in all.

To improve performance, modify coccicheck to use at most only
one thread per core by default.

Signed-off-by: Sumera Priyadarsini 

---
Changes in V2:
- Change commit message as suggested by Julia Lawall
Changes in V3:
- Use J/2 as optimal value for machines with more
than 8 hyperthreads as well.
---
 scripts/coccicheck | 5 +
 1 file changed, 5 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..a72aa6c037ff 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -75,8 +75,13 @@ else
 OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
 fi
 
+# Use only one thread per core by default if hyperthreading is enabled
+THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd [:digit:])
 if [ -z "$J" ]; then
 NPROC=$(getconf _NPROCESSORS_ONLN)
+   if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 2 ] ; then
+   NPROC=$((NPROC/2))
+   fi
 else
 NPROC="$J"
 fi
-- 
2.25.1

[PATCH 0/2 v2] Improve Coccinelle Parallelisation

[Sumera Priyadarsini]

Coccinelle utilises all available threads to implement parallelisation.
However, this results in a decrease in performance.

This patchset aims to improve performance by modifying cocciccheck to
use at most one thread per core by default.

Sumera Priyadarsini (2):
  scripts: coccicheck: Change default value for parallelism
  Documentation: Coccinelle: Modify parallelisation information in docs

 Documentation/dev-tools/coccinelle.rst | 4 ++--
 scripts/coccicheck | 5 +
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH] scripts: coccicheck: Refactor display messages on coccinelle start up

[Sumera Priyadarsini]

Currently, coccinelle starts by printing
"When using "patch" mode, carefully review the
patch before submitting it."

Modify coccicheck to print this message only when the user has
explicitly selected "patch"  or "chain" mode.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..07d1b5831bf6 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -118,7 +118,9 @@ fi
 if [ "$ONLINE" = "0" ] ; then
 echo ''
 echo 'Please check for false positives in the output before submitting a 
patch.'
-echo 'When using "patch" mode, carefully review the patch before 
submitting it.'
+if [ "$MODE" = "patch" -o "$MODE" = "chain" ] ; then
+echo 'When using "patch" mode, carefully review the patch before 
submitting it.'
+fi
 echo ''
 fi
 
-- 
2.25.1

[PATCH 2/2] Documentation: Coccinelle: Modify parallelisation information in docs

[Sumera Priyadarsini]

This patchset modifies coccicheck to use at most one thread per core by
default for optimal performance. Modify documentation in coccinelle.rst
to reflect the same.

Signed-off-by: Sumera Priyadarsini 
---
 Documentation/dev-tools/coccinelle.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/dev-tools/coccinelle.rst 
b/Documentation/dev-tools/coccinelle.rst
index 74c5e6aeeff5..a27a4867018c 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -130,8 +130,8 @@ To enable verbose messages set the V= variable, for 
example::
 Coccinelle parallelization
 --
 
-By default, coccicheck tries to run as parallel as possible. To change
-the parallelism, set the J= variable. For example, to run across 4 CPUs::
+By default, coccicheck uses at most only one thread per core of the system.
+To change the parallelism, set the J= variable. For example, to run across 4 
CPUs::
 
make coccicheck MODE=report J=4
 
-- 
2.25.1

[PATCH 1/2] scripts: coccicheck: Change default value for parallelism

[Sumera Priyadarsini]

By default, coccicheck utilizes all available threads to implement
parallelisation. However, when all available threads are used,
a decrease in performance is noted. The elapsed time is  minimum
when at most one thread per core is used.

For example, on benchmarking the semantic patch kfree.cocci for
usb/serial using hyperfine, the outputs obtained for J=5 and J=2
are 1.32 and 1.90 times faster than those for J=10 and J=9
respectively for two separate runs. For the larger drivers/staging
directory, minimium elapsed time is obtained for J=3 which is 1.86
times faster than that for J=12. The optimal J value does not
exceed 6 in any of the test runs. The benchmarks are run on a machine
with 6 cores, with 2 threads per core, i.e, 12 hyperthreads in all.

To improve performance, modify coccicheck to use at most only
one thread per core by default.

Signed-off-by: Sumera Priyadarsini 

---
Changes in V2:
- Change commit message as suggested by Julia Lawall
Changes in V3:
- Use J/2 as optimal value for machines with more
than 8 hyperthreads as well.
---
 scripts/coccicheck | 5 +
 1 file changed, 5 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..a72aa6c037ff 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -75,8 +75,13 @@ else
 OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
 fi
 
+# Use only one thread per core by default if hyperthreading is enabled
+THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd [:digit:])
 if [ -z "$J" ]; then
 NPROC=$(getconf _NPROCESSORS_ONLN)
+   if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 2 ] ; then
+   NPROC=$((NPROC/2))
+   fi
 else
 NPROC="$J"
 fi
-- 
2.25.1

[PATCH 0/2] Improve Coccinelle Parallelisation

[Sumera Priyadarsini]

Coccinelle utilises all available threads to implement parallelisation.
However, this results in a decrease in performance.

This patchset aims to improve performance by modifying cocciccheck to
use at most one thread per core by default.

Sumera Priyadarsini (2):
  scripts: coccicheck: Change default value for parallelism
  Documentation: Coccinelle: Modify parallelisation information in docs

 Documentation/dev-tools/coccinelle.rst | 4 ++--
 scripts/coccicheck | 5 +
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH] [Cocci] coccinelle: iterators: Add for_each_child.cocci script

[Sumera Priyadarsini]

While iterating over child nodes with the for_each functions, if
control is transferred from the middle of the loop, as in the case
of a break or return or goto, there is no decrement in the
reference counter thus ultimately resulting in a memory leak.

Add this script to detect potential memory leaks caused by
the absence of of_node_put() before break, goto, or, return
statements which transfer control outside the loop.

Signed-off-by: Sumera Priyadarsini 
---
 .../coccinelle/iterators/for_each_child.cocci | 348 ++
 1 file changed, 348 insertions(+)
 create mode 100644 scripts/coccinelle/iterators/for_each_child.cocci

diff --git a/scripts/coccinelle/iterators/for_each_child.cocci 
b/scripts/coccinelle/iterators/for_each_child.cocci
new file mode 100644
index ..0abc12ca2ad3
--- /dev/null
+++ b/scripts/coccinelle/iterators/for_each_child.cocci
@@ -0,0 +1,348 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Adds missing of_node_put() before return/break/goto statement within a 
for_each iterator for child nodes.
+//# False positives can be due to function calls within the for_each
+//# loop that may encapsulate an of_node_put.
+///
+// Confidence: High
+// Copyright: (C) 2020 Sumera Priyadarsini
+// URL: http://coccinelle.lip6.fr
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r@
+local idexpression n;
+expression e1,e2;
+iterator name for_each_node_by_name, for_each_node_by_type,
+for_each_compatible_node, for_each_matching_node,
+for_each_matching_node_and_match, for_each_child_of_node,
+for_each_available_child_of_node, for_each_node_with_property;
+iterator i;
+statement S;
+expression list [n1] es;
+@@
+
+(
+(
+for_each_node_by_name(n,e1) S
+|
+for_each_node_by_type(n,e1) S
+|
+for_each_compatible_node(n,e1,e2) S
+|
+for_each_matching_node(n,e1) S
+|
+for_each_matching_node_and_match(n,e1,e2) S
+|
+for_each_child_of_node(e1,n) S
+|
+for_each_available_child_of_node(e1,n) S
+|
+for_each_node_with_property(n,e1) S
+)
+&
+i(es,n,...) S
+)
+
+@ruleone depends on patch && !context && !org && !report@
+
+local idexpression r.n;
+iterator r.i,i1;
+expression e;
+expression list [r.n1] es;
+statement S;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   return n;
+|
+   i1(...,n,...) S
+|
++  of_node_put(n);
+?  return ...;
+)
+   ... when any
+ }
+
+@ruletwo depends on patch && !context && !org && !report@
+
+local idexpression r.n;
+iterator r.i,i1,i2;
+expression e,e1;
+expression list [r.n1] es;
+statement S,S2;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
++  of_node_put(n);
+?  break;
+)
+   ... when any
+ }
+... when != n
+when strict
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+@rulethree depends on patch && !context && !org && !report exists@
+
+local idexpression r.n;
+iterator r.i,i1,i2;
+expression e,e1;
+identifier l;
+expression list [r.n1] es;
+statement S,S2;
+@@
+
+ i(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
++  of_node_put(n);
+?  goto l;
+)
+   ... when any
+ }
+... when exists
+l: ... when != n
+   when strict
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+// 
+
+@ruleone_context depends on !patch && (context || org || report) exists@
+statement S;
+expression e;
+expression list[r.n1] es;
+iterator r.i, i1;
+local idexpression r.n;
+position j0, j1;
+@@
+
+ i@j0(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   return n;
+|
+   i1(...,n,...) S
+|
+  return @j1 ...;
+)
+   ... when any
+ }
+
+@ruleone_disj depends on !patch && (context || org || report)@
+expression list[r.n1] es;
+iterator r.i;
+local idexpression r.n;
+position ruleone_context.j0, ruleone_context.j1;
+@@
+
+*  i@j0(es,n,...) {
+   ...
+*return  @j1...;
+   ... when any
+ }
+
+@ruletwo_context depends on !patch && (context || org || report) exists@
+statement S, S2;
+expression e, e1;
+expression list[r.n1] es;
+iterator r.i, i1, i2;
+local idexpression r.n;
+position j0, j2;
+@@
+
+ i@j0(es,n,...) {
+   ...
+(
+   of_node_put(n);
+|
+   e = n
+|
+   i1(...,n,...) S
+|
+  break@j2;
+)
+   ... when any
+ }
+... when != n
+when strict
+(
+ n = e1;
+|
+?i2(...,n,...) S2
+)
+
+@ruletwo_disj depends on !patch && (context || org || report)@
+statement S2;
+expression e1;
+expression list[r.n1] es;
+iterator r.i, i2;
+local idexpression r.n;
+position ruletwo_context.j0, ruletwo_context.j2;
+@@
+
+*  i@j0(es,n,...) {
+   ...
+*break @j2;
+   ... when any
+ }
+... when != n
+when strict
+(
+  n = e1;
+|
+?i2(...,n,...) S2
+)
+
+@rulethree_context depends on !patch && (context || org || report) exists@
+identifier l;
+statement S,S2;
+expression e, e1;
+expression list[r.n1] es;
+iterator r.i, i1, i2;
+local idexpression r.n;
+position j0, j3;
+@@
+
+ i@j0(es,n,...) {

[RFC PATCH] scripts: coccicheck: Improve error feedback when coccicheck fails

[Sumera Priyadarsini]

Currently, coccicheck fails with only the message "coccicheck failed"
and the error code for the failure. To obtain the error logs,
one needs to specify a debug file using the DEBUG_FILE option.

Modify coccicheck to display error logs when it crashes unless
DEBUG_FILE is set, in which case, the error logs are stored in
the specified debug file.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..dbeafa21f359 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -126,8 +126,14 @@ run_cmd_parmap() {
if [ $VERBOSE -ne 0 ] ; then
echo "Running ($NPROC in parallel): $@"
fi
-   echo $@ >>$DEBUG_FILE
-   $@ 2>>$DEBUG_FILE
+   if [ "$DEBUG_FILE" != "/dev/null" -a "$DEBUG_FILE" != "" ]; then
+echo $@>>$DEBUG_FILE
+$@ 2>>$DEBUG_FILE
+else
+echo $@
+$@ 2>&1
+   fi
+
err=$?
if [[ $err -ne 0 ]]; then
echo "coccicheck failed"
-- 
2.25.1

[PATCH V3] scripts: coccicheck: Do not use shift command when rule is specified

[Sumera Priyadarsini]

The command "make coccicheck C=1 CHECK=scripts/coccicheck" results in the
error:
./scripts/coccicheck: line 65: -1: shift count out of range

This happens because every time the C variable is specified,
the shell arguments need to be "shifted" in order to take only
the last argument, which is the C file to test. These shell arguments
mostly comprise flags that have been set in the Makefile. However,
when coccicheck is specified in the make command as a rule, the
number of shell arguments is zero, thus passing the invalid value -1
to the shift command, resulting in an error.

Modify coccicheck to use the shift command only when
number of shell arguments is not zero.

Signed-off-by: Sumera Priyadarsini 

---
Changes in V2:
- Fix spelling errors as suggested by Markus Elfring
---
 scripts/coccicheck | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..5c8df337e1e3 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -61,9 +61,19 @@ COCCIINCLUDE=${COCCIINCLUDE// -include/ --include}
 if [ "$C" = "1" -o "$C" = "2" ]; then
 ONLINE=1
 
-# Take only the last argument, which is the C file to test
-shift $(( $# - 1 ))
-OPTIONS="$COCCIINCLUDE $1"
+# If the rule coccicheck is specified when calling make, number of
+# arguments is zero
+if [ $# -ne 0 ]; then
+   # Take only the last argument, which is the C file to test
+   shift $(( $# -1 ))
+   OPTIONS="$COCCIINCLUDE $1"
+else
+   if [ "$KBUILD_EXTMOD" = "" ] ; then
+   OPTIONS="--dir $srctree $COCCIINCLUDE"
+   else
+   OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
+   fi
+fi
 
 # No need to parallelize Coccinelle since this mode takes one input file.
 NPROC=1
-- 
2.25.1

[PATCH V2] scripts: coccicheck: Do not use shift command when rule is specfified

[Sumera Priyadarsini]

The command "make coccicheck C=1 CHECK=scripts/coccicheck" results in the
error:
./scripts/coccicheck: line 65: -1: shift count out of range

This happens because every time the C variable is specified,
the shell arguments need to be "shifted" in order to take only
the last argument, which is the C file to test. These shell arguments
mostly comprise flags that have been set in the Makefile. However,
when coccicheck is specified in the make command as a rule, the
number of shell arguments is zero, thus passing the invalid value -1
to the shift command, resulting in an error.

Modify coccicheck to use the shift command only when
number of shell arguments is not zero.

Signed-off-by: Sumera Priyadarsini 

---
Changes in V2:
- Fix spelling errors as suggested by Markus Elfring
---
 scripts/coccicheck | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..5c8df337e1e3 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -61,9 +61,19 @@ COCCIINCLUDE=${COCCIINCLUDE// -include/ --include}
 if [ "$C" = "1" -o "$C" = "2" ]; then
 ONLINE=1
 
-# Take only the last argument, which is the C file to test
-shift $(( $# - 1 ))
-OPTIONS="$COCCIINCLUDE $1"
+# If the rule coccicheck is specified when calling make, number of
+# arguments is zero
+if [ $# -ne 0 ]; then
+   # Take only the last argument, which is the C file to test
+   shift $(( $# -1 ))
+   OPTIONS="$COCCIINCLUDE $1"
+else
+   if [ "$KBUILD_EXTMOD" = "" ] ; then
+   OPTIONS="--dir $srctree $COCCIINCLUDE"
+   else
+   OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
+   fi
+fi
 
 # No need to parallelize Coccinelle since this mode takes one input file.
 NPROC=1
-- 
2.25.1

Re: [PATCH] scripts: coccicheck: Do not use shift command when rule is specified

[Sumera Priyadarsini]

On Wed, Sep 09, 2020 at 08:52:19AM +0200, Markus Elfring wrote:
> I find it helpful to avoid typos (like the following) in the change 
> description.
> 
> 
> > … Makfeile. …
> 
> … Makefile. …
> 
> 
> > … paasing …
> 
> … passing …
> 
> 
> > …, resuting …
> 
> …, resulting …
> 
> 
> > This patch modifies coccicheck …
> 

I did make those errors but I also rectified them. This is strange
because my commit message shows the rectified version.
Either way, I will send a v2. Thanks for pointing this out.

> Would an imperative wording be preferred for the commit message?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=34d4ddd359dbcdf6c5fb3f85a179243d7a1cb7f8#n151
> 
> Regards,
> Markus

[PATCH] scripts: coccicheck: Do not use shift command when rule is specfified

[Sumera Priyadarsini]

The command "make coccicheck C=1 CHECK=scripts/coccicheck" results in the
error:
./scripts/coccicheck: line 65: -1: shift count out of range

This happens because every time the C variable is specified,
the shell arguments need to be "shifted" in order to take only
the last argument, which is the C file to test. These shell arguments
mostly comprise flags that have been set in the Makfeile. However,
when coccicheck is specified in the make command as a rule,
number of shell arguments is zero, thus paasing the invalid value -1
to the shift command, resuting in an error.

This patch modifies coccicheck to use the shift command only when
number of shell arguments is not zero.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..5c8df337e1e3 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -61,9 +61,19 @@ COCCIINCLUDE=${COCCIINCLUDE// -include/ --include}
 if [ "$C" = "1" -o "$C" = "2" ]; then
 ONLINE=1
 
-# Take only the last argument, which is the C file to test
-shift $(( $# - 1 ))
-OPTIONS="$COCCIINCLUDE $1"
+# If the rule coccicheck is specified when calling make, number of
+# arguments is zero
+if [ $# -ne 0 ]; then
+   # Take only the last argument, which is the C file to test
+   shift $(( $# -1 ))
+   OPTIONS="$COCCIINCLUDE $1"
+else
+   if [ "$KBUILD_EXTMOD" = "" ] ; then
+   OPTIONS="--dir $srctree $COCCIINCLUDE"
+   else
+   OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
+   fi
+fi
 
 # No need to parallelize Coccinelle since this mode takes one input file.
 NPROC=1
-- 
2.25.1

[PATCH] bus: arm: Add of_node_put() before break statement

[Sumera Priyadarsini]

Every iteration of for_each_available_child_of_node() decrements the
reference count of the previous node, however when control is
transferred from the middle of the loop, as in the case of a
return or break or goto, there is no decrement thus ultimately
resulting in a memory leak.

Fix a potential memory leak in arm-cci.c by inserting
of_node_put() before a break statement.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/bus/arm-cci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index b8184a903583..1f84a5528073 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -461,8 +461,10 @@ static int cci_probe_ports(struct device_node *np)
 
i = nb_ace + nb_ace_lite;
 
-   if (i >= nb_cci_ports)
+   if (i >= nb_cci_ports) {
+   of_node_put(cp);
break;
+   }
 
if (of_property_read_string(cp, "interface-type",
&match_str)) {
-- 
2.17.1

[PATCH] clk: versatile: Add of_node_put() before return statement

[Sumera Priyadarsini]

Every iteration of for_each_available_child_of_node() decrements
the reference count of the previous node, however when control is
transferred from the middle of the loop, as in the case of a return
or break or goto, there is no decrement thus ultimately resulting in
a memory leak.

Fix a potential memory leak in clk-impd1.c by inserting
of_node_put() before a return statement.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/clk/versatile/clk-impd1.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/versatile/clk-impd1.c 
b/drivers/clk/versatile/clk-impd1.c
index ca798249544d..85c395df9c00 100644
--- a/drivers/clk/versatile/clk-impd1.c
+++ b/drivers/clk/versatile/clk-impd1.c
@@ -109,8 +109,10 @@ static int integrator_impd1_clk_probe(struct 
platform_device *pdev)
 
for_each_available_child_of_node(np, child) {
ret = integrator_impd1_clk_spawn(dev, np, child);
-   if (ret)
+   if (ret) {
+   of_node_put(child);
break;
+   }
}
 
return ret;
-- 
2.17.1

[PATCH] bus: qcom-ebi2: Add of_node_put() before return statement

[Sumera Priyadarsini]

Every iteration of for_each_available_child_of_node() decrements
the reference count of the previous node, however when control is
transferred from the middle of the loop, as in the case of a return
or break or goto, there is no decrement thus ultimately resulting in
a memory leak.

Fix a potential memory leak in qcom-ebi2.c by inserting
of_node_put() before a return statement.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/bus/qcom-ebi2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/qcom-ebi2.c b/drivers/bus/qcom-ebi2.c
index 03ddcf426887..0b8f53a688b8 100644
--- a/drivers/bus/qcom-ebi2.c
+++ b/drivers/bus/qcom-ebi2.c
@@ -353,8 +353,10 @@ static int qcom_ebi2_probe(struct platform_device *pdev)
 
/* Figure out the chipselect */
ret = of_property_read_u32(child, "reg", &csindex);
-   if (ret)
+   if (ret) {
+   of_node_put(child);
return ret;
+   }
 
if (csindex > 5) {
dev_err(dev,
-- 
2.17.1

[PATCH] bus: arm-integrator-lm: Add of_node_put() before return statement

[Sumera Priyadarsini]

Every iteration of for_each_available_child_of_node() decrements
the reference count of the previous node, however when control is
transferred from the middle of the loop, as in the case of a return
or break or goto, there is no decrement thus ultimately resulting in
a memory leak.

Fix a potential memory leak in arm-integrator-lm.c by inserting
of_node_put() before a return statement.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/bus/arm-integrator-lm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bus/arm-integrator-lm.c b/drivers/bus/arm-integrator-lm.c
index 845b6c43fef8..2344d560b144 100644
--- a/drivers/bus/arm-integrator-lm.c
+++ b/drivers/bus/arm-integrator-lm.c
@@ -54,6 +54,7 @@ static int integrator_lm_populate(int num, struct device *dev)
ret = of_platform_default_populate(child, NULL, dev);
if (ret) {
dev_err(dev, "failed to populate module\n");
+   of_node_put(child);
return ret;
}
}
-- 
2.17.1

[PATCH V4] net: dsa: mt7530: Add of_node_put() before break and return statements

[Sumera Priyadarsini]

Every iteration of for_each_child_of_node() decrements
the reference count of the previous node, however when control
is transferred from the middle of the loop, as in the case of
a return or break or goto, there is no decrement thus ultimately
resulting in a memory leak.

Fix a potential memory leak in mt7530.c by inserting of_node_put()
before the break and return statements.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
Changes in v2:
Add another of_node_put() in for_each_child_of_node() as
pointed out by Andrew.

Changes in v3:
- Correct syntax errors
- Modify commit message

Changes in v4:
- Change commit prefix to include the driver name,
mt7530, as pointed out by Vladimir.
- Change the signoff to the correct format.

---
 drivers/net/dsa/mt7530.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 8dcb8a49ab67..4b4701c69fe1 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1326,14 +1326,17 @@ mt7530_setup(struct dsa_switch *ds)
 
if (phy_node->parent == priv->dev->of_node->parent) {
ret = of_get_phy_mode(mac_np, &interface);
-   if (ret && ret != -ENODEV)
+   if (ret && ret != -ENODEV) {
+   of_node_put(mac_np);
return ret;
+   }
id = of_mdio_parse_addr(ds->dev, phy_node);
if (id == 0)
priv->p5_intf_sel = P5_INTF_SEL_PHY_P0;
if (id == 4)
priv->p5_intf_sel = P5_INTF_SEL_PHY_P4;
}
+   of_node_put(mac_np);
of_node_put(phy_node);
break;
}
-- 
2.17.1

[PATCH V3] net: dsa: Add of_node_put() before break and return statements

[Sumera Priyadarsini]

Every iteration of for_each_child_of_node() decrements
the reference count of the previous node, however when control
is transferred from the middle of the loop, as in the case of
a return or break or goto, there is no decrement thus ultimately
resulting in a memory leak.

Fix a potential memory leak in mt7530.c by inserting of_node_put()
before the break and return statements.

Issue found with Coccinelle.

---
Changes in v2:
Add another of_node_put() in for_each_child_of_node() as pointed
out by Andrew.

Changes in v3:
- Correct syntax errors
- Modify commit message

---

Signed-off-by: Sumera Priyadarsini 

Signed-off-by: Sumera Priyadarsini 
---
 drivers/net/dsa/mt7530.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 8dcb8a49ab67..4b4701c69fe1 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1326,14 +1326,17 @@ mt7530_setup(struct dsa_switch *ds)
 
if (phy_node->parent == priv->dev->of_node->parent) {
ret = of_get_phy_mode(mac_np, &interface);
-   if (ret && ret != -ENODEV)
+   if (ret && ret != -ENODEV) {
+   of_node_put(mac_np);
return ret;
+   }
id = of_mdio_parse_addr(ds->dev, phy_node);
if (id == 0)
priv->p5_intf_sel = P5_INTF_SEL_PHY_P0;
if (id == 4)
priv->p5_intf_sel = P5_INTF_SEL_PHY_P4;
}
+   of_node_put(mac_np);
of_node_put(phy_node);
break;
}
-- 
2.17.1

[PATCH V2] net: dsa: Add of_node_put() before break statement

[Sumera Priyadarsini]

Every iteration of for_each_child_of_node() decrements
the reference count of the previous node, however when control
is transferred from the middle of the loop, as in the case of
a return or break or goto, there is no decrement thus ultimately
resulting in a memory leak.

Fix a potential memory leak in mt7530.c by inserting of_node_put()
before the break statement.

Issue found with Coccinelle.

---
Changes in v2:
Add another of_node_put() in for_each_child_of_node() as pointed
out by Andrew.

---

Signed-off-by: Sumera Priyadarsini 
---
 drivers/net/dsa/mt7530.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 8dcb8a49ab67..e81198a65c26 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1327,6 +1327,7 @@ mt7530_setup(struct dsa_switch *ds)
if (phy_node->parent == priv->dev->of_node->parent) {
ret = of_get_phy_mode(mac_np, &interface);
if (ret && ret != -ENODEV)
+   of_node_put(mac_np)
return ret;
id = of_mdio_parse_addr(ds->dev, phy_node);
if (id == 0)
@@ -1334,6 +1335,7 @@ mt7530_setup(struct dsa_switch *ds)
if (id == 4)
priv->p5_intf_sel = P5_INTF_SEL_PHY_P4;
}
+   of_node_put(mac_np);
of_node_put(phy_node);
break;
}
-- 
2.17.1

[PATCH] net: dsa: Add of_node_put() before break statement

[Sumera Priyadarsini]

Every iteration of for_each_child_of_node() decrements
the reference count of the previous node, however when control
is transferred from the middle of the loop, as in the case of
a return or break or goto, there is no decrement thus ultimately
resulting in a memory leak.

Fix a potential memory leak in mt7530.c by inserting of_node_put()
before the break statement.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/net/dsa/mt7530.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 8dcb8a49ab67..af83e5034842 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1334,6 +1334,7 @@ mt7530_setup(struct dsa_switch *ds)
if (id == 4)
priv->p5_intf_sel = P5_INTF_SEL_PHY_P4;
}
+   of_node_put(mac_np);
of_node_put(phy_node);
break;
}
-- 
2.17.1

[PATCH] net: ocelot: Add of_node_put() before return statement

[Sumera Priyadarsini]

Every iteration of for_each_available_child_of_node() decrements
the reference count of the previous node, however when control
is transferred from the middle of the loop, as in the case of
a return or break or goto, there is no decrement thus ultimately
resulting in a memory leak.

Fix a potential memory leak in felix.c by inserting of_node_put()
before the return statement.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/net/dsa/ocelot/felix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index c69d9592a2b7..04bfa6e465ff 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -400,6 +400,7 @@ static int felix_parse_ports_node(struct felix *felix,
if (err < 0) {
dev_err(dev, "Unsupported PHY mode %s on port %d\n",
phy_modes(phy_mode), port);
+   of_node_put(child);
return err;
}
 
-- 
2.17.1

[PATCH V3] scripts: coccicheck: Change default value for parallelism

[Sumera Priyadarsini]

By default, coccicheck utilizes all available threads to implement
parallelisation. However, when all available threads are used,
a decrease in performance is noted. The elapsed time is  minimum
when at most one thread per core is used.

For example, on benchmarking the semantic patch kfree.cocci for
usb/serial using hyperfine, the outputs obtained for J=5 and J=2
are 1.32 and 1.90 times faster than those for J=10 and J=9
respectively for two separate runs. For the larger drivers/staging
directory, minimium elapsed time is obtained for J=3 which is 1.86
times faster than that for J=12. The optimal J value does not
exceed 6 in any of the test runs. The benchmarks are run on a machine
with 6 cores, with 2 threads per core, i.e, 12 hyperthreads in all.

To improve performance, modify coccicheck to use at most only
one thread per core by default.

Signed-off-by: Sumera Priyadarsini 

---
Changes in V2:
- Change commit message as suggested by Julia Lawall
Changes in V3:
- Use J/2 as optimal value for machines with more
than 8 hyperthreads as well.
---
 scripts/coccicheck | 5 +
 1 file changed, 5 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..a72aa6c037ff 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -75,8 +75,13 @@ else
 OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
 fi
 
+# Use only one thread per core by default if hyperthreading is enabled
+THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd [:digit:])
 if [ -z "$J" ]; then
 NPROC=$(getconf _NPROCESSORS_ONLN)
+   if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 2 ] ; then
+   NPROC=$((NPROC/2))
+   fi
 else
 NPROC="$J"
 fi
-- 
2.17.1

[PATCH] net: gianfar: Add of_node_put() before goto statement

[Sumera Priyadarsini]

Every iteration of for_each_available_child_of_node() decrements
reference count of the previous node, however when control
is transferred from the middle of the loop, as in the case of
a return or break or goto, there is no decrement thus ultimately
resulting in a memory leak.

Fix a potential memory leak in gianfar.c by inserting of_node_put()
before the goto statement.

Issue found with Coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 drivers/net/ethernet/freescale/gianfar.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c 
b/drivers/net/ethernet/freescale/gianfar.c
index b513b8c5c3b5..41dd3d0f3452 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -750,8 +750,10 @@ static int gfar_of_init(struct platform_device *ofdev, 
struct net_device **pdev)
continue;
 
err = gfar_parse_group(child, priv, model);
-   if (err)
+   if (err) {
+   of_node_put(child);
goto err_grp_init;
+   }
}
} else { /* SQ_SG_MODE */
err = gfar_parse_group(np, priv, model);
-- 
2.17.1

[PATCH V2] scripts: coccicheck: Change default value for parallelism

[Sumera Priyadarsini]

By default, coccicheck utilizes all available threads to implement
parallelisation. However, when all available threads are used,
a decrease in performance is noted. The elapsed time is  minimum
when at most one thread per core is used.

For example, on benchmarking the semantic patch kfree.cocci for
usb/serial using hyperfine, the outputs obtained for J=5 and J=2
are 1.32 and 1.90 times faster than those for J=10 and J=9
respectively for two separate runs. For the larger drivers/staging
directory, minimium elapsed time is obtained for J=3 which is 1.86
times faster than that for J=12. The optimal J value does not
exceed 6 in any of the test runs. The benchmarks are run on a machine
with 6 cores, with 2 threads per core, i.e, 12 hyperthreads in all.

To improve performance, modify coccicheck to use at most only
one thread per core by default.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 9 +
 1 file changed, 9 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..dd228dcc915e 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -75,8 +75,17 @@ else
 OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
 fi
 
+# Use only one thread per core by default if hyperthreading is enabled
+THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd [:digit:])
 if [ -z "$J" ]; then
 NPROC=$(getconf _NPROCESSORS_ONLN)
+   if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 2 ] ; then
+   if [ $NPROC -gt 8 ] ; then
+   NPROC=$((NPROC/4))
+   else
+   NPROC=$((NPROC/2))
+   fi
+   fi
 else
 NPROC="$J"
 fi
-- 
2.17.1

[PATCH] scripts: coccicheck: Change default value for parallelism

[Sumera Priyadarsini]

By default, coccicheck utilizes all available threads to implement
parallelisation. However, when hyperthreading is enabled, this leads
to all threads per core being occupied resulting in longer wall-clock
times and higher power consumption. Hence, to improve performance,
modify coccicheck to use only one thread per core atmost.

In the cases where the total number of threads is more than 8 and
hyperthreading is enabled, it was observed that optimum performance
is achieved around one-fourth of the total number of cores.
Modify the script further to accommodate this use case.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 9 +
 1 file changed, 9 insertions(+)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..dd228dcc915e 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -75,8 +75,17 @@ else
 OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
 fi
 
+# Use only one thread per core by default if hyperthreading is enabled
+THREADS_PER_CORE=$(lscpu | grep "Thread(s) per core: " | tr -cd [:digit:])
 if [ -z "$J" ]; then
 NPROC=$(getconf _NPROCESSORS_ONLN)
+   if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 2 ] ; then
+   if [ $NPROC -gt 8 ] ; then
+   NPROC=$((NPROC/4))
+   else
+   NPROC=$((NPROC/2))
+   fi
+   fi
 else
 NPROC="$J"
 fi
-- 
2.17.1

[PATCH v4] documentation: coccinelle: Improve command example for make C={1,2}

[Sumera Priyadarsini]

Modify coccinelle documentation to further clarify
the usage of the makefile C variable by coccicheck.

Signed-off-by: Sumera Priyadarsini 

---
Changes in v4:
- Modify commit message to clarify C is a variable
---
 Documentation/dev-tools/coccinelle.rst | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/dev-tools/coccinelle.rst 
b/Documentation/dev-tools/coccinelle.rst
index 6c791af1c859..74c5e6aeeff5 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -175,13 +175,20 @@ For example, to check drivers/net/wireless/ one may 
write::
 make coccicheck M=drivers/net/wireless/
 
 To apply Coccinelle on a file basis, instead of a directory basis, the
-following command may be used::
+C variable is used by the makefile to select which files to work with.
+This variable can be used to run scripts for the entire kernel, a
+specific directory, or for a single file.
 
-make C=1 CHECK="scripts/coccicheck"
+For example, to check drivers/bluetooth/bfusb.c, the value 1 is
+passed to the C variable to check files that make considers
+need to be compiled.::
 
-To check only newly edited code, use the value 2 for the C flag, i.e.::
+make C=1 CHECK=scripts/coccicheck drivers/bluetooth/bfusb.o
 
-make C=2 CHECK="scripts/coccicheck"
+The value 2 is passed to the C variable to check files regardless of
+whether they need to be compiled or not.::
+
+make C=2 CHECK=scripts/coccicheck drivers/bluetooth/bfusb.o
 
 In these modes, which work on a file basis, there is no information
 about semantic patches displayed, and no commit message proposed.
-- 
2.17.1

[PATCH v3] documentation: coccinelle: Improve command example for make C={1,2}

[Sumera Priyadarsini]

Modify coccinelle documentation to further clarify
the usage of the makefile C variable flag by coccicheck.

Signed-off-by: Sumera Priyadarsini 

---
Changes in v3:
- Remove quotes as suggested by Markus Elfring
- Change in wording, and punctuation, as suggested by Julia Lawall
---
 Documentation/dev-tools/coccinelle.rst | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/dev-tools/coccinelle.rst 
b/Documentation/dev-tools/coccinelle.rst
index 6c791af1c859..74c5e6aeeff5 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -175,13 +175,20 @@ For example, to check drivers/net/wireless/ one may 
write::
 make coccicheck M=drivers/net/wireless/
 
 To apply Coccinelle on a file basis, instead of a directory basis, the
-following command may be used::
+C variable is used by the makefile to select which files to work with.
+This variable can be used to run scripts for the entire kernel, a
+specific directory, or for a single file.
 
-make C=1 CHECK="scripts/coccicheck"
+For example, to check drivers/bluetooth/bfusb.c, the value 1 is
+passed to the C variable to check files that make considers
+need to be compiled.::
 
-To check only newly edited code, use the value 2 for the C flag, i.e.::
+make C=1 CHECK=scripts/coccicheck drivers/bluetooth/bfusb.o
 
-make C=2 CHECK="scripts/coccicheck"
+The value 2 is passed to the C variable to check files regardless of
+whether they need to be compiled or not.::
+
+make C=2 CHECK=scripts/coccicheck drivers/bluetooth/bfusb.o
 
 In these modes, which work on a file basis, there is no information
 about semantic patches displayed, and no commit message proposed.
-- 
2.17.1

[PATCH v2] scripts: coccicheck: Add chain mode to list of modes

[Sumera Priyadarsini]

This patch adds chain mode to the list of available modes in coccicheck.

Signed-off-by: Sumera Priyadarsini 
---
Changes in v2:
- Change coccinelle to coccicheck as suggested by Julia Lawall.
---
 scripts/coccicheck | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..6e37cf36caae 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -99,7 +99,7 @@ fi
 if [ "$MODE" = "" ] ; then
 if [ "$ONLINE" = "0" ] ; then
echo 'You have not explicitly specified the mode to use. Using default 
"report" mode.'
-   echo 'Available modes are the following: patch, report, context, org'
+   echo 'Available modes are the following: patch, report, context, org, 
chain'
echo 'You can specify the mode with "make coccicheck MODE="'
echo 'Note however that some modes are not implemented by some semantic 
patches.'
 fi
-- 
2.17.1

[PATCH] scripts: coccicheck: Add chain mode to list of modes

[Sumera Priyadarsini]

This patch adds chain mode to the list of available modes in coccinelle.

Signed-off-by: Sumera Priyadarsini 
---
 scripts/coccicheck | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e04d328210ac..6e37cf36caae 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -99,7 +99,7 @@ fi
 if [ "$MODE" = "" ] ; then
 if [ "$ONLINE" = "0" ] ; then
echo 'You have not explicitly specified the mode to use. Using default 
"report" mode.'
-   echo 'Available modes are the following: patch, report, context, org'
+   echo 'Available modes are the following: patch, report, context, org, 
chain'
echo 'You can specify the mode with "make coccicheck MODE="'
echo 'Note however that some modes are not implemented by some semantic 
patches.'
 fi
-- 
2.17.1