From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 020BD38DC0 for ; Fri, 3 Jan 2025 09:35:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.188 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735896927; cv=none; b=GiKWQ1j0YTh4qh1ruNygTMRhYbwUxPiWJUvL7ck4kXAXWltSycT1hLL+J2so1OtR3GTqBs9T4gDTztTu3CL9FKqykzy2CoiRoaO8Ckg1jmfwRQqMk+SRcNRQWgC+x0NcOmOQjxH44cZtL7opZzQkM11TRDsbbPDY8HIMCx/l3io= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735896927; c=relaxed/simple; bh=gVFUV3IRfWwwSfZfrYaJANt8hJOiucgVJoFcy0A9cCk=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=R1kcCmFd2MZ7wN/qBnID0QI+NxyVqUposOxBGpfxmHHb3THP4d/A5KmPEA9XPPVI2RtXjv5kQRNwGDsU/W8k5xkaB7+V5BhMG+WrRmk6e6guECfWipGm+k3El2YdJ81qFQuTI6VMRJO3JU9tN/qSI0M7dzWuDzNXf/oX3lAXxsY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.188 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.163.48]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4YPddZ6ZXZzckM2; Fri, 3 Jan 2025 17:32:18 +0800 (CST) Received: from kwepemd500013.china.huawei.com (unknown [7.221.188.12]) by mail.maildlp.com (Postfix) with ESMTPS id 11BC5180064; Fri, 3 Jan 2025 17:35:22 +0800 (CST) Received: from [10.159.166.136] (10.159.166.136) by kwepemd500013.china.huawei.com (7.221.188.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1258.34; Fri, 3 Jan 2025 17:35:20 +0800 Message-ID: <5ddc4d0d-148f-4649-9e18-386272abca67@huawei.com> Date: Fri, 3 Jan 2025 17:35:19 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v8 drm-dp 0/5] Add dp module in hibmc driver To: Dmitry Baryshkov CC: , , , , , , , , , , , , , , , , References: <20241213060229.1191037-1-shiyongbang@huawei.com> <046b010c-d40b-42c8-b701-4570b7a536eb@huawei.com> <0e862c4f-a383-4381-8883-3e28d4e254e7@huawei.com> From: Yongbang Shi In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To kwepemd500013.china.huawei.com (7.221.188.12) > On Thu, 2 Jan 2025 at 14:53, Yongbang Shi wrote: >>> On Thu, Dec 19, 2024 at 10:52:32AM +0800, Yongbang Shi wrote: >>>> Does everyone have a question with the patch? >>> -:225: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'reg_value' - possible side-effects? >>> #225: FILE: drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h:23: >>> +#define dp_field_modify(reg_value, mask, val) \ >>> + do { \ >>> + (reg_value) &= ~(mask); \ >>> + (reg_value) |= FIELD_PREP(mask, val); \ >>> + } while (0) \ >>> + >>> >>> -:225: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'mask' - possible side-effects? >>> #225: FILE: drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h:23: >>> +#define dp_field_modify(reg_value, mask, val) \ >>> + do { \ >>> + (reg_value) &= ~(mask); \ >>> + (reg_value) |= FIELD_PREP(mask, val); \ >>> + } while (0) \ >>> + >> Hi Dmitry, >> Thanks for your reminding. For this point, we have to use two variables(mask, reg_value) twice here. >> I tried to another way that static inline functions, but there are build errors. So the only way is like this. >> After reviewed all references, we have a review session about it, and we think it's acceptable and relatively safe. >> (like we won't use varible with post/pre-increment operator) >> If you have a better way to realize this, plz tell me. Appreciate! > Well, there is an obvious way: > > do { > u32 __mask = (mask); > (reg_value) &= ~__mask; > (reg_value) |= FIELD_PREP(__mask, (val)); > } while (0) > > But I don't really think that it makes sense to change. It's a > check-level, not a warning. The next one is important. Hi Dmitry, I tried this way, and there was a build error occured because the parameter of FIELD_PREP() is a variable. https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/ >> >>> -:277: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: baihan li ' != 'Signed-off-by: Baihan Li ' >>> >>> total: 0 errors, 2 warnings, 2 checks, 239 lines checked >>> 1c3faaf4e729 drm/hisilicon/hibmc: add dp link moduel in hibmc drivers >>> -:70: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? >>> #70: >>> new file mode 100644 >>> >>> -:435: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: baihan li ' != 'Signed-off-by: Baihan Li ' >>> >>> total: 0 errors, 2 warnings, 0 checks, 399 lines checked >>> 70bf7b765c82 drm/hisilicon/hibmc: add dp hw moduel in hibmc driver >>> -:28: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? >>> #28: >>> new file mode 100644 >>> >>> -:371: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: baihan li ' != 'Signed-off-by: Baihan Li ' >>> >>> total: 0 errors, 2 warnings, 0 checks, 327 lines checked >>> 24d6be2577d8 drm/hisilicon/hibmc: refactored struct hibmc_drm_private >>> -:188: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: baihan li ' != 'Signed-off-by: Baihan Li ' >>> >>> total: 0 errors, 1 warnings, 0 checks, 151 lines checked >>> 9a74395d14e2 (HEAD -> drm-misc-next) drm/hisilicon/hibmc: add dp module in hibmc >>> -:30: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? >>> #30: >>> new file mode 100644 >>> >>> -:219: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: baihan li ' != 'Signed-off-by: Baihan Li ' >> We already have maintainers for our hibmc-drm module, so there is no need add maintainers in each new file. > Please read carefully. Those warnings are about sign-off mismatches. > You can ignore the maintainers entry. Thanks for your reminder! I will fix it in v9. Baihan Li >> Thanks , >> Baihan Li >> >> >>>>> From: baihan li >>>>> >>>>> Realizing the basic display function of DP cable for DP connector >>>>> displaying. Add DP module in hibmc drm driver, which is for Hisilicon >>>>> Hibmc SoC which used for Out-of-band management. Blow is the general >>>>> hardware connection, both the Hibmc and the host CPU are on the same >>>>> mother board. >>>>> >>>>> +----------+ +----------+ +----- ----+ +----------------+ >>>>> | | PCIe | Hibmc | | | | | >>>>> |host CPU( |<----->| display |<---->| dp kapi |<---->| dp aux moduel | >>>>> |arm64,x86)| |subsystem | | moduel |<---->| dp link moduel | >>>>> +----------+ +----------+ +----------+ +----------------+ >>>>> >>>>> --- >>>>> ChangeLog: >>>>> v7 -> v8: >>>>> - adding DP_AUX_NATIVE_WRITE case in switch statement in hibmc_dp_aux_parse_xfer(). >>>>> - adding pci_set_master() in hibmc_pci_probe(). >>>>> v7:https://lore.kernel.org/all/20241209144840.1933265-1-shiyongbang@huawei.com/ >>>>> v6 -> v7: >>>>> - deleteing unset fields in struct hibmc_link_cap, suggested by Dmitry Baryshkov. >>>>> - using macro instead of constants in hibmc_dp_link_training_configure(), suggested by Dmitry Baryshkov. >>>>> - lowercasing hex, suggested by Dmitry Baryshkov. >>>>> v6:https://lore.kernel.org/all/20241202131322.1847078-1-shiyongbang@huawei.com/ >>>>> v5 -> v6: >>>>> - adding do{} while(0) in macro defination function, suggested by Dmitry Baryshkov. >>>>> - using drm_dbg_dp() to print debug info instead of drm_info(), suggested by Dmitry Baryshkov. >>>>> - adding code comments in hibmc_dp_set_sst(), suggested by Dmitry Baryshkov. >>>>> - adding hpd and get_edid comments in the beginning of patch, suggested by Dmitry Baryshkov. >>>>> v5:https://lore.kernel.org/all/20241118142805.3326443-1-shiyongbang@huawei.com/ >>>>> v4 -> v5: >>>>> - fixing build errors reported by kernel test robot >>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/ >>>>> v4:https://lore.kernel.org/all/20241112132348.2631150-1-shiyongbang@huawei.com/ >>>>> v3 -> v4: >>>>> - retun error codes in result incorrect branch, suggested by Dmitry Baryshkov. >>>>> - replacing all ret= with returns, suggested by Dmitry Baryshkov. >>>>> - moving the comment below the judgment statement, suggested by Dmitry Baryshkov. >>>>> - moving definations to the source file and clearing headers, suggested by Dmitry Baryshkov. >>>>> - reanaming dp_prefix to hibmc_dp_prefix, suggested by Dmitry Baryshkov. >>>>> - changing hibmc_dp_reg_write_field to static inline and lock, suggested by Dmitry Baryshkov. >>>>> - moving some structs to later patch, suggested by Dmitry Baryshkov. >>>>> - optimizing hibmc_dp_link_get_adjust_train() to delete for loop, suggested by Dmitry Baryshkov. >>>>> - changing ELNRNG to EIO error code, suggested by Dmitry Baryshkov. >>>>> - deleting meaningless macro, suggested by Dmitry Baryshkov. >>>>> - fixing build errors reported by kernel test robot >>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202411041559.WIfxRN6n-lkp@intel.com/ >>>>> - changed the type of train_set to array, suggested by Dmitry Baryshkov. >>>>> - using actual link rate instead of magic num, suggested by Dmitry Baryshkov. >>>>> - deleting hibmc_dp_hw_uninit(), suggested by Dmitry Baryshkov. >>>>> - separating hibmc_vdac and hibmc_dp changes into separate patche, suggested by Dmitry Baryshkov. >>>>> - static int hibmc_dp_prepare(), suggested by Dmitry Baryshkov. >>>>> v3:https://lore.kernel.org/all/20241101105028.2177274-1-shiyongbang@huawei.com/ >>>>> v2 -> v3: >>>>> - put the macro definations in latter patch where they are actually used, suggested by Dmitry Baryshkov. >>>>> - rename some macro definations to make them sensible, suggested by Dmitry Baryshkov. >>>>> - using FIELD_PREP and FIELD_GET, suggested by Dmitry Baryshkov. >>>>> - using DP_DPCD_REV_foo, suggested by Dmitry Baryshkov. >>>>> - using switchcase in dp_link_reduce_lane, suggested by Dmitry Baryshkov. >>>>> - deleting dp_link_pattern2dpcd function and using macros directly, suggested by Dmitry Baryshkov. >>>>> - deleting EFAULT error codes, suggested by Dmitry Baryshkov. >>>>> - fix build errors reported by kernel test robot >>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202410250305.UHKDhtxy-lkp@intel.com/ >>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202410250931.UDQ9s66H-lkp@intel.com/ >>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202410251136.1m7BlR68-lkp@intel.com/ >>>>> v2:https://lore.kernel.org/all/20241022124148.1952761-1-shiyongbang@huawei.com/ >>>>> v1 -> v2: >>>>> - using drm_dp_aux frame implement dp aux read and write functions, suggested by Jani Nikula. >>>>> - using drm dp header files' dp macros instead, suggested by Andy Yan. >>>>> - using drm_dp_* functions implement dp link training process, suggested by Jani Nikula. >>>>> - changed some defines and functions to former patch, suggested by Dmitry Baryshkov. >>>>> - sorting the headers including in dp_hw.h and hibmc_drm_drv.c files, suggested by Dmitry Baryshkov. >>>>> - deleting struct dp_mode and dp_mode_cfg function, suggested by Dmitry Baryshkov. >>>>> - modifying drm_simple_encoder_init function, suggested by Dmitry Baryshkov. >>>>> - refactoring struct hibmc_connector, suggested by Dmitry Baryshkov. >>>>> - withdrawing the modification in hibmc_kms_init, suggested by Dmitry Baryshkov. >>>>> - fix build errors reported by kernel test robot >>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202410031735.8iRZZR6T-lkp@intel.com/ >>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202410040328.VeVxM9yB-lkp@intel.com/ >>>>> v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/ >>>>> --- >>>>> >>>>> baihan li (5): >>>>> drm/hisilicon/hibmc: add dp aux in hibmc drivers >>>>> drm/hisilicon/hibmc: add dp link moduel in hibmc drivers >>>>> drm/hisilicon/hibmc: add dp hw moduel in hibmc driver >>>>> drm/hisilicon/hibmc: refactored struct hibmc_drm_private >>>>> drm/hisilicon/hibmc: add dp module in hibmc >>>>> >>>>> drivers/gpu/drm/hisilicon/hibmc/Makefile | 3 +- >>>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c | 164 +++++++++ >>>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h | 63 ++++ >>>>> .../gpu/drm/hisilicon/hibmc/dp/dp_config.h | 19 + >>>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 220 ++++++++++++ >>>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 28 ++ >>>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c | 332 ++++++++++++++++++ >>>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h | 76 ++++ >>>>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 118 +++++++ >>>>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 14 + >>>>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 19 +- >>>>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c | 41 ++- >>>>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 20 +- >>>>> 13 files changed, 1078 insertions(+), 39 deletions(-) >>>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >>>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h >>>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h >>>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c >>>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h >>>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c >>>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h >>>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>>>> > >