from:"Julia Lawall"

Re: [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev

2023-07-12 Thread Julia Lawall



On Wed, 12 Jul 2023, Uwe Kleine-König wrote:

> On Wed, Jul 12, 2023 at 12:46:33PM +0200, Christian König wrote:
> > Am 12.07.23 um 11:46 schrieb Uwe Kleine-König:
> > > Hello,
> > >
> > > while I debugged an issue in the imx-lcdc driver I was constantly
> > > irritated about struct drm_device pointer variables being named "dev"
> > > because with that name I usually expect a struct device pointer.
> > >
> > > I think there is a big benefit when these are all renamed to "drm_dev".
> > > I have no strong preference here though, so "drmdev" or "drm" are fine
> > > for me, too. Let the bikesheding begin!
> > >
> > > Some statistics:
> > >
> > > $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort | uniq 
> > > -c | sort -n
> > >1 struct drm_device *adev_to_drm
> > >1 struct drm_device *drm_
> > >1 struct drm_device  *drm_dev
> > >1 struct drm_device*drm_dev
> > >1 struct drm_device *pdev
> > >1 struct drm_device *rdev
> > >1 struct drm_device *vdev
> > >2 struct drm_device *dcss_drv_dev_to_drm
> > >2 struct drm_device **ddev
> > >2 struct drm_device *drm_dev_alloc
> > >2 struct drm_device *mock
> > >2 struct drm_device *p_ddev
> > >5 struct drm_device *device
> > >9 struct drm_device * dev
> > >   25 struct drm_device *d
> > >   95 struct drm_device *
> > >  216 struct drm_device *ddev
> > >  234 struct drm_device *drm_dev
> > >  611 struct drm_device *drm
> > > 4190 struct drm_device *dev
> > >
> > > This series starts with renaming struct drm_crtc::dev to drm_dev. If
> > > it's not only me and others like the result of this effort it should be
> > > followed up by adapting the other structs and the individual usages in
> > > the different drivers.
> > >
> > > To make this series a bit easier handleable, I first added an alias for
> > > drm_crtc::dev, then converted the drivers one after another and the last
> > > patch drops the "dev" name. This has the advantage of being easier to
> > > review, and if I should have missed an instance only the last patch must
> > > be dropped/reverted. Also this series might conflict with other patches,
> > > in this case the remaining patches can still go in (apart from the last
> > > one of course). Maybe it also makes sense to delay applying the last
> > > patch by one development cycle?
> >
> > When you automatically generate the patch (with cocci for example) I usually
> > prefer a single patch instead.
>
> Maybe I'm too stupid, but only parts of this patch were created by
> coccinelle. I failed to convert code like
>
> -   spin_lock_irq(>dev->event_lock);
> +   spin_lock_irq(>drm_dev->event_lock);
>
> Added Julia to Cc, maybe she has a hint?!

A priori, I see no reason why the rule below should not apply to the above
code.  Is there a parsing problem in the containing function?  You can run

spatch --parse-c file.c

If there is a paring problem, please let me know and i will try to fix it
so the while thing can be done automatically.

julia

>
> (Up to now it's only
>
> @@
> struct drm_crtc *crtc;
> @@
> -crtc->dev
> +crtc->drm_dev
>
> )
>
> > Background is that this makes merge conflicts easier to handle and detect.
>
> Really? Each file (apart from include/drm/drm_crtc.h) is only touched
> once. So unless I'm missing something you don't get less or easier
> conflicts by doing it all in a single patch. But you gain the freedom to
> drop a patch for one driver without having to drop the rest with it. So
> I still like the split version better, but I'm open to a more verbose
> reasoning from your side.
>
> > When you have multiple patches and a merge conflict because of some added
> > lines using the old field the build breaks only on the last patch which
> > removes the old field.
>
> Then you can revert/drop the last patch without having to respin the
> whole single patch and thus caring for still more conflicts that arise
> until the new version is sent.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K.   | Uwe Kleine-König|
> Industrial Linux Solutions | https://www.pengutronix.de/ |
>

[PATCH] drm/amdkfd: fix typo in comment

2022-05-21 Thread Julia Lawall

Spelling mistake (triple letters) in comment.
Detected with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 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 8b5452a8d330..67abf8dcd30a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1621,7 +1621,7 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 
mutex_lock(>lock);
 
-   /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
+   /* Unpin MMIO/DOORBELL BO's that were pinned during allocation */
if (mem->alloc_flags &
(KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
 KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {

[PATCH] drm/amdgpu/gfx: fix typos in comments

2022-05-21 Thread Julia Lawall

Spelling mistakes (triple letters) in comments.
Detected with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c |2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  |4 ++--
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  |2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 02754ee86c81..c5f46d264b23 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -5111,7 +5111,7 @@ static void gfx_v10_0_init_compute_vmid(struct 
amdgpu_device *adev)
mutex_unlock(>srbm_mutex);
 
/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-  acccess. These should be enabled by FW for target VMIDs. */
+  access. These should be enabled by FW for target VMIDs. */
for (i = adev->vm_manager.first_kfd_vmid; i < AMDGPU_NUM_VMID; i++) {
WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index fb9302910742..7f0b18b0d4c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -3714,7 +3714,7 @@ static void gfx_v8_0_init_compute_vmid(struct 
amdgpu_device *adev)
mutex_unlock(>srbm_mutex);
 
/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-  acccess. These should be enabled by FW for target VMIDs. */
+  access. These should be enabled by FW for target VMIDs. */
for (i = adev->vm_manager.first_kfd_vmid; i < AMDGPU_NUM_VMID; i++) {
WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
@@ -5815,7 +5815,7 @@ static void 
gfx_v8_0_update_coarse_grain_clock_gating(struct amdgpu_device *adev
/* wait for RLC_SERDES_CU_MASTER & RLC_SERDES_NONCU_MASTER idle 
*/
gfx_v8_0_wait_for_rlc_serdes(adev);
 
-   /* write cmd to Set CGCG Overrride */
+   /* write cmd to Set CGCG Override */
gfx_v8_0_send_serdes_cmd(adev, BPM_REG_CGCG_OVERRIDE, 
SET_BPM_SERDES_CMD);
 
/* wait for RLC_SERDES_CU_MASTER & RLC_SERDES_NONCU_MASTER idle 
*/
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index f12ae6e2359a..5349ca4d19e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -2535,7 +2535,7 @@ static void gfx_v9_0_init_compute_vmid(struct 
amdgpu_device *adev)
mutex_unlock(>srbm_mutex);
 
/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-  acccess. These should be enabled by FW for target VMIDs. */
+  access. These should be enabled by FW for target VMIDs. */
for (i = adev->vm_manager.first_kfd_vmid; i < AMDGPU_NUM_VMID; i++) {
WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);

[PATCH] drm/amdgpu/powerplay/vega10: fix minmax.cocci warnings

2022-04-16 Thread Julia Lawall

From: kernel test robot 

Use max to simplify the code.

Generated by: scripts/coccinelle/misc/minmax.cocci

CC: Denis Efremov 
Reported-by: kernel test robot 
Signed-off-by: kernel test robot 
Signed-off-by: Julia Lawall 

---

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   028192fea1de083f4f12bfb1eb7c4d7beb5c8ecd
commit: 5f66f73b9ff4dcabd4e2405ba9c32e80e02f9408 coccinelle: misc: add minmax 
script
:: branch date: 17 hours ago
:: commit date: 12 months ago

Please take the patch only if it's a positive warning. Thanks!

 drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c |   10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -345,12 +345,10 @@ static int vega10_odn_initial_default_se
odn_table->min_vddc = dep_table[0]->entries[0].vddc;

i = od_table[2]->count - 1;
-   od_table[2]->entries[i].clk = 
hwmgr->platform_descriptor.overdriveLimit.memoryClock > 
od_table[2]->entries[i].clk ?
-   
hwmgr->platform_descriptor.overdriveLimit.memoryClock :
-   od_table[2]->entries[i].clk;
-   od_table[2]->entries[i].vddc = odn_table->max_vddc > 
od_table[2]->entries[i].vddc ?
-   odn_table->max_vddc :
-   od_table[2]->entries[i].vddc;
+   od_table[2]->entries[i].clk = 
max(hwmgr->platform_descriptor.overdriveLimit.memoryClock,
+ od_table[2]->entries[i].clk);
+   od_table[2]->entries[i].vddc = max(odn_table->max_vddc,
+  od_table[2]->entries[i].vddc);

return 0;
 }

[PATCH 23/30] drm/amdgpu/dc: fix typos in comments

2022-03-14 Thread Julia Lawall

Various spelling mistakes in comments.
Detected with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/gpu/drm/amd/display/dc/bios/command_table.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c 
b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
index ad13e4e36d77..0e36cd800fc9 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
@@ -456,7 +456,7 @@ static enum bp_result transmitter_control_v2(
if ((CONNECTOR_ID_DUAL_LINK_DVII == connector_id) ||
(CONNECTOR_ID_DUAL_LINK_DVID == connector_id))
/* on INIT this bit should be set according to the
-* phisycal connector
+* physical connector
 * Bit0: dual link connector flag
 * =0 connector is single link connector
 * =1 connector is dual link connector
@@ -468,7 +468,7 @@ static enum bp_result transmitter_control_v2(
cpu_to_le16((uint8_t)cntl->connector_obj_id.id);
break;
case TRANSMITTER_CONTROL_SET_VOLTAGE_AND_PREEMPASIS:
-   /* votage swing and pre-emphsis */
+   /* voltage swing and pre-emphsis */
params.asMode.ucLaneSel = (uint8_t)cntl->lane_select;
params.asMode.ucLaneSet = (uint8_t)cntl->lane_settings;
break;
@@ -2120,7 +2120,7 @@ static enum bp_result program_clock_v5(
memset(, 0, sizeof(params));
if (!bp->cmd_helper->clock_source_id_to_atom(
bp_params->pll_id, _pll_id)) {
-   BREAK_TO_DEBUGGER(); /* Invalid Inpute!! */
+   BREAK_TO_DEBUGGER(); /* Invalid Input!! */
return BP_RESULT_BADINPUT;
}

[PATCH 01/30] drm/amd/pm: fix typos in comments

2022-03-14 Thread Julia Lawall

Various spelling mistakes in comments.
Detected with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c 
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index cbbbd4079249..5cd67ddf8495 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -1870,7 +1870,7 @@ static ssize_t amdgpu_set_smartshift_bias(struct device 
*dev,
amdgpu_smartshift_bias = bias;
r = count;
 
-   /* TODO: upadte bias level with SMU message */
+   /* TODO: update bias level with SMU message */
 
 out:
pm_runtime_mark_last_busy(ddev->dev);

[PATCH 00/30] fix typos in comments

2022-03-14 Thread Julia Lawall

Various spelling mistakes in comments.
Detected with the help of Coccinelle.

---

 drivers/base/devres.c   |4 ++--
 drivers/clk/qcom/gcc-sm6125.c   |2 +-
 drivers/clk/ti/clkctrl.c|2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |4 ++--
 drivers/gpu/drm/amd/display/dc/bios/command_table.c |6 +++---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c  |2 +-
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c  |4 ++--
 drivers/gpu/drm/sti/sti_gdp.c   |2 +-
 drivers/infiniband/hw/qib/qib_iba7220.c |4 ++--
 drivers/leds/leds-pca963x.c |2 +-
 drivers/media/i2c/ov5695.c  |2 +-
 drivers/mfd/rohm-bd9576.c   |2 +-
 drivers/mtd/ubi/block.c |2 +-
 drivers/net/can/usb/ucan.c  |4 ++--
 drivers/net/ethernet/packetengines/yellowfin.c  |2 +-
 drivers/net/wireless/ath/ath6kl/htc_mbox.c  |2 +-
 drivers/net/wireless/cisco/airo.c   |2 +-
 drivers/net/wireless/mediatek/mt76/mt7915/init.c|2 +-
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c |6 +++---
 drivers/platform/x86/uv_sysfs.c |2 +-
 drivers/s390/crypto/pkey_api.c  |2 +-
 drivers/scsi/aic7xxx/aicasm/aicasm.c|2 +-
 drivers/scsi/elx/libefc_sli/sli4.c  |2 +-
 drivers/scsi/lpfc/lpfc_mbox.c   |2 +-
 drivers/scsi/qla2xxx/qla_gs.c   |2 +-
 drivers/spi/spi-sun4i.c |2 +-
 drivers/staging/rtl8723bs/core/rtw_mlme.c   |2 +-
 drivers/usb/gadget/udc/snps_udc_core.c  |2 +-
 fs/kernfs/file.c|2 +-
 kernel/events/core.c|2 +-
 30 files changed, 39 insertions(+), 39 deletions(-)

[PATCH 29/30] drm/amdgpu: fix typos in comments

2022-03-14 Thread Julia Lawall

Various spelling mistakes in comments.
Detected with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index fe660a8e150f..970b065e9a6b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -340,7 +340,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct 
amdgpu_device *adev,
if (free_vram >= 128 * 1024 * 1024 || free_vram >= total_vram / 8) {
s64 min_us;
 
-   /* Be more aggresive on dGPUs. Try to fill a portion of free
+   /* Be more aggressive on dGPUs. Try to fill a portion of free
 * VRAM now.
 */
if (!(adev->flags & AMD_IS_APU))
@@ -1280,7 +1280,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
continue;
 
/*
-* Work around dma_resv shortcommings by wrapping up the
+* Work around dma_resv shortcomings by wrapping up the
 * submission in a dma_fence_chain and add it as exclusive
 * fence.
 */

[PATCH] drm/amdgpu: fix semicolon.cocci warnings

2021-04-03 Thread Julia Lawall

From: kernel test robot 

Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: 37439a51ff17 ("drm/amdgpu: Add mode2 reset support for aldebaran")
CC: Lijo Lazar 
Reported-by: kernel test robot 
Signed-off-by: kernel test robot 
Signed-off-by: Julia Lawall 
---

tree:   https://gitlab.freedesktop.org/agd5f/linux.git drm-next-5.13
head:   ef95d2a98d642a537190d73c45ae3c308afee890
commit: 37439a51ff171f938f886d6078802926fb27ccf8 [100/149] drm/amdgpu: Add 
mode2 reset support for aldebaran
:: branch date: 14 hours ago
:: commit date: 4 days ago

 aldebaran.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
+++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
@@ -227,7 +227,7 @@ static int aldebaran_mode2_restore_ip(st
break;
default:
break;
-   };
+   }
}

/* Reinit NBIF block */
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-11-01 Thread Julia Lawall




On Sat, 31 Oct 2020, Joe Perches wrote:

> On Fri, 2020-10-30 at 09:03 +0100, Greg KH wrote:
> > On Fri, Oct 30, 2020 at 01:27:16PM +0530, Deepak R Varma wrote:
> > > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
> > > > > function in place of the debugfs_create_file() function will make the
> > > > > file operation struct "reset" aware of the file's lifetime. Additional
> > > > > details here: https://lists.archive.carbon60.com/linux/kernel/2369498
> > > > >
> > > > > Issue reported by Coccinelle script:
> > > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> []
> > There is a reason we didn't just do a global search/replace for this in
> > the kernel when the new functions were added, so I don't know why
> > checkpatch is now saying it must be done.
>
> I think it's not a checkpatch warning here.

That is correct, it's a coccinelle script.

julia

>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/0b818156537f354904938f437cbb9dd02e765653.camel%40perches.com.
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Julia Lawall




On Fri, 30 Oct 2020, Greg KH wrote:

> On Fri, Oct 30, 2020 at 01:47:05PM +0530, Sumera Priyadarsini wrote:
> > On Fri, 30 Oct, 2020, 1:32 PM Greg KH,  wrote:
> >
> > > On Fri, Oct 30, 2020 at 01:27:16PM +0530, Deepak R Varma wrote:
> > > > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with
> > > debugfs_create_file_unsafe()
> > > > > > function in place of the debugfs_create_file() function will make 
> > > > > > the
> > > > > > file operation struct "reset" aware of the file's lifetime.
> > > Additional
> > > > > > details here:
> > > https://lists.archive.carbon60.com/linux/kernel/2369498
> > > > > >
> > > > > > Issue reported by Coccinelle script:
> > > > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > > > >
> > > > > > Signed-off-by: Deepak R Varma 
> > > > > > ---
> > > > > > Please Note: This is a Outreachy project task patch.
> > > > > >
> > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20
> > > ++--
> > > > > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > index 2d125b8b15ee..f076b1ba7319 100644
> > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void
> > > *data, u64 val)
> > > > > >   return 0;
> > > > > >  }
> > > > > >
> > > > > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > > - amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > > +  amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > >
> > > > > Are you sure this is ok?  Do these devices need this additional
> > > > > "protection"?  Do they have the problem that these macros were written
> > > > > for?
> > > > >
> > > > > Same for the other patches you just submitted here, I think you need 
> > > > > to
> > > > > somehow "prove" that these changes are necessary, checkpatch isn't 
> > > > > able
> > > > > to determine this all the time.
> > > >
> > > > Hi Greg,
> > > > Based on my understanding, the current function debugfs_create_file()
> > > > adds an overhead of lifetime managing proxy for such fop structs. This
> > > > should be applicable to these set of drivers as well. Hence I think this
> > > > change will be useful.
> > >
> > > Why do these drivers need these changes?  Are these files dynamically
> > > removed from the system at random times?
> > >
> > > There is a reason we didn't just do a global search/replace for this in
> > > the kernel when the new functions were added, so I don't know why
> > > checkpatch is now saying it must be done.
> > >
> >
> > Hi,
> >
> > Sorry to jump in on the thread this way, but what exactly does a 'lifetime
> > managing proxy' for file operations mean? I am trying to understand how
> > DEFINE_DEBUGFS_ATTRIBUTE changes things wrt debug_ fs file operations but
> > can't find many resources. :(
>
> It means that the debugfs core can handle debugfs files being removed
> from the system while they are still open when they were created by a
> driver/module that is now unloaded from memory.
>
> This is only an issue for drivers that manage devices that have unknown
> lifespans (i.e. they can be yanked out of the system at any time, and
> the memory for those debugfs files can be freed).
>
> For the entire DRM/GPU subsystem, I strongly doubt this is the case.
>
> > Please let me know if I should be asking this in a different mailing
> > list/irc instead.
> >
> > The change seems to be suggested by a coccinelle script.
>
> I know, and I don't think that script knows the nuances behind this, as,
> again, we would have just done a global search/replace for this when the
> debugfs fixes went into the kernel.

The script comes from Nicolai Stange .

If there are some precise considerations that make the change likely to be
useful, then the script can be changed.

If the script is not helpful, then it can be removed.

julia


>
> thanks,
>
> greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20201030082425.GA1619669%40kroah.com.
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Cocci] [RFC] treewide: cleanup unreachable breaks

2020-10-19 Thread Julia Lawall




On Sat, 17 Oct 2020, Joe Perches wrote:

> On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote:
> > From: Tom Rix 
> >
> > This is a upcoming change to clean up a new warning treewide.
> > I am wondering if the change could be one mega patch (see below) or
> > normal patch per file about 100 patches or somewhere half way by collecting
> > early acks.
> >
> > clang has a number of useful, new warnings see
> > https://clang.llvm.org/docs/DiagnosticsReference.html
> >
> > This change cleans up -Wunreachable-code-break
> > https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break
> > for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64.
>
> Early acks/individual patches by subsystem would be good.
> Better still would be an automated cocci script.

Coccinelle is not especially good at this, because it is based on control
flow, and a return or goto diverts the control flow away from the break.
A hack to solve the problem is to put an if around the return or goto, but
that gives the break a meaningless file name and line number.  I collected
the following list, but it only has 439 results, so fewer than clang.  But
maybe there are some files that are not considered by clang in the x86
allyesconfig configuration.

Probably checkpatch is the best solution here, since it is not
configuration sensitive and doesn't care about control flow.

julia

drivers/scsi/mvumi.c: function mvumi_cfg_hw_reg line 114
drivers/watchdog/geodewdt.c: function geodewdt_ioctl line 18
drivers/media/usb/b2c2/flexcop-usb.c: function flexcop_usb_init line 21
drivers/media/usb/b2c2/flexcop-usb.c: function flexcop_usb_memory_req line 20
drivers/tty/nozomi.c: function write_mem32 line 17
drivers/tty/nozomi.c: function write_mem32 line 25
drivers/tty/nozomi.c: function read_mem32 line 17
drivers/tty/nozomi.c: function read_mem32 line 21
sound/soc/codecs/wl1273.c: function wl1273_startup line 27
drivers/iio/adc/meson_saradc.c: function meson_sar_adc_iio_info_read_raw line 12
drivers/iio/adc/meson_saradc.c: function meson_sar_adc_iio_info_read_raw line 19
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function 
map_transmitter_id_to_phy_instance line 6
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function 
map_transmitter_id_to_phy_instance line 9
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function 
map_transmitter_id_to_phy_instance line 12
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function 
map_transmitter_id_to_phy_instance line 15
drivers/media/tuners/mt2063.c: function mt2063_init line 81
drivers/nfc/st21nfca/core.c: function st21nfca_hci_im_transceive line 46
arch/sh/boards/mach-landisk/gio.c: function gio_ioctl line 53
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 11
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 15
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 18
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 22
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 25
drivers/media/dvb-frontends/cx24117.c: function cx24117_attach line 16
drivers/block/xen-blkback/blkback.c: function dispatch_rw_block_io line 48
drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get 
line 16
drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get 
line 22
drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get 
line 31
drivers/char/mwave/mwavedd.c: function mwave_ioctl line 288
drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 15
drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 19
drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 22
drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 27
drivers/iio/imu/bmi160/bmi160_core.c: function bmi160_write_raw line 11
drivers/block/z2ram.c: function z2_open line 138
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c: function 
_rtl8723e_set_media_status line 38
samples/hidraw/hid-example.c: function bus_str line 6
samples/hidraw/hid-example.c: function bus_str line 9
samples/hidraw/hid-example.c: function bus_str line 12
samples/hidraw/hid-example.c: function bus_str line 15
samples/hidraw/hid-example.c: function bus_str line 18
drivers/scsi/ipr.c: function ipr_pci_error_detected line 10
drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 11
drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 17
drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 21
drivers/pinctrl/pinctrl-rockchip.c: function rockchip_pinconf_get line 71
drivers/pinctrl/pinctrl-rockchip.c: function rockchip_pinconf_set line 74
drivers/gpu/drm/amd/display/include/signal_types.h: function dc_is_dvi_signal 
line 6
security/keys/trusted-keys/trusted_tpm1.c: function datablob_parse line 63
arch/x86/math-emu/fpu_trig.c: function

Re: [PATCH v3] drm/amd: Fix memory leak according to error branch

2020-06-22 Thread Julia Lawall



On Sat, 20 Jun 2020, Markus Elfring wrote:

> > The function kobject_init_and_add alloc memory like:
> > kobject_init_and_add->kobject_add_varg->kobject_set_name_vargs
> > ->kvasprintf_const->kstrdup_const->kstrdup->kmalloc_track_caller
> > ->kmalloc_slab, in err branch this memory not free. If use
> > kmemleak, this path maybe catched.
> > These changes are to add kobject_put in kobject_init_and_add
> > failed branch, fix potential memleak.
> …
> > Changes since V2:
> > *remove duplicate kobject_put in kfd_procfs_init.
>
> Under which circumstances are going to improve this change description 
> accordingly?

Bernard, please update the log message as well.  The mail you sent was
much more clear, but mail just gets lost over time.  The log message
itself should be improved.

julia

>
> Would you like to add the tag “Fixes” to the commit message?
>
> Regards,
> Markus
>___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re:Re: [PATCH v2] drm/amdkfd: Fix memory leaks according to error branches

2020-06-22 Thread Julia Lawall



On Sat, 20 Jun 2020, Bernard wrote:

>
>
> From: Julia Lawall 
> Date: 2020-06-20 17:37:19
> To:  Markus Elfring 
> Cc:  Bernard Zhao 
> ,opensource.ker...@vivo.com,amd-gfx@lists.freedesktop.org,dri-de...@lists.freedesktop.org,kernel-janit...@vger.kernel.org,linux-ker...@vger.kernel.org,Alex
>  Deucher ,"Christian König" 
> ,"Felix Kühling" ,Daniel 
> Vetter ,David Airlie 
> Subject: Re: [PATCH v2] drm/amdkfd: Fix memory leaks according to error 
> branches>
> >
> >On Sat, 20 Jun 2020, Markus Elfring wrote:
> >
> >> > The function kobject_init_and_add alloc memory like:
> >> > kobject_init_and_add->kobject_add_varg->kobject_set_name_vargs
> >> > ->kvasprintf_const->kstrdup_const->kstrdup->kmalloc_track_caller
> >> > ->kmalloc_slab, in err branch this memory not free. If use
> >> > kmemleak, this path maybe catched.
> >> > These changes are to add kobject_put in kobject_init_and_add
> >> > failed branch, fix potential memleak.
> >>
> >> I suggest to improve this change description.
> >>
> >> * Can an other wording variant be nicer?
> >
> >Markus's suggestion is as usual extremely imprecise.  However, I also find
> >the message quite unclear.
> >
> >It would be good to always use English words.  alloc and err are not
> >English words.  Perhaps most people will figure out what they are
> >abbreviations for, but it would be better to use a few more letters to
> >make it so that no one has to guess.
> >
> >Then there are a bunch of things that are connected by arrows with no
> >spaces between them.  The most obvious meaning of an arrow with no space
> >around it is a variable dereference.  After spending some mental effort,
> >one can realize that that is not what you mean here.  A layout like:
> >
> >   first_function ->
> > second_function ->
> >   third_function
> >
> >would be much more readable.
> >
> >I don't know what "this patch maybe catched" means.  Is "catched" supposed
> >to be "caught" or "cached"?  Overall, the sentence could be "Kmemleak
> >could possibly detect this issue", or something like that.  But I don't
> >know what this means.  Did you detect the problem with kmemleak?  if you
> >did not detect the problem with kmemleak, and overall you don't know
> >whether kmemleak would detect the bug or not, is this information useful
> >at all for the patch?
>
> Hi:
>
> Kmemleak detected a memory leak as below:
> kobject_init_and_add->
>   kobject_add_varg->
>   kobject_set_name_vargs->
>   kvasprintf_const->
>   kstrdup_const->
>   kstrdup->
>   kmalloc_track_caller->
>   kmalloc_slab
>
> If kobject_init_and_add is called, but kobject_put is not called in the error 
> branch.
> This will be detected by kmemleak.

Thanks.  This is much more understandable.  The last part still seems a
bit hypothetical.  After the trace, which explain why you made the change,
just say what you did in the patch to fix the problem.

julia

>
> BR//Bernard
>
> >"These changes are to" makes a lot of words with no information.  While it
> >is perhaps not necessary to slavishly follow the rule about using the
> >imperative, if it is convenient to use the imperative, doing so eliminates
> >such meaningless phrases.
> >
> >memleak is also not an English word.  Memory leak is only a few more
> >characters, and doesn't require the reader to make the small extra effort
> >to figure out what you mean.
> >
> >julia
> >
> >>
> >> * Will the tag “Fixes” become helpful for the commit message?
> >>
> >> Regards,
> >> Markus
> >>
>
>
>___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2] drm/amdkfd: Fix memory leaks according to error branches

2020-06-22 Thread Julia Lawall

On Sat, 20 Jun 2020, Markus Elfring wrote:

> > The function kobject_init_and_add alloc memory like:
> > kobject_init_and_add->kobject_add_varg->kobject_set_name_vargs
> > ->kvasprintf_const->kstrdup_const->kstrdup->kmalloc_track_caller
> > ->kmalloc_slab, in err branch this memory not free. If use
> > kmemleak, this path maybe catched.
> > These changes are to add kobject_put in kobject_init_and_add
> > failed branch, fix potential memleak.
>
> I suggest to improve this change description.
>
> * Can an other wording variant be nicer?

Markus's suggestion is as usual extremely imprecise.  However, I also find
the message quite unclear.

It would be good to always use English words.  alloc and err are not
English words.  Perhaps most people will figure out what they are
abbreviations for, but it would be better to use a few more letters to
make it so that no one has to guess.

Then there are a bunch of things that are connected by arrows with no
spaces between them.  The most obvious meaning of an arrow with no space
around it is a variable dereference.  After spending some mental effort,
one can realize that that is not what you mean here.  A layout like:

   first_function ->
 second_function ->
   third_function

would be much more readable.

I don't know what "this patch maybe catched" means.  Is "catched" supposed
to be "caught" or "cached"?  Overall, the sentence could be "Kmemleak
could possibly detect this issue", or something like that.  But I don't
know what this means.  Did you detect the problem with kmemleak?  if you
did not detect the problem with kmemleak, and overall you don't know
whether kmemleak would detect the bug or not, is this information useful
at all for the patch?

"These changes are to" makes a lot of words with no information.  While it
is perhaps not necessary to slavishly follow the rule about using the
imperative, if it is convenient to use the imperative, doing so eliminates
such meaningless phrases.

memleak is also not an English word.  Memory leak is only a few more
characters, and doesn't require the reader to make the small extra effort
to figure out what you mean.

julia

>
> * Will the tag “Fixes” become helpful for the commit message?
>
> Regards,
> Markus
>___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'

2019-06-23 Thread Julia Lawall




On Sun, 23 Jun 2019, Dan Carpenter wrote:

> On Sat, Jun 22, 2019 at 01:43:19PM +0300, Dan Carpenter wrote:
> > On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote:
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c 
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > > index 0e6dba9..0bf4dd9 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device 
> > > *adev,
> > >  /* init amdgpu_pmu */
> > >  int amdgpu_pmu_init(struct amdgpu_device *adev)
> > >  {
> > > - int ret = 0;
> > > -
> > >   switch (adev->asic_type) {
> > >   case CHIP_VEGA20:
> > >   /* init df */
> > > - ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> > > + init_pmu_by_type(adev, df_v3_6_attr_groups,
> > >  "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> > >  DF_V3_6_MAX_COUNTERS);
> >
> >
> > You're resending this for other reasons, but don't forget to update the
> > indenting on the arguments so they still line up with the '('.
> >
>
> Sorry, I was unclear.  If you pull the init_pmu_by_type( back 6
> characters then you also need to pull the "DF" back 6 characters.
>
>   init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df",
>PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS);
>
> You can actually fit it into two lines afterwards.

My suggestion was to keep the ret = instead of discarding the indication
of failure, so I think that this is not relevant.

julia

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

2019-06-22 Thread Julia Lawall



On Sat, 22 Jun 2019, maowenan wrote:

>
>
> On 2019/6/22 21:06, Julia Lawall wrote:
> >
> >
> > On Sat, 22 Jun 2019, Mao Wenan wrote:
> >
> >> There is one warning:
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set 
> >> but not used [-Wunused-but-set-variable]
> >>   int ret = 0;
> >>   ^
> >> amdgpu_pmu_init() is called by amdgpu_device_init() in 
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
> >> which will use the return value. So it returns 'ret' for caller.
> >> amdgpu_device_init()
> >>r = amdgpu_pmu_init(adev);
> >>
> >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
> >>
> >> Signed-off-by: Mao Wenan 
> >> ---
> >>  v1->v2: change the subject for this patch; change the indenting when it 
> >> calls init_pmu_by_type; use the value 'ret' in
> >>  amdgpu_pmu_init().
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> index 0e6dba9..145e720 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
> >>case CHIP_VEGA20:
> >>/* init df */
> >>ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> >> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> >> - DF_V3_6_MAX_COUNTERS);
> >> + "DF", "amdgpu_df", 
> >> PERF_TYPE_AMDGPU_DF,
> >> + 
> >> DF_V3_6_MAX_COUNTERS);
> >>
> >>/* other pmu types go here*/
> >
> > I don't know what is the impact of the other pmu types that are planned
> > for the future.  Perhaps it would be better to abort the function
> > immediately in the case of a failure.
>
> I guess it would be better to use new function or new switch case clause to 
> process different PMU types
> in future.

I don't know.  But normally when an error may occur it is checked for
immediately, rather than just letting it go until the end of the function.
But maybe the developer know what is planned for the future for this
function.

julia

>
> >
> > julia
> >
> >>break;
> >> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
> >>return 0;
> >>}
> >>
> >> -  return 0;
> >> +  return ret;
> >>  }
> >>
> >>
> >> --
> >> 2.7.4
> >>
> >>
> >
>
>

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

2019-06-22 Thread Julia Lawall



On Sat, 22 Jun 2019, Mao Wenan wrote:

> There is one warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set 
> but not used [-Wunused-but-set-variable]
>   int ret = 0;
>   ^
> amdgpu_pmu_init() is called by amdgpu_device_init() in 
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
> which will use the return value. So it returns 'ret' for caller.
> amdgpu_device_init()
>   r = amdgpu_pmu_init(adev);
>
> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
>
> Signed-off-by: Mao Wenan 
> ---
>  v1->v2: change the subject for this patch; change the indenting when it 
> calls init_pmu_by_type; use the value 'ret' in
>  amdgpu_pmu_init().
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 0e6dba9..145e720 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>   case CHIP_VEGA20:
>   /* init df */
>   ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> -"DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> -DF_V3_6_MAX_COUNTERS);
> +"DF", "amdgpu_df", 
> PERF_TYPE_AMDGPU_DF,
> +
> DF_V3_6_MAX_COUNTERS);
>
>   /* other pmu types go here*/

I don't know what is the impact of the other pmu types that are planned
for the future.  Perhaps it would be better to abort the function
immediately in the case of a failure.

julia

>   break;
> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>   return 0;
>   }
>
> - return 0;
> + return ret;
>  }
>
>
> --
> 2.7.4
>
>

Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'

2019-06-22 Thread Julia Lawall



On Sat, 22 Jun 2019, Mao Wenan wrote:

> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set 
> but not used [-Wunused-but-set-variable]
>   int ret = 0;
>   ^
>
> It is never used since introduction in 9c7c85f7ea1f ("drm/amdgpu: add pmu 
> counters")
>
> Signed-off-by: Mao Wenan 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 0e6dba9..0bf4dd9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
>  /* init amdgpu_pmu */
>  int amdgpu_pmu_init(struct amdgpu_device *adev)
>  {
> - int ret = 0;
> -
>   switch (adev->asic_type) {
>   case CHIP_VEGA20:
>   /* init df */
> - ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> + init_pmu_by_type(adev, df_v3_6_attr_groups,
>  "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>  DF_V3_6_MAX_COUNTERS);

Maybe it would be better to use ret?

If knowing whether the call has failed is really useless, then maybe the
return type should be void?

julia


>
> --
> 2.7.4
>
>

Re: [PATCH] drm/amd/powerplay: Fix double unlock bug in smu_sys_set_pp_table()

2019-03-21 Thread Julia Lawall



On Thu, 21 Mar 2019, Dan Carpenter wrote:

> On Thu, Mar 21, 2019 at 09:20:38AM +0100, Julia Lawall wrote:
> >
> >
> > On Thu, 21 Mar 2019, Huang, Ray wrote:
> >
> > > > -Original Message-
> > > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> > > > Sent: Thursday, March 21, 2019 2:28 PM
> > > > To: Deucher, Alexander ; Wang, Kevin(Yang)
> > > > 
> > > > Cc: Koenig, Christian ; Zhou, David(ChunMing)
> > > > ; David Airlie ; Daniel Vetter
> > > > ; Huang, Ray ; Gao, Likun
> > > > ; Gui, Jack ; amd-
> > > > g...@lists.freedesktop.org; kernel-janit...@vger.kernel.org
> > > > Subject: [PATCH] drm/amd/powerplay: Fix double unlock bug in
> > > > smu_sys_set_pp_table()
> > > >
> > > > We already unlocked a few lines earlier so this code unlocks twice on 
> > > > the
> > > > success path.
> > > >
> > > > Fixes: 5809d7420f97 ("drm/amd/powerplay: implement sysfs of pp_table for
> > > > smu11 (v2)")
> > > > Signed-off-by: Dan Carpenter 
> > >
> > > Nice catch!  Thanks, Dan.
> > > Kevin, could you please verify this patch.
> > > Reviewed-by: Huang Rui 
> > >
> > > > ---
> > > > I'm not sure what this bug looks like at runtime, but it's slightly 
> > > > weird that no
> > > > one noticed.  This is from static analysis and I haven't tested it 
> > > > myself.
> > > >
> > > >  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > > > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > > > index 00b7c885772b..7e8c74da6a74 100644
> > > > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > > > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > > > @@ -187,6 +187,8 @@ int smu_sys_set_pp_table(struct smu_context *smu,
> > > > void *buf, size_t size)
> > > > if (ret)
> > > > pr_info("smu reset failed, ret = %d\n", ret);
> > > >
> > > > +   return ret;
> >
> > Why not return 0?
>
> It's not necessarily zero.

Oops, I was looking at the invisble goto after the pr_info :)

julia
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/powerplay: Fix double unlock bug in smu_sys_set_pp_table()

2019-03-21 Thread Julia Lawall



On Thu, 21 Mar 2019, Huang, Ray wrote:

> > -Original Message-
> > From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> > Sent: Thursday, March 21, 2019 2:28 PM
> > To: Deucher, Alexander ; Wang, Kevin(Yang)
> > 
> > Cc: Koenig, Christian ; Zhou, David(ChunMing)
> > ; David Airlie ; Daniel Vetter
> > ; Huang, Ray ; Gao, Likun
> > ; Gui, Jack ; amd-
> > g...@lists.freedesktop.org; kernel-janit...@vger.kernel.org
> > Subject: [PATCH] drm/amd/powerplay: Fix double unlock bug in
> > smu_sys_set_pp_table()
> >
> > We already unlocked a few lines earlier so this code unlocks twice on the
> > success path.
> >
> > Fixes: 5809d7420f97 ("drm/amd/powerplay: implement sysfs of pp_table for
> > smu11 (v2)")
> > Signed-off-by: Dan Carpenter 
>
> Nice catch!  Thanks, Dan.
> Kevin, could you please verify this patch.
> Reviewed-by: Huang Rui 
>
> > ---
> > I'm not sure what this bug looks like at runtime, but it's slightly weird 
> > that no
> > one noticed.  This is from static analysis and I haven't tested it myself.
> >
> >  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > index 00b7c885772b..7e8c74da6a74 100644
> > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > @@ -187,6 +187,8 @@ int smu_sys_set_pp_table(struct smu_context *smu,
> > void *buf, size_t size)
> > if (ret)
> > pr_info("smu reset failed, ret = %d\n", ret);
> >
> > +   return ret;

Why not return 0?

julia

> > +
> >  failed:
> > mutex_unlock(>mutex);
> > return ret;
> > --
> > 2.17.1
>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/powerplay: fix memdup.cocci warnings

2019-03-04 Thread Julia Lawall

From: kbuild test robot 

Simplify the code a bit by using kmemdup instead of kzalloc and memcpy.

Generated by: scripts/coccinelle/api/memdup.cocci

Fixes: 76760fe3c00d ("drm/amd/powerplay: add function to store overdrive 
information for smu11")
CC: Likun Gao 
Signed-off-by: kbuild test robot 
Signed-off-by: Julia Lawall 
---

tree:   git://people.freedesktop.org/~agd5f/linux.git drm-next-5.2-wip
head:   25752e1fc83e9f983b11d680fc7bfc129b4eaae6
commit: 76760fe3c00d04f25cc1a6115294310d4effeb77 [161/226] drm/amd/powerplay: 
add function to store overdrive information for smu11
:: branch date: 6 hours ago
:: commit date: 6 hours ago

 vega20_ppt.c |   24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

--- a/drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c
@@ -173,14 +173,12 @@ static int vega20_setup_od8_information(
if (table_context->od_feature_capabilities)
return -EINVAL;

-   table_context->od_feature_capabilities = 
kzalloc(od_feature_array_size, GFP_KERNEL);
+   table_context->od_feature_capabilities = 
kmemdup(_table->OverDrive8Table.ODFeatureCapabilities,
+
od_feature_array_size,
+GFP_KERNEL);
if (!table_context->od_feature_capabilities)
return -ENOMEM;

-   memcpy(table_context->od_feature_capabilities,
-  _table->OverDrive8Table.ODFeatureCapabilities,
-  od_feature_array_size);
-
/* Setup correct ODSettingCount, and store ODSettingArray from
 * powerplay table to od_settings_max and od_setting_min */
od_setting_count =
@@ -194,7 +192,9 @@ static int vega20_setup_od8_information(
if (table_context->od_settings_max)
return -EINVAL;

-   table_context->od_settings_max = kzalloc(od_setting_array_size, 
GFP_KERNEL);
+   table_context->od_settings_max = 
kmemdup(_table->OverDrive8Table.ODSettingsMax,
+od_setting_array_size,
+GFP_KERNEL);

if (!table_context->od_settings_max) {
kfree(table_context->od_feature_capabilities);
@@ -202,14 +202,12 @@ static int vega20_setup_od8_information(
return -ENOMEM;
}

-   memcpy(table_context->od_settings_max,
-  _table->OverDrive8Table.ODSettingsMax,
-  od_setting_array_size);
-
if (table_context->od_settings_min)
return -EINVAL;

-   table_context->od_settings_min = kzalloc(od_setting_array_size, 
GFP_KERNEL);
+   table_context->od_settings_min = 
kmemdup(_table->OverDrive8Table.ODSettingsMin,
+od_setting_array_size,
+GFP_KERNEL);

if (!table_context->od_settings_min) {
kfree(table_context->od_feature_capabilities);
@@ -218,10 +216,6 @@ static int vega20_setup_od8_information(
table_context->od_settings_max = NULL;
return -ENOMEM;
}
-
-   memcpy(table_context->od_settings_min,
-  _table->OverDrive8Table.ODSettingsMin,
-  od_setting_array_size);
}

return 0;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/6] drm/dp_mst: Introduce drm_dp_mst_connector_atomic_check() (fwd)

2018-09-19 Thread Julia Lawall

Hello,

I don't think you can update the loop index variable in
list_for_each_entry, because the mcro uses th index variable to get to the
next element.  Perhaps list_for_each_entry_safe would be more suitable?

julia

-- Forwarded message --
Date: Thu, 20 Sep 2018 04:30:13 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [PATCH 1/6] drm/dp_mst: Introduce
drm_dp_mst_connector_atomic_check()

CC: kbuild-...@01.org
In-Reply-To: <20180918230637.20700-2-ly...@redhat.com>
References: <20180918230637.20700-2-ly...@redhat.com>
TO: Lyude Paul 
CC: dri-de...@lists.freedesktop.org, nouv...@lists.freedesktop.org, 
intel-...@lists.freedesktop.org, amd-gfx@lists.freedesktop.org
CC: David Airlie , linux-ker...@vger.kernel.org, 
sta...@vger.kernel.org, Sean Paul 

Hi Lyude,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.19-rc4 next-20180919]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Lyude-Paul/Fix-legacy-DPMS-changes-with-MST/20180919-203434
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
:: branch date: 8 hours ago
:: commit date: 8 hours ago

>> drivers/gpu/drm/drm_dp_mst_topology.c:3144:1-20: iterator with update on 
>> line 3145

# 
https://github.com/0day-ci/linux/commit/f8df31d5221b9a6da6698d4a37e622253bb17cdc
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout f8df31d5221b9a6da6698d4a37e622253bb17cdc
vim +3144 drivers/gpu/drm/drm_dp_mst_topology.c

3f3353b7 Pandiyan, Dhinakaran 2017-04-20  3131
f8df31d5 Lyude Paul   2018-09-18  3132  static bool
f8df31d5 Lyude Paul   2018-09-18  3133  
drm_dp_mst_connector_still_exists(struct drm_connector *connector,
f8df31d5 Lyude Paul   2018-09-18  3134  
  struct drm_dp_mst_topology_mgr *mgr,
f8df31d5 Lyude Paul   2018-09-18  3135  
  struct drm_dp_mst_branch *mstb)
f8df31d5 Lyude Paul   2018-09-18  3136  {
f8df31d5 Lyude Paul   2018-09-18  3137  struct drm_dp_mst_port 
*port;
f8df31d5 Lyude Paul   2018-09-18  3138  bool exists = false;
f8df31d5 Lyude Paul   2018-09-18  3139
f8df31d5 Lyude Paul   2018-09-18  3140  mstb = 
drm_dp_get_validated_mstb_ref(mgr, mstb);
f8df31d5 Lyude Paul   2018-09-18  3141  if (!mstb)
f8df31d5 Lyude Paul   2018-09-18  3142  return false;
f8df31d5 Lyude Paul   2018-09-18  3143
f8df31d5 Lyude Paul   2018-09-18 @3144  
list_for_each_entry(port, >ports, next) {
f8df31d5 Lyude Paul   2018-09-18 @3145  port = 
drm_dp_get_validated_port_ref(mgr, port);
f8df31d5 Lyude Paul   2018-09-18  3146  if (!port)
f8df31d5 Lyude Paul   2018-09-18  3147  
continue;
f8df31d5 Lyude Paul   2018-09-18  3148
f8df31d5 Lyude Paul   2018-09-18  3149  exists = 
(port->connector == connector ||
f8df31d5 Lyude Paul   2018-09-18  3150
(port->mstb &&
f8df31d5 Lyude Paul   2018-09-18  3151 
drm_dp_mst_connector_still_exists(connector, mgr,
f8df31d5 Lyude Paul   2018-09-18  3152  
 port->mstb)));
f8df31d5 Lyude Paul   2018-09-18  3153
f8df31d5 Lyude Paul   2018-09-18  3154  
drm_dp_put_port(port);
f8df31d5 Lyude Paul   2018-09-18  3155  if (exists)
f8df31d5 Lyude Paul   2018-09-18  3156  break;
f8df31d5 Lyude Paul   2018-09-18  3157  }
f8df31d5 Lyude Paul   2018-09-18  3158
f8df31d5 Lyude Paul   2018-09-18  3159  
drm_dp_put_mst_branch_device(mstb);
f8df31d5 Lyude Paul   2018-09-18  3160  return exists;
f8df31d5 Lyude Paul   2018-09-18  3161  }
f8df31d5 Lyude Paul   2018-09-18  3162

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: fix spelling mistake: "Usupported" -> "Unsupported"

2018-04-03 Thread Julia Lawall



On Tue, 3 Apr 2018, Jani Nikula wrote:

> On Fri, 30 Mar 2018, Colin King  wrote:
> > From: Colin Ian King 
> >
> > Trivial fix to spelling mistake in DRM_ERROR error message text
>
> Thanks for the patch.
>
> Please do consider limiting the distribution in the future,
> though. There's no need to include lkml or even dri-devel for trivial
> patches like this.

It's complex to have to remember the preferences for every subsystem.
Preferences should be expressed in the MAINTAINERS file in some way.
Also, since no one reads lkml, does it hurt to have even trivial patches?

julia

>
> Thanks,
> Jani.
>
> >
> > Signed-off-by: Colin Ian King 
> > ---
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index e42a28e3adc5..1df1c91b6ae5 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -1521,7 +1521,7 @@ static int amdgpu_dm_initialize_drm_device(struct 
> > amdgpu_device *adev)
> > break;
> >  #endif
> > default:
> > -   DRM_ERROR("Usupported ASIC type: 0x%X\n", adev->asic_type);
> > +   DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
> > goto fail;
> > }
> >
> > @@ -1715,7 +1715,7 @@ static int dm_early_init(void *handle)
> > break;
> >  #endif
> > default:
> > -   DRM_ERROR("Usupported ASIC type: 0x%X\n", adev->asic_type);
> > +   DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
> > return -EINVAL;
> > }
>
> --
> Jani Nikula, Intel Open Source Technology Center
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Outreachy kernel] Re: [PATCH] gpu: drm: Use list_first_entry instead of list_entry

2018-03-19 Thread Julia Lawall



On Mon, 19 Mar 2018, Christian König wrote:

> Mhm, actually that patch isn't correct. What we grab get here is the next
> entry, not the first one.
>
> We don't have an alias list_next_entry for list_first_entry?

As compared to the semantic patch I proposed earlier today, it would seem
that list_first_entry is useful when the types are different?  One would
have to check the result of course, but a list eleemnt with the same type
as the structure that contains the list might be unlikely?

julia

>
> Regards,
> Christian.
>
> Am 18.03.2018 um 22:51 schrieb Arushi Singhal:
> > This patch replaces list_entry with list_first_entry as it makes the
> > code more clear.
> > Done using coccinelle:
> >
> > @@
> > expression e;
> > @@
> > (
> > - list_entry(e->next,
> > + list_first_entry(e,
> >...)
> > |
> > - list_entry(e->prev,
> > + list_last_entry(e,
> >...)
> > )
> >
> > Signed-off-by: Arushi Singhal 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 4 ++--
> >   drivers/gpu/drm/omapdrm/dss/display.c  | 4 ++--
> >   drivers/gpu/drm/radeon/radeon_sa.c | 4 ++--
> >   3 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> > index 3144400..646f593 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> > @@ -158,7 +158,7 @@ static void amdgpu_sa_bo_try_free(struct
> > amdgpu_sa_manager *sa_manager)
> > if (sa_manager->hole->next == _manager->olist)
> > return;
> >   - sa_bo = list_entry(sa_manager->hole->next, struct amdgpu_sa_bo,
> > olist);
> > +   sa_bo = list_first_entry(sa_manager->hole, struct amdgpu_sa_bo,
> > olist);
> > list_for_each_entry_safe_from(sa_bo, tmp, _manager->olist, olist) {
> > if (sa_bo->fence == NULL ||
> > !dma_fence_is_signaled(sa_bo->fence)) {
> > @@ -183,7 +183,7 @@ static inline unsigned amdgpu_sa_bo_hole_eoffset(struct
> > amdgpu_sa_manager *sa_ma
> > struct list_head *hole = sa_manager->hole;
> > if (hole->next != _manager->olist) {
> > -   return list_entry(hole->next, struct amdgpu_sa_bo,
> > olist)->soffset;
> > +   return list_first_entry(hole, struct amdgpu_sa_bo,
> > olist)->soffset;
> > }
> > return sa_manager->size;
> >   }
> > diff --git a/drivers/gpu/drm/omapdrm/dss/display.c
> > b/drivers/gpu/drm/omapdrm/dss/display.c
> > index 0c9480b..fb9ecae 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/display.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/display.c
> > @@ -158,8 +158,8 @@ struct omap_dss_device *omap_dss_get_next_device(struct
> > omap_dss_device *from)
> > goto out;
> > }
> >   - dssdev = list_entry(l->next, struct omap_dss_device,
> > -   panel_list);
> > +   dssdev = list_first_entry(l, struct omap_dss_device,
> > + panel_list);
> > omap_dss_get_device(dssdev);
> > goto out;
> > }
> > diff --git a/drivers/gpu/drm/radeon/radeon_sa.c
> > b/drivers/gpu/drm/radeon/radeon_sa.c
> > index 197b157..66c0482 100644
> > --- a/drivers/gpu/drm/radeon/radeon_sa.c
> > +++ b/drivers/gpu/drm/radeon/radeon_sa.c
> > @@ -158,7 +158,7 @@ static void radeon_sa_bo_try_free(struct
> > radeon_sa_manager *sa_manager)
> > if (sa_manager->hole->next == _manager->olist)
> > return;
> >   - sa_bo = list_entry(sa_manager->hole->next, struct radeon_sa_bo,
> > olist);
> > +   sa_bo = list_first_entry(sa_manager->hole, struct radeon_sa_bo,
> > olist);
> > list_for_each_entry_safe_from(sa_bo, tmp, _manager->olist, olist) {
> > if (sa_bo->fence == NULL ||
> > !radeon_fence_signaled(sa_bo->fence)) {
> > return;
> > @@ -182,7 +182,7 @@ static inline unsigned radeon_sa_bo_hole_eoffset(struct
> > radeon_sa_manager *sa_ma
> > struct list_head *hole = sa_manager->hole;
> > if (hole->next != _manager->olist) {
> > -   return list_entry(hole->next, struct radeon_sa_bo,
> > olist)->soffset;
> > +   return list_first_entry(hole, struct radeon_sa_bo,
> > olist)->soffset;
> > }
> > return sa_manager->size;
> >   }
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/8b1e22f8-7a05-b66b-8825-7d4d97e46dac%40amd.com.
> For more options, visit https://groups.google.com/d/optout.
>___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

[PATCH] drm/radeon: adjust tested variable

2018-01-28 Thread Julia Lawall

Check the variable that was most recently initialized.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression x, y, f, g, e, m;
statement S1,S2,S3,S4;
@@

x = f(...);
if (\(<+...x...+>\\)) S1 else S2
(
x = g(...);
|
m = g(...,,...);
|
y = g(...);
*if (e)
 S3 else S4
)
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/gpu/drm/radeon/radeon_uvd.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index d34d1cf..95f4db7 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -995,7 +995,7 @@ int radeon_uvd_calc_upll_dividers(struct radeon_device 
*rdev,
/* calc dclk divider with current vco freq */
dclk_div = radeon_uvd_calc_upll_post_div(vco_freq, dclk,
 pd_min, pd_even);
-   if (vclk_div > pd_max)
+   if (dclk_div > pd_max)
break; /* vco is too big, it has to stop */
 
/* calc score with current vco freq */

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

2018-01-02 Thread Julia Lawall



On Tue, 2 Jan 2018, Bob Peterson wrote:

> - Original Message -
> | Drop newline at the end of a message string when the printing function adds
> | a newline.
>
> Hi Julia,
>
> NACK.
>
> As much as it's a pain when searching the source code for output strings,
> this patch set goes against the accepted Linux coding style document. See:
>
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings

I don't think that's the case:

"However, never break user-visible strings such as printk messages,
because that breaks the ability to grep for them."

julia

>
> Regards,
>
> Bob Peterson
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

2018-01-02 Thread Julia Lawall



On Tue, 2 Jan 2018, Bob Peterson wrote:

> - Original Message -
> | - Original Message -
> | | Drop newline at the end of a message string when the printing function 
> adds
> | | a newline.
> |
> | Hi Julia,
> |
> | NACK.
> |
> | As much as it's a pain when searching the source code for output strings,
> | this patch set goes against the accepted Linux coding style document. See:
> |
> | 
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings
> |
> | Regards,
> |
> | Bob Peterson
> |
> |
> Hm. I guess I stand corrected. The document reads:
>
> "However, never break user-visible strings such as printk messages, because 
> that breaks the ability to grep for them."
>
> Still, the GFS2 and DLM code has a plethora of broken-up printk messages,
> and I don't like the thought of re-combining them all.

Actually, the point of the patch was to remove the unnecessary \n at the
end of the string, because log_print will add another one.  If you prefer
to keep the string broken up, I can resend the patch in that form, but
without the unnecessary \n.

julia
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

2018-01-02 Thread Julia Lawall



On Tue, 2 Jan 2018, Bart Van Assche wrote:

> On Tue, 2018-01-02 at 15:00 +0100, Julia Lawall wrote:
> > On Tue, 2 Jan 2018, Bob Peterson wrote:
> > > - Original Message -
> > > > - Original Message -
> > > >
> > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages,
> > > and I don't like the thought of re-combining them all.
> >
> > Actually, the point of the patch was to remove the unnecessary \n at the
> > end of the string, because log_print will add another one.  If you prefer
> > to keep the string broken up, I can resend the patch in that form, but
> > without the unnecessary \n.
>
> Please combine any user-visible strings into a single line for which the
> unneeded newline is dropped since these strings are modified anyway by
> your patch.

That is what the submitted patch (2/12 specifically) did.

julia
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 11/12] drm/amd/powerplay: drop unneeded newline

2017-12-27 Thread Julia Lawall



On Wed, 27 Dec 2017, Michel Dänzer wrote:

> On 2017-12-27 03:51 PM, Julia Lawall wrote:
> > PP_ASSERT_WITH_CODE prints a newline at the end of the message string,
> > so the message string does not need to include a newline explicitly.
> > Done using Coccinelle.
> >
> > Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
> >
> > ---
> >
> > I couldn't figure out how to configure the kernel to get any of this code
> > to compile.
>
> Just enabling CONFIG_DRM_AMDGPU should be enough AFAICT.

That seems to work.  Thanks.

julia___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 11/12] drm/amd/powerplay: drop unneeded newline

2017-12-27 Thread Julia Lawall

PP_ASSERT_WITH_CODE prints a newline at the end of the message string,
so the message string does not need to include a newline explicitly.
Done using Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---

I couldn't figure out how to configure the kernel to get any of this code
to compile.

 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c|   12 
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c  |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c   |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c |2 +-
 5 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 40adc85..8d7fd06 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -2266,14 +2266,18 @@ static int 
smu7_set_private_data_based_on_pptable_v0(struct pp_hwmgr *hwmgr)
struct phm_clock_voltage_dependency_table *allowed_mclk_vddci_table = 
hwmgr->dyn_state.vddci_dependency_on_mclk;
 
PP_ASSERT_WITH_CODE(allowed_sclk_vddc_table != NULL,
-   "VDDC dependency on SCLK table is missing. This table is 
mandatory\n", return -EINVAL);
+   "VDDC dependency on SCLK table is missing. This table is 
mandatory",
+   return -EINVAL);
PP_ASSERT_WITH_CODE(allowed_sclk_vddc_table->count >= 1,
-   "VDDC dependency on SCLK table has to have is missing. This 
table is mandatory\n", return -EINVAL);
+   "VDDC dependency on SCLK table has to have is missing. This 
table is mandatory",
+   return -EINVAL);
 
PP_ASSERT_WITH_CODE(allowed_mclk_vddc_table != NULL,
-   "VDDC dependency on MCLK table is missing. This table is 
mandatory\n", return -EINVAL);
+   "VDDC dependency on MCLK table is missing. This table is 
mandatory",
+   return -EINVAL);
PP_ASSERT_WITH_CODE(allowed_mclk_vddc_table->count >= 1,
-   "VDD dependency on MCLK table has to have is missing. This 
table is mandatory\n", return -EINVAL);
+   "VDD dependency on MCLK table has to have is missing. This 
table is mandatory",
+   return -EINVAL);
 
data->min_vddc_in_pptable = 
(uint16_t)allowed_sclk_vddc_table->entries[0].v;
data->max_vddc_in_pptable = 
(uint16_t)allowed_sclk_vddc_table->entries[allowed_sclk_vddc_table->count - 
1].v;
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
index 085d81c..427daa6 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
@@ -1799,7 +1799,7 @@ static int 
fiji_populate_clock_stretcher_data_table(struct pp_hwmgr *hwmgr)
phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
PHM_PlatformCaps_ClockStretcher);
PP_ASSERT_WITH_CODE(false,
-   "Stretch Amount in PPTable not supported\n",
+   "Stretch Amount in PPTable not supported",
return -EINVAL);
}
 
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c
index 1253126..6400065 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c
@@ -546,7 +546,7 @@ static int iceland_get_std_voltage_value_sidd(struct 
pp_hwmgr *hwmgr,
 
/* SCLK/VDDC Dependency Table has to exist. */
PP_ASSERT_WITH_CODE(NULL != hwmgr->dyn_state.vddc_dependency_on_sclk,
-   "The SCLK/VDDC Dependency Table does not exist.\n",
+   "The SCLK/VDDC Dependency Table does not exist.",
return -EINVAL);
 
if (NULL == hwmgr->dyn_state.cac_leakage_table) {
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
index cdb4765..fd874f7 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
@@ -1652,7 +1652,7 @@ static int 
polaris10_populate_clock_stretcher_data_table(struct pp_hwmgr *hwmgr)
phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
PHM_PlatformCaps_ClockStretcher);
PP_ASSERT_WITH_CODE(false,
-   "Stretch Amount in PPTable not supported\n",
+   "Stretch Amount in PPTable not supported"

[PATCH 00/12] drop unneeded newline

2017-12-27 Thread Julia Lawall

Drop newline at the end of a message string when the printing function adds
a newline.

The complete semantic patch that detects this issue is as shown below
(http://coccinelle.lip6.fr/).  It works in two phases - the first phase
counts how many uses of a function involve a newline and how many don't,
and then the second phase removes newlines in the case of calls where a
newline is used one fourth of the times or less.

This approach is only moderately reliable, and all patches have been
checked to ensure that the newline is not needed.

This also converts some cases of string concatenation to single strings in
modified code, as this improves greppability.

// 
virtual after_start

@initialize:ocaml@
@@

let withnl = Hashtbl.create 101
let withoutnl = Hashtbl.create 101

let ignore =
  ["strcpy";"strlcpy";"strcat";"strlcat";"strcmp";"strncmp";"strcspn";
"strsep";"sprintf";"printf";"strncasecmp";"seq_printf";"strstr";"strspn";
"strlen";"strpbrk";"strtok_r";"memcmp";"memcpy"]

let dignore = ["tools";"samples"]

let inc tbl k =
  let cell =
try Hashtbl.find tbl k
with Not_found ->
  let cell = ref 0 in
  Hashtbl.add tbl k cell;
  cell in
  cell := 1 + !cell

let endnl c =
  let len = String.length c in
  try
String.get c (len-3) = '\\' && String.get c (len-2) = 'n' &&
String.get c (len-1) = '"'
  with _ -> false

let clean_string s extra =
  let pieces = Str.split (Str.regexp "\" \"") s in
  let nonempty s =
not (s = "") && String.get s 0 = '"' && not (String.get s 1 = '"') in
  let rec loop = function
  [] -> []
| [x] -> [x]
| x::y::rest ->
if nonempty x && nonempty y
then
  let xend = String.get x (String.length x - 2) = ' ' in
  let yend = String.get y 1 = ' ' in
  match (xend,yend) with
(true,false) | (false,true) -> x :: (loop (y::rest))
  | (true,true) ->
  x :: (loop (((String.sub y 0 (String.length y - 2))^"\"")::rest))
  | (false,false) ->
  ((String.sub x 0 (String.length x - 1)) ^ " \"") ::
  (loop (y::rest))
else x :: (loop (y::rest)) in
  (String.concat "" (loop pieces))^extra

@r depends on !after_start@
constant char[] c;
expression list[n] es;
identifier f;
position p;
@@

f@p(es,c,...)

@script:ocaml@
f << r.f;
n << r.n;
p << r.p;
c << r.c;
@@

let pieces = Str.split (Str.regexp "/") (List.hd p).file in
if not (List.mem f ignore) &&
  List.for_all (fun x -> not (List.mem x pieces)) dignore
then
  (if endnl c
  then inc withnl (f,n)
  else inc withoutnl (f,n))

@finalize:ocaml depends on !after_start@
w1 << merge.withnl;
w2 << merge.withoutnl;
@@

let names = ref [] in
let incn tbl k v =
  let cell =
try Hashtbl.find tbl k
with Not_found ->
  begin
let cell = ref 0 in
Hashtbl.add tbl k cell;
cell
  end in
  (if not (List.mem k !names) then names := k :: !names);
  cell := !v + !cell in
List.iter (function w -> Hashtbl.iter (incn withnl) w) w1;
List.iter (function w -> Hashtbl.iter (incn withoutnl) w) w2;

List.iter
  (function name ->
let wth = try !(Hashtbl.find withnl name) with _ -> 0 in
let wo = try !(Hashtbl.find withoutnl name) with _ -> 0 in
if wth > 0 && wth <= wo / 3 then Hashtbl.remove withnl name
else (Printf.eprintf "dropping %s %d %d\n" (fst name) wth wo; 
Hashtbl.remove withoutnl name; Hashtbl.remove withnl name))
  !names;

let it = new iteration() in
it#add_virtual_rule After_start;
it#register()

@s1 depends on after_start@
constant char[] c;
expression list[n] es;
identifier f;
position p;
@@

f(es,c@p,...)

@script:ocaml s2@
f << s1.f;
n << s1.n;
c << s1.c;
newc;
@@

try
  let _ = Hashtbl.find withnl (f,n) in
  if endnl c
  then Coccilib.include_match false
  else newc :=
make_expr(clean_string (String.sub c 0 (String.length c - 1)) "\\n\"")
with Not_found ->
try
  let _ = Hashtbl.find withoutnl (f,n) in
  if endnl c
  then newc :=
make_expr(clean_string (String.sub c 0 (String.length c - 3)) "\"")
  else Coccilib.include_match false
with Not_found -> Coccilib.include_match false

@@
constant char[] s1.c;
position s1.p;
expression s2.newc;
@@

- c@p
+ newc
// 

---

 arch/arm/mach-davinci/board-da850-evm.c |4 ++--
 drivers/block/DAC960.c  |4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c|   12 
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c  |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c   |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c |2 +-
 drivers/media/usb/pvrusb2/pvrusb2-hdw.c |3 ++-
 drivers/s390/block/dasd_diag.c  |3 +--
 drivers/scsi/hpsa.c |2 +-
 fs/dlm/plock.c  |3 +--
 fs/ext2/super.c

Re: [Outreachy kernel] [PATCH v3] drm/amd/powerplay: Remove unnecessary cast on void pointer

2017-10-13 Thread Julia Lawall



On Sat, 14 Oct 2017, Harsha Sharma wrote:

> Done with following coccinelle patch
>
> @r@
> expression x;
> void* e;
> type T;
> identifier f;
> @@
> (
>   *((T *)e)
> |
>   ((T *)x)[...]
> |
>   ((T*)x)->f
> |
>
> - (T*)
>   e
> )
>
> Signed-off-by: Harsha Sharma 
> ---
> Changes in v3:
>  -Removed unnecessary lines
>  -Remove more useless casts
> Changes in v2:
>  -Remove unnecessary parentheses
>  -Remove one more useless cast
>
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c |   6 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c|   8 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c   |   2 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c |   6 +-
>  .../gpu/drm/amd/powerplay/hwmgr/processpptables.c  |   2 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 177 
> ++---
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c |   4 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  22 +--
>  .../gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c   |   2 +-
>  9 files changed, 107 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> index bc839ff0bdd0..f22104c78dcb 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> @@ -474,7 +474,7 @@ static int cz_tf_upload_pptable_to_smu(struct pp_hwmgr 
> *hwmgr, void *input,
>   PP_ASSERT_WITH_CODE((0 == ret && NULL != table),
>   "Fail to get clock table from SMU!", return 
> -EINVAL;);
>
> - clock_table = (struct SMU8_Fusion_ClkTable *)table;
> + clock_table = table;
>
>   /* patch clock table */
>   PP_ASSERT_WITH_CODE((vddc_table->count <= CZ_MAX_HARDWARE_POWERLEVELS),
> @@ -868,8 +868,8 @@ static int cz_tf_update_low_mem_pstate(struct pp_hwmgr 
> *hwmgr,
>  {
>   bool disable_switch;
>   bool enable_low_mem_state;
> - struct cz_hwmgr *hw_data = (struct cz_hwmgr *)(hwmgr->backend);
> - const struct phm_set_power_state_input *states = (struct 
> phm_set_power_state_input *)input;
> + struct cz_hwmgr *hw_data = hwmgr->backend;
> + const struct phm_set_power_state_input *states = input;
>   const struct cz_power_state *pnew_state = 
> cast_const_PhwCzPowerState(states->pnew_state);
>
>   if (hw_data->sys_info.nb_dpm_enable) {
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> index 9547f265a8bb..5d63a1b18b39 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> @@ -469,7 +469,7 @@ int phm_reset_single_dpm_table(void *table,
>  {
>   int i;
>
> - struct vi_dpm_table *dpm_table = (struct vi_dpm_table *)table;
> + struct vi_dpm_table *dpm_table = table;
>
>   dpm_table->count = count > max ? max : count;
>
> @@ -484,7 +484,7 @@ void phm_setup_pcie_table_entry(
>   uint32_t index, uint32_t pcie_gen,
>   uint32_t pcie_lanes)
>  {
> - struct vi_dpm_table *dpm_table = (struct vi_dpm_table *)table;
> + struct vi_dpm_table *dpm_table = table;
>   dpm_table->dpm_level[index].value = pcie_gen;
>   dpm_table->dpm_level[index].param1 = pcie_lanes;
>   dpm_table->dpm_level[index].enabled = 1;
> @@ -494,7 +494,7 @@ int32_t phm_get_dpm_level_enable_mask_value(void *table)
>  {
>   int32_t i;
>   int32_t mask = 0;
> - struct vi_dpm_table *dpm_table = (struct vi_dpm_table *)table;
> + struct vi_dpm_table *dpm_table = table;
>
>   for (i = dpm_table->count; i > 0; i--) {
>   mask = mask << 1;
> @@ -566,7 +566,7 @@ int phm_find_boot_level(void *table,
>  {
>   int result = -EINVAL;
>   uint32_t i;
> - struct vi_dpm_table *dpm_table = (struct vi_dpm_table *)table;
> + struct vi_dpm_table *dpm_table = table;
>
>   for (i = 0; i < dpm_table->count; i++) {
>   if (value == dpm_table->dpm_level[i].value) {
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> index 953e0c9ad7cd..676f2e8bb2ee 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> @@ -579,7 +579,7 @@ static ATOM_GPIO_PIN_LUT *get_gpio_lookup_table(void 
> *device)
>   PP_ASSERT_WITH_CODE((NULL != table_address),
>   "Error retrieving BIOS Table Address!", return NULL;);
>
> - return (ATOM_GPIO_PIN_LUT *)table_address;
> + return table_address;
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c
> index c062844b15f3..05e3f5302994 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c
> @@ -66,7 +66,7 @@ static struct atom_voltage_objects_info_v4_1 
>

Re: [Outreachy kernel] [PATCH v2] drm/amd/powerplay: Remove unnecessary cast on void pointer

2017-10-13 Thread Julia Lawall

> @@ -3400,7 +3400,7 @@ static int smu7_read_sensor(struct pp_hwmgr *hwmgr, int 
> idx,
>  static int smu7_find_dpm_states_clocks_in_dpm_table(struct pp_hwmgr *hwmgr, 
> const void *input)
>  {
>   const struct phm_set_power_state_input *states =
> - (const struct phm_set_power_state_input *)input;
> + input;

Actually, there are some more cleanup opportunities heer and in some other
cases.  There is no need for the newline before input.

julia
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Outreachy kernel] [PATCH] drm/amd/powerplay: Remove unnecessary cast on void pointer

2017-10-13 Thread Julia Lawall



On Fri, 13 Oct 2017, Harsha Sharma wrote:

> Done with following coccinelle patch
>
> @r@
> expression x;
> void* e;
> type T;
> identifier f;
> @@
> (
>   *((T *)e)
> |
>   ((T *)x)[...]
> |
>   ((T*)x)->f
> |
>
> - (T*)
>   e
> )
>
> Signed-off-by: Harsha Sharma 
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c|  6 +++---
>  drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c   |  8 
>  drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c  |  2 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c|  6 +++---
>  drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c |  2 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c  | 18 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c|  4 ++--
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 12 ++--
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c  |  2 +-
>  9 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> index bc839ff0bdd0..897f22f3 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> @@ -474,7 +474,7 @@ static int cz_tf_upload_pptable_to_smu(struct pp_hwmgr 
> *hwmgr, void *input,
>   PP_ASSERT_WITH_CODE((0 == ret && NULL != table),
>   "Fail to get clock table from SMU!", return 
> -EINVAL;);
>
> - clock_table = (struct SMU8_Fusion_ClkTable *)table;
> + clock_table = table;
>
>   /* patch clock table */
>   PP_ASSERT_WITH_CODE((vddc_table->count <= CZ_MAX_HARDWARE_POWERLEVELS),
> @@ -868,8 +868,8 @@ static int cz_tf_update_low_mem_pstate(struct pp_hwmgr 
> *hwmgr,
>  {
>   bool disable_switch;
>   bool enable_low_mem_state;
> - struct cz_hwmgr *hw_data = (struct cz_hwmgr *)(hwmgr->backend);
> - const struct phm_set_power_state_input *states = (struct 
> phm_set_power_state_input *)input;
> + struct cz_hwmgr *hw_data = (hwmgr->backend);
> + const struct phm_set_power_state_input *states = input;
>   const struct cz_power_state *pnew_state = 
> cast_const_PhwCzPowerState(states->pnew_state);
>
>   if (hw_data->sys_info.nb_dpm_enable) {
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> index 9547f265a8bb..5d63a1b18b39 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> @@ -469,7 +469,7 @@ int phm_reset_single_dpm_table(void *table,
>  {
>   int i;
>
> - struct vi_dpm_table *dpm_table = (struct vi_dpm_table *)table;
> + struct vi_dpm_table *dpm_table = table;
>
>   dpm_table->count = count > max ? max : count;
>
> @@ -484,7 +484,7 @@ void phm_setup_pcie_table_entry(
>   uint32_t index, uint32_t pcie_gen,
>   uint32_t pcie_lanes)
>  {
> - struct vi_dpm_table *dpm_table = (struct vi_dpm_table *)table;
> + struct vi_dpm_table *dpm_table = table;
>   dpm_table->dpm_level[index].value = pcie_gen;
>   dpm_table->dpm_level[index].param1 = pcie_lanes;
>   dpm_table->dpm_level[index].enabled = 1;
> @@ -494,7 +494,7 @@ int32_t phm_get_dpm_level_enable_mask_value(void *table)
>  {
>   int32_t i;
>   int32_t mask = 0;
> - struct vi_dpm_table *dpm_table = (struct vi_dpm_table *)table;
> + struct vi_dpm_table *dpm_table = table;
>
>   for (i = dpm_table->count; i > 0; i--) {
>   mask = mask << 1;
> @@ -566,7 +566,7 @@ int phm_find_boot_level(void *table,
>  {
>   int result = -EINVAL;
>   uint32_t i;
> - struct vi_dpm_table *dpm_table = (struct vi_dpm_table *)table;
> + struct vi_dpm_table *dpm_table = table;
>
>   for (i = 0; i < dpm_table->count; i++) {
>   if (value == dpm_table->dpm_level[i].value) {
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> index 953e0c9ad7cd..676f2e8bb2ee 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> @@ -579,7 +579,7 @@ static ATOM_GPIO_PIN_LUT *get_gpio_lookup_table(void 
> *device)
>   PP_ASSERT_WITH_CODE((NULL != table_address),
>   "Error retrieving BIOS Table Address!", return NULL;);
>
> - return (ATOM_GPIO_PIN_LUT *)table_address;
> + return table_address;
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c
> index c062844b15f3..05e3f5302994 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c
> @@ -66,7 +66,7 @@ static struct atom_voltage_objects_info_v4_1 
> *pp_atomfwctrl_get_voltage_info_tab
>  "Error retrieving BIOS Table Address!",
>  return NULL);
>
> -return (struct

[PATCH] drm/amd/amdkcl: fix drm-get-put.cocci warnings

2017-06-07 Thread Julia Lawall

 Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference() and
 drm_*_unreference() helpers.

Generated by: scripts/coccinelle/api/drm-get-put.cocci

CC: annwang <annie.w...@amd.com>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
Signed-off-by: Fengguang Wu <fengguang...@intel.com>
---
tree:   git://people.freedesktop.org/~agd5f/linux.git
amd-mainline-hybrid-4.11
head:   7ccf5ab3da7a87288cc0fd11910b212e4ac154a6
commit: 67207f0941969278dd47e2549fae4fe5502183c1 [1119/1800]
drm/amd/amdkcl: [4.7] fix dev->struct_mutex

Please take the patch only if it's a positive warning. Thanks!

 amdgpu_gem.c |   24 
 1 file changed, 12 insertions(+), 12 deletions(-)

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -108,9 +108,9 @@ void amdgpu_gem_force_release(struct amd
idr_for_each_entry(>object_idr, gobj, handle) {
WARN_ONCE(1, "And also active allocations!\n");
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 7, 0)
-   drm_gem_object_unreference(gobj);
+   drm_gem_object_put(gobj);
 #else
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
 #endif
}
idr_destroy(>object_idr);
@@ -287,7 +287,7 @@ int amdgpu_gem_create_ioctl(struct drm_d

r = drm_gem_handle_create(filp, gobj, );
/* drop reference from allocate - handle holds it now */
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
if (r)
return r;

@@ -365,7 +365,7 @@ int amdgpu_gem_userptr_ioctl(struct drm_

r = drm_gem_handle_create(filp, gobj, );
/* drop reference from allocate - handle holds it now */
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
if (r)
return r;

@@ -379,7 +379,7 @@ unlock_mmap_sem:
up_read(>mm->mmap_sem);

 release_object:
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);

return r;
 }
@@ -398,11 +398,11 @@ int amdgpu_mode_dumb_mmap(struct drm_fil
robj = gem_to_amdgpu_bo(gobj);
if (amdgpu_ttm_tt_get_usermm(robj->tbo.ttm) ||
(robj->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
return -EPERM;
}
*offset_p = amdgpu_bo_mmap_offset(robj);
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
return 0;
 }

@@ -472,7 +472,7 @@ int amdgpu_gem_wait_idle_ioctl(struct dr
} else
r = ret;

-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
return r;
 }

@@ -515,7 +515,7 @@ int amdgpu_gem_metadata_ioctl(struct drm
 unreserve:
amdgpu_bo_unreserve(robj);
 out:
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
return r;
 }

@@ -686,7 +686,7 @@ error_backoff:
ttm_eu_backoff_reservation(, );

 error_unref:
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
return r;
 }

@@ -748,7 +748,7 @@ int amdgpu_gem_op_ioctl(struct drm_devic
}

 out:
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
return r;
 }

@@ -776,7 +776,7 @@ int amdgpu_mode_dumb_create(struct drm_f

r = drm_gem_handle_create(file_priv, gobj, );
/* drop reference from allocate - handle holds it now */
-   drm_gem_object_unreference_unlocked(gobj);
+   drm_gem_object_put_unlocked(gobj);
if (r) {
return r;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: fix drm-get-put.cocci warnings

2017-06-07 Thread Julia Lawall

 Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference() and
 drm_*_unreference() helpers.

Generated by: scripts/coccinelle/api/drm-get-put.cocci

CC: Christian König <christian.koe...@amd.com>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
Signed-off-by: Fengguang Wu <fengguang...@intel.com>
---

Please take the patch only if it's a positive warning. Thanks!

 amdgpu_prime.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -173,7 +173,7 @@ amdgpu_gem_prime_foreign_bo(struct amdgp
continue;

ww_mutex_unlock(>tbo.resv->lock);
-   drm_gem_object_reference(>base);
+   drm_gem_object_get(>base);
return >base;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/4] sync_file: add a mutex to protect fence and callback members. (fwd)

2017-03-14 Thread Julia Lawall

Perhaps the mutex on line 410 needs to be considered on line 423.

julia

-- Forwarded message --

Hi Dave,

[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.11-rc2 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Dave-Airlie/sync_file-add-a-mutex-to-protect-fence-and-callback-members/20170314-155609
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
:: branch date: 52 minutes ago
:: commit date: 52 minutes ago

>> drivers/dma-buf/sync_file.c:423:2-8: preceding lock on line 410

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a319d478cdd641742a07f809ddb7c143a0a685e9
vim +423 drivers/dma-buf/sync_file.c

d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  404
if (copy_from_user(, (void __user *)arg, sizeof(info)))
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  405
return -EFAULT;
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  406
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  407
if (info.flags || info.pad)
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  408
return -EINVAL;
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  409
a319d478 drivers/dma-buf/sync_file.c Dave Airlie 2017-03-14 @410
mutex_lock(_file->lock);
a02b9dc9 drivers/dma-buf/sync_file.c Gustavo Padovan 2016-08-05  411
fences = get_fences(sync_file, _fences);
a02b9dc9 drivers/dma-buf/sync_file.c Gustavo Padovan 2016-08-05  412
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  413
/*
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  414
 * Passing num_fences = 0 means that userspace doesn't want to
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  415
 * retrieve any sync_fence_info. If num_fences = 0 we skip filling
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  416
 * sync_fence_info and return the actual number of fences on
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  417
 * info->num_fences.
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  418
 */
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  419
if (!info.num_fences)
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  420
goto no_fences;
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  421
a02b9dc9 drivers/dma-buf/sync_file.c Gustavo Padovan 2016-08-05  422
if (info.num_fences < num_fences)
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28 @423
return -EINVAL;
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  424
a02b9dc9 drivers/dma-buf/sync_file.c Gustavo Padovan 2016-08-05  425
size = num_fences * sizeof(*fence_info);
d4cab38e drivers/staging/android/sync_file.c Gustavo Padovan 2016-04-28  426
fence_info = kzalloc(size, GFP_KERNEL);

:: The code at line 423 was first introduced by commit
:: d4cab38e153d62ecd502645390c0289c1b8337df staging/android: prepare 
sync_file for de-staging

:: TO: Gustavo Padovan 
:: CC: Greg Kroah-Hartman 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

38 matches

Mail list logo