Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
Hi Sean, On Mon, Mar 18, 2024 at 03:05:57PM -0400, Sean Anderson wrote: > On 3/18/24 13:50, Laurent Pinchart wrote: > > On Mon, Mar 18, 2024 at 11:06:40AM -0400, Sean Anderson wrote: > >> On 3/16/24 06:14, kernel test robot wrote: > >> > Hi Sean, > >> > > >> > kernel test robot noticed the following build warnings: > >> > > >> > [auto build test WARNING on v6.8] > >> > [cannot apply to drm-misc/drm-misc-next linus/master next-20240315] > >> > [If your patch is applied to the wrong git tree, kindly drop us a note. > >> > And when submitting patch, we suggest to use '--base' as documented in > >> > https://git-scm.com/docs/git-format-patch#_base_tree_information] > >> > > >> > url: > >> > https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208 > >> > base: v6.8 > >> > patch link: > >> > https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev > >> > patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for > >> > compliance testing > >> > config: microblaze-allmodconfig > >> > (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/config) > >> > compiler: microblaze-linux-gcc (GCC) 13.2.0 > >> > reproduce (this is a W=1 build): > >> > (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/reproduce) [...] > >> > > >> > If you fix the issue in a separate patch/commit (i.e. not just a new > >> > version of > >> > the same patch/commit), kindly add following tags > >> > | Reported-by: kernel test robot > >> > | Closes: > >> > https://lore.kernel.org/oe-kbuild-all/202403161704.achjdsjg-...@intel.com/ > >> > > >> > All warnings (new ones prefixed by >>): > >> > > >> >drivers/gpu/drm/xlnx/zynqmp_dp.c: In function > >> > 'zynqmp_dp_bridge_debugfs_init': > >> >>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write > >> >>> a terminating nul past the end of the destination [-Wformat-overflow=] > >> > 2168 | sprintf(name, fmt, i); > >> > | ^~~ > >> >drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output > >> > between 18 and 20 bytes into a destination of size 19 > >> > 2168 | sprintf(name, fmt, i); > >> > | ^ > >> > >> Not a bug, as i will be at most 4, which uses 1 digit. > > > > The compiler can't know that. Please fix this, there's a zero warning > > policy. > > I cannot reproduce this with GCC 13.2.0. So given that this is not a bug and > I can't reproduce > it, I don't see how I can verify any fix. There is a "reproduce" link in the bot's report. We followed the steps in that link and found that the warning could be reproduced. Please notice that this is a "W=1" warning. BTW, we also tested the v2 patch and the warning has been fixed there. Just for your information. $ cd linux $ git checkout v6.8 HEAD is now at e8f897f4afef0 Linux 6.8 $ b4 am https://lore.kernel.org/r/20240315230916.1759060-7-sean.ander...@linux.dev $ git am ./20240315_sean_anderson_drm_zynqmp_dp_misc_patches_and_debugfs_support.mbx Applying: drm: zynqmp_dp: Downgrade log level for aux retries message Applying: drm: zynqmp_dp: Adjust training values per-lane Applying: drm: zynqmp_dp: Add locking Applying: drm: zynqmp_dp: Split off several helper functions Applying: drm: zynqmp_dp: Optionally ignore DPCD errors Applying: drm: zynqmp_dp: Add debugfs interface for compliance testing $ wget https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/config $ mkdir build_dir $ cp config build_dir/.config $ git clone https://github.com/intel/lkp-tests.git ~/lkp-tests $ COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-13.2.0 ~/lkp-tests/kbuild/make.cross W=1 O=build_dir ARCH=microblaze olddefconfig $ COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-13.2.0 ~/lkp-tests/kbuild/make.cross W=1 O=build_dir ARCH=microblaze drivers/gpu/drm/xlnx/zynqmp_dp.o ... CC [M] drivers/gpu/drm/xlnx/zynqmp_dp.o ../drivers/gpu/drm/xlnx/zynqmp_dp.c: In function 'zynqmp_dp_bridge_debugfs_init': ../drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=] 2168 | s
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
On 3/18/24 13:50, Laurent Pinchart wrote: > On Mon, Mar 18, 2024 at 11:06:40AM -0400, Sean Anderson wrote: >> On 3/16/24 06:14, kernel test robot wrote: >> > Hi Sean, >> > >> > kernel test robot noticed the following build warnings: >> > >> > [auto build test WARNING on v6.8] >> > [cannot apply to drm-misc/drm-misc-next linus/master next-20240315] >> > [If your patch is applied to the wrong git tree, kindly drop us a note. >> > And when submitting patch, we suggest to use '--base' as documented in >> > https://git-scm.com/docs/git-format-patch#_base_tree_information] >> > >> > url: >> > https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208 >> > base: v6.8 >> > patch link: >> > https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev >> > patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for >> > compliance testing >> > config: microblaze-allmodconfig >> > (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/config) >> > compiler: microblaze-linux-gcc (GCC) 13.2.0 >> > reproduce (this is a W=1 build): >> > (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/reproduce) >> > >> > If you fix the issue in a separate patch/commit (i.e. not just a new >> > version of >> > the same patch/commit), kindly add following tags >> > | Reported-by: kernel test robot >> > | Closes: >> > https://lore.kernel.org/oe-kbuild-all/202403161704.achjdsjg-...@intel.com/ >> > >> > All warnings (new ones prefixed by >>): >> > >> >drivers/gpu/drm/xlnx/zynqmp_dp.c: In function >> > 'zynqmp_dp_bridge_debugfs_init': >> >>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a >> >>> terminating nul past the end of the destination [-Wformat-overflow=] >> > 2168 | sprintf(name, fmt, i); >> > | ^~~ >> >drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output >> > between 18 and 20 bytes into a destination of size 19 >> > 2168 | sprintf(name, fmt, i); >> > | ^ >> >> Not a bug, as i will be at most 4, which uses 1 digit. > > The compiler can't know that. Please fix this, there's a zero warning > policy. I cannot reproduce this with GCC 13.2.0. So given that this is not a bug and I can't reproduce it, I don't see how I can verify any fix. --Sean >> > vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c >> > >> > 2136 >> > 2137 DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, >> > zynqmp_dp_rate_get, >> > 2138 zynqmp_dp_rate_set, "%llu\n"); >> > 2139 >> > 2140 static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge >> > *bridge, >> > 2141 struct dentry *root) >> > 2142 { >> > 2143 struct zynqmp_dp *dp = bridge_to_dp(bridge); >> > 2144 struct dentry *test; >> > 2145 int i; >> > 2146 >> > 2147 dp->test.bw_code = DP_LINK_BW_5_4; >> > 2148 dp->test.link_cnt = dp->num_lanes; >> > 2149 >> > 2150 test = debugfs_create_dir("test", root); >> > 2151 #define CREATE_FILE(name) \ >> > 2152 debugfs_create_file(#name, 0600, test, dp, >> > _zynqmp_dp_##name) >> > 2153 CREATE_FILE(pattern); >> > 2154 CREATE_FILE(enhanced); >> > 2155 CREATE_FILE(downspread); >> > 2156 CREATE_FILE(active); >> > 2157 CREATE_FILE(custom); >> > 2158 CREATE_FILE(rate); >> > 2159 CREATE_FILE(lanes); >> > 2160 >> > 2161 for (i = 0; i < dp->num_lanes; i++) { >> > 2162 static const char fmt[] = "lane%d_preemphasis"; >> > 2163 char name[sizeof(fmt)]; >> > 2164 >> > 2165 dp->debugfs_train_set[i].dp = dp; >> > 2166 dp->debugfs_train_set[i].lane = i; >> > 2167 >> >> 2168 sprintf(name, fmt, i); >> > 2169 debugfs_create_file(name, 0600, test, >> > 2170 >debugfs_train_set[i], >> > 2171 >> > _zynqmp_dp_preemphasis); >> > 2172 >> > 2173 sprintf(name, "lane%d_swing", i); >> > 2174 debugfs_create_file(name, 0600, test, >> > 2175 >debugfs_train_set[i], >> > 2176 _zynqmp_dp_swing); >> > 2177 } >> > 2178 } >> > 2179 >
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
On Sat, Mar 16, 2024 at 07:56:52PM +0200, Dmitry Baryshkov wrote: > On Sat, 16 Mar 2024 at 01:09, Sean Anderson wrote: > > > > Add a debugfs interface for exercising the various test modes supported > > by the DisplayPort controller. This allows performing compliance > > testing, or performing signal integrity measurements on a failing link. > > At the moment, we do not support sink-driven link quality testing, > > although such support would be fairly easy to add. > > Could you please point out how this is used for compliance testing? We > have been using the msm_dp_compliance tool [1]. > > I think it would be nice to rework our drivers towards a common > debugfs interface used for DP connectors, maybe defining generic > internal interface/helpers like Maxime is implementing for HDMI > connectors. This would be really nice :-) > [1] > https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tools/msm_dp_compliance.c?ref_type=heads > > > > > Signed-off-by: Sean Anderson > > --- > > > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 591 ++- > > 1 file changed, 590 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > index 040f7b88ee51..57032186e1ca 100644 > > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > @@ -18,7 +18,9 @@ > > #include > > #include > > > > +#include > > #include > > +#include > > #include > > #include > > #include > > @@ -50,6 +52,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in > > msec (default: 4)"); > > #define ZYNQMP_DP_LANE_COUNT_SET 0x4 > > #define ZYNQMP_DP_ENHANCED_FRAME_EN0x8 > > #define ZYNQMP_DP_TRAINING_PATTERN_SET 0xc > > +#define ZYNQMP_DP_LINK_QUAL_PATTERN_SET0x10 > > #define ZYNQMP_DP_SCRAMBLING_DISABLE 0x14 > > #define ZYNQMP_DP_DOWNSPREAD_CTL 0x18 > > #define ZYNQMP_DP_SOFTWARE_RESET 0x1c > > @@ -63,6 +66,9 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in > > msec (default: 4)"); > > > > ZYNQMP_DP_SOFTWARE_RESET_STREAM3 | \ > > > > ZYNQMP_DP_SOFTWARE_RESET_STREAM4 | \ > > > > ZYNQMP_DP_SOFTWARE_RESET_AUX) > > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_1 0x20 > > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_2 0x24 > > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_3 0x28 > > > > /* Core enable registers */ > > #define ZYNQMP_DP_TRANSMITTER_ENABLE 0x80 > > @@ -206,6 +212,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay > > in msec (default: 4)"); > > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_2 BIT(2) > > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_3 BIT(3) > > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_ALL0xf > > +#define ZYNQMP_DP_TRANSMIT_PRBS7 0x230 > > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_0 0x23c > > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_1 0x240 > > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_2 0x244 > > @@ -273,6 +280,69 @@ struct zynqmp_dp_config { > > u8 bpp; > > }; > > > > +/** > > + * enum test_pattern - Test patterns for test testing > > + * TEST_VIDEO: Use regular video input > > + * TEST_SYMBOL_ERROR: Symbol error measurement pattern > > + * TEST_PRBS7: Output of the PRBS7 (x^7 + x^6 + 1) polynomial > > + * TEST_80BIT_CUSTOM: A custom 80-bit pattern > > + * TEST_CP2520: HBR2 compliance eye pattern > > + * TEST_TPS1: Link training symbol pattern TPS1 (/D10.2/) > > + * TEST_TPS2: Link training symbol pattern TPS2 > > + * TEST_TPS3: Link training symbol pattern TPS3 (for HBR2) > > + */ > > +enum test_pattern { > > + TEST_VIDEO, > > + TEST_TPS1, > > + TEST_TPS2, > > + TEST_TPS3, > > + TEST_SYMBOL_ERROR, > > + TEST_PRBS7, > > + TEST_80BIT_CUSTOM, > > + TEST_CP2520, > > +}; > > + > > +static const char *const test_pattern_str[] = { > > + [TEST_VIDEO] = "video", > > + [TEST_TPS1] = "tps1", > > + [TEST_TPS2] = "tps2", > > + [TEST_TPS3] = "tps3", > > + [TEST_SYMBOL_ERROR] = "symbol-error", > > + [TEST_PRBS7] = "prbs7", > > + [TEST_80BIT_CUSTOM] = "80bit-custom", > > + [TEST_CP2520] = "cp2520", > > +}; > > + > > +/** > > + * struct zynqmp_dp_test - Configuration for test mode > > + * @pattern: The test pattern > > + * @enhanced: Use enhanced framing > > + * @downspread: Use SSC > > + * @active: Whether test mode is active > > + * @custom: Custom pattern for %TEST_80BIT_CUSTOM > > + * @train_set: Voltage/preemphasis settings > > + * @bw_code: Bandwidth code for the link > > + * @link_cnt: Number of lanes >
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
On Mon, Mar 18, 2024 at 11:06:40AM -0400, Sean Anderson wrote: > On 3/16/24 06:14, kernel test robot wrote: > > Hi Sean, > > > > kernel test robot noticed the following build warnings: > > > > [auto build test WARNING on v6.8] > > [cannot apply to drm-misc/drm-misc-next linus/master next-20240315] > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > And when submitting patch, we suggest to use '--base' as documented in > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > > > url: > > https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208 > > base: v6.8 > > patch link: > > https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev > > patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for > > compliance testing > > config: microblaze-allmodconfig > > (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/config) > > compiler: microblaze-linux-gcc (GCC) 13.2.0 > > reproduce (this is a W=1 build): > > (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/reproduce) > > > > If you fix the issue in a separate patch/commit (i.e. not just a new > > version of > > the same patch/commit), kindly add following tags > > | Reported-by: kernel test robot > > | Closes: > > https://lore.kernel.org/oe-kbuild-all/202403161704.achjdsjg-...@intel.com/ > > > > All warnings (new ones prefixed by >>): > > > >drivers/gpu/drm/xlnx/zynqmp_dp.c: In function > > 'zynqmp_dp_bridge_debugfs_init': > >>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a > >>> terminating nul past the end of the destination [-Wformat-overflow=] > > 2168 | sprintf(name, fmt, i); > > | ^~~ > >drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between > > 18 and 20 bytes into a destination of size 19 > > 2168 | sprintf(name, fmt, i); > > | ^ > > Not a bug, as i will be at most 4, which uses 1 digit. The compiler can't know that. Please fix this, there's a zero warning policy. > > vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c > > > > 2136 > > 2137 DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, > > zynqmp_dp_rate_get, > > 2138 zynqmp_dp_rate_set, "%llu\n"); > > 2139 > > 2140 static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge > > *bridge, > > 2141struct dentry *root) > > 2142 { > > 2143 struct zynqmp_dp *dp = bridge_to_dp(bridge); > > 2144 struct dentry *test; > > 2145 int i; > > 2146 > > 2147 dp->test.bw_code = DP_LINK_BW_5_4; > > 2148 dp->test.link_cnt = dp->num_lanes; > > 2149 > > 2150 test = debugfs_create_dir("test", root); > > 2151 #define CREATE_FILE(name) \ > > 2152 debugfs_create_file(#name, 0600, test, dp, > > _zynqmp_dp_##name) > > 2153 CREATE_FILE(pattern); > > 2154 CREATE_FILE(enhanced); > > 2155 CREATE_FILE(downspread); > > 2156 CREATE_FILE(active); > > 2157 CREATE_FILE(custom); > > 2158 CREATE_FILE(rate); > > 2159 CREATE_FILE(lanes); > > 2160 > > 2161 for (i = 0; i < dp->num_lanes; i++) { > > 2162 static const char fmt[] = "lane%d_preemphasis"; > > 2163 char name[sizeof(fmt)]; > > 2164 > > 2165 dp->debugfs_train_set[i].dp = dp; > > 2166 dp->debugfs_train_set[i].lane = i; > > 2167 > >> 2168 sprintf(name, fmt, i); > > 2169 debugfs_create_file(name, 0600, test, > > 2170 >debugfs_train_set[i], > > 2171 > > _zynqmp_dp_preemphasis); > > 2172 > > 2173 sprintf(name, "lane%d_swing", i); > > 2174 debugfs_create_file(name, 0600, test, > > 2175 >debugfs_train_set[i], > > 2176 _zynqmp_dp_swing); > > 2177 } > > 2178 } > > 2179 -- Regards, Laurent Pinchart
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
On 3/16/24 13:56, Dmitry Baryshkov wrote: > On Sat, 16 Mar 2024 at 01:09, Sean Anderson wrote: >> >> Add a debugfs interface for exercising the various test modes supported >> by the DisplayPort controller. This allows performing compliance >> testing, or performing signal integrity measurements on a failing link. >> At the moment, we do not support sink-driven link quality testing, >> although such support would be fairly easy to add. > > Could you please point out how this is used for compliance testing? We > have been using the msm_dp_compliance tool [1]. Here's some quick documentation I wrote up. This probably could be put under Documentation for v2. The following files in /sys/kernel/debug/dri/1/DP-1/test/ control the DisplayPort test modes: active: Writing a 1 to this file will activate test mode, and writing a 0 will deactivate test mode. Writing a 1 or 0 when the test mode is already active/inactive will reactivate/re-deactivate test mode. When test mode is inactive, changes made to other files will have no effect. When test mode is active, changes made to other files will apply instantly. Additionally, hotplug events (as removing the cable or if the monitor requests link retraining) are ignored. custom: Custom test pattern value downspread: Enable/disable clock downspreading (spread-spectrum clocking) by writing 1/0 enhanced: Enable/disable enhanced framing lane0_preemphasis: Preemphasis from 0 (lowest) to 2 (most) for lane 0 lane0_swing: Voltage swing from 0 (lowest) to 3 (most) for lane 0 lane1_preemphasis: Preemphasis from 0 (lowest) to 2 (most) for lane 1 lane1_swing: Voltage swing from 0 (lowest) to 3 (most) for lane 1 lanes: Number of lanes to use (1 or 2) pattern: Test pattern. May be one of: - video: Use regular video input - symbol-error: Symbol error measurement pattern - prbs7: Output of the PRBS7 (x^7 + x^6 + 1) polynomial - 80bit-custom: A custom 80-bit pattern - cp2520: HBR2 compliance eye pattern - tps1: Link training symbol pattern TPS1 (/D10.2/) - tps2: Link training symbol pattern TPS2 - tps3: Link training symbol pattern TPS3 (for HBR2) rate: Rate in hertz. One of - 54: HBR2 - 27: HBR - 162000: RBR You can dump the displayport test settings with the following command: for prop in /sys/kernel/debug/dri/1/DP-1/test/*; do printf '%-20s ' ${prop##*/} if [ ${prop##*/} = custom ]; then hexdump -C $prop | head -1 else cat $prop fi done The output could look something like active 1 custom 00 00 00 00 00 00 00 00 00 00 |..| downspread 0 enhanced 1 lane0_preemphasis0 lane0_swing 3 lane1_preemphasis0 lane1_swing 3 lanes2 pattern prbs7 rate 162000 The recommended test procedure is to connect the board to a monitor, configure test mode, activate test mode, and then disconnect the cable and connect it to your test equipment of choice. For example, one sequence of commands could be: echo 1 > /sys/kernel/debug/dri/1/DP-1/test/enhanced echo tps1 > /sys/kernel/debug/dri/1/DP-1/test/pattern echo 162000 > /sys/kernel/debug/dri/1/DP-1/test/rate echo 1 > /sys/kernel/debug/dri/1/DP-1/test/active at which point the cable could be disconnected from the monitor. When the cable is disconnected there will be several errors while changing the settings. This is expected. > I think it would be nice to rework our drivers towards a common > debugfs interface used for DP connectors, maybe defining generic > internal interface/helpers like Maxime is implementing for HDMI > connectors. > > [1] > https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tools/msm_dp_compliance.c?ref_type=heads I was definitely inspired by the msm, intel, and amd approaches. However, those debugfs implementations seem to be oriented towards DisplayPort text fixtures which emulate DPRXs. In particular, both the intel and msm debugfs interfaces provide no method for configuring test parameters in userspace. As test fixtures supporting DPCD can run into the thousands of dollars, I think it is more economical to support userspace-driven testing. I was particularly inspired by the AMD approach: /* Usage: set DP physical test pattern using debugfs with normal DP * panel. Then plug out DP panel and connect a scope to measure * For normal video mode and test pattern generated from CRCT, * they are visibile to user. So do not disable HPD. * Video Mode is also set to clear the test pattern, so enable HPD * because it might have been disabled after a test pattern was set. * AUX depends on HPD * sequence dependent, do not move! */ But I chose to always disable HPD events and ignore DPCD errors in test mode. I think this is
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
On 3/16/24 06:55, kernel test robot wrote: > Hi Sean, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on v6.8] > [cannot apply to drm-misc/drm-misc-next linus/master next-20240315] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208 > base: v6.8 > patch link: > https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev > patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for > compliance testing > config: m68k-allyesconfig > (https://download.01.org/0day-ci/archive/20240316/202403161837.76okcezn-...@intel.com/config) > compiler: m68k-linux-gcc (GCC) 13.2.0 > reproduce (this is a W=1 build): > (https://download.01.org/0day-ci/archive/20240316/202403161837.76okcezn-...@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version > of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot > | Closes: > https://lore.kernel.org/oe-kbuild-all/202403161837.76okcezn-...@intel.com/ > > All errors (new ones prefixed by >>): > >m68k-linux-ld: drivers/gpu/drm/xlnx/zynqmp_dp.o: in function > `zynqmp_dp_rate_set': >>> zynqmp_dp.c:(.text+0x1a7a): undefined reference to `__udivdi3' > Will fix. --Sean
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
On 3/16/24 06:14, kernel test robot wrote: > Hi Sean, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on v6.8] > [cannot apply to drm-misc/drm-misc-next linus/master next-20240315] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208 > base: v6.8 > patch link: > https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev > patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for > compliance testing > config: microblaze-allmodconfig > (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/config) > compiler: microblaze-linux-gcc (GCC) 13.2.0 > reproduce (this is a W=1 build): > (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version > of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot > | Closes: > https://lore.kernel.org/oe-kbuild-all/202403161704.achjdsjg-...@intel.com/ > > All warnings (new ones prefixed by >>): > >drivers/gpu/drm/xlnx/zynqmp_dp.c: In function > 'zynqmp_dp_bridge_debugfs_init': >>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a >>> terminating nul past the end of the destination [-Wformat-overflow=] > 2168 | sprintf(name, fmt, i); > | ^~~ >drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between > 18 and 20 bytes into a destination of size 19 > 2168 | sprintf(name, fmt, i); > | ^ Not a bug, as i will be at most 4, which uses 1 digit. --Sean > vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c > > 2136 > 2137DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, > zynqmp_dp_rate_get, > 2138 zynqmp_dp_rate_set, "%llu\n"); > 2139 > 2140static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge > *bridge, > 2141 struct dentry *root) > 2142{ > 2143struct zynqmp_dp *dp = bridge_to_dp(bridge); > 2144struct dentry *test; > 2145int i; > 2146 > 2147dp->test.bw_code = DP_LINK_BW_5_4; > 2148dp->test.link_cnt = dp->num_lanes; > 2149 > 2150test = debugfs_create_dir("test", root); > 2151#define CREATE_FILE(name) \ > 2152debugfs_create_file(#name, 0600, test, dp, > _zynqmp_dp_##name) > 2153CREATE_FILE(pattern); > 2154CREATE_FILE(enhanced); > 2155CREATE_FILE(downspread); > 2156CREATE_FILE(active); > 2157CREATE_FILE(custom); > 2158CREATE_FILE(rate); > 2159CREATE_FILE(lanes); > 2160 > 2161for (i = 0; i < dp->num_lanes; i++) { > 2162static const char fmt[] = "lane%d_preemphasis"; > 2163char name[sizeof(fmt)]; > 2164 > 2165dp->debugfs_train_set[i].dp = dp; > 2166dp->debugfs_train_set[i].lane = i; > 2167 >> 2168 sprintf(name, fmt, i); > 2169debugfs_create_file(name, 0600, test, > 2170>debugfs_train_set[i], > 2171 > _zynqmp_dp_preemphasis); > 2172 > 2173sprintf(name, "lane%d_swing", i); > 2174debugfs_create_file(name, 0600, test, > 2175>debugfs_train_set[i], > 2176_zynqmp_dp_swing); > 2177} > 2178} > 2179 >
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
On Sat, 16 Mar 2024 at 01:09, Sean Anderson wrote: > > Add a debugfs interface for exercising the various test modes supported > by the DisplayPort controller. This allows performing compliance > testing, or performing signal integrity measurements on a failing link. > At the moment, we do not support sink-driven link quality testing, > although such support would be fairly easy to add. Could you please point out how this is used for compliance testing? We have been using the msm_dp_compliance tool [1]. I think it would be nice to rework our drivers towards a common debugfs interface used for DP connectors, maybe defining generic internal interface/helpers like Maxime is implementing for HDMI connectors. [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tools/msm_dp_compliance.c?ref_type=heads > > Signed-off-by: Sean Anderson > --- > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 591 ++- > 1 file changed, 590 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c > b/drivers/gpu/drm/xlnx/zynqmp_dp.c > index 040f7b88ee51..57032186e1ca 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > @@ -18,7 +18,9 @@ > #include > #include > > +#include > #include > +#include > #include > #include > #include > @@ -50,6 +52,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in > msec (default: 4)"); > #define ZYNQMP_DP_LANE_COUNT_SET 0x4 > #define ZYNQMP_DP_ENHANCED_FRAME_EN0x8 > #define ZYNQMP_DP_TRAINING_PATTERN_SET 0xc > +#define ZYNQMP_DP_LINK_QUAL_PATTERN_SET0x10 > #define ZYNQMP_DP_SCRAMBLING_DISABLE 0x14 > #define ZYNQMP_DP_DOWNSPREAD_CTL 0x18 > #define ZYNQMP_DP_SOFTWARE_RESET 0x1c > @@ -63,6 +66,9 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in > msec (default: 4)"); > > ZYNQMP_DP_SOFTWARE_RESET_STREAM3 | \ > > ZYNQMP_DP_SOFTWARE_RESET_STREAM4 | \ > > ZYNQMP_DP_SOFTWARE_RESET_AUX) > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_1 0x20 > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_2 0x24 > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_3 0x28 > > /* Core enable registers */ > #define ZYNQMP_DP_TRANSMITTER_ENABLE 0x80 > @@ -206,6 +212,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in > msec (default: 4)"); > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_2 BIT(2) > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_3 BIT(3) > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_ALL0xf > +#define ZYNQMP_DP_TRANSMIT_PRBS7 0x230 > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_0 0x23c > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_1 0x240 > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_2 0x244 > @@ -273,6 +280,69 @@ struct zynqmp_dp_config { > u8 bpp; > }; > > +/** > + * enum test_pattern - Test patterns for test testing > + * TEST_VIDEO: Use regular video input > + * TEST_SYMBOL_ERROR: Symbol error measurement pattern > + * TEST_PRBS7: Output of the PRBS7 (x^7 + x^6 + 1) polynomial > + * TEST_80BIT_CUSTOM: A custom 80-bit pattern > + * TEST_CP2520: HBR2 compliance eye pattern > + * TEST_TPS1: Link training symbol pattern TPS1 (/D10.2/) > + * TEST_TPS2: Link training symbol pattern TPS2 > + * TEST_TPS3: Link training symbol pattern TPS3 (for HBR2) > + */ > +enum test_pattern { > + TEST_VIDEO, > + TEST_TPS1, > + TEST_TPS2, > + TEST_TPS3, > + TEST_SYMBOL_ERROR, > + TEST_PRBS7, > + TEST_80BIT_CUSTOM, > + TEST_CP2520, > +}; > + > +static const char *const test_pattern_str[] = { > + [TEST_VIDEO] = "video", > + [TEST_TPS1] = "tps1", > + [TEST_TPS2] = "tps2", > + [TEST_TPS3] = "tps3", > + [TEST_SYMBOL_ERROR] = "symbol-error", > + [TEST_PRBS7] = "prbs7", > + [TEST_80BIT_CUSTOM] = "80bit-custom", > + [TEST_CP2520] = "cp2520", > +}; > + > +/** > + * struct zynqmp_dp_test - Configuration for test mode > + * @pattern: The test pattern > + * @enhanced: Use enhanced framing > + * @downspread: Use SSC > + * @active: Whether test mode is active > + * @custom: Custom pattern for %TEST_80BIT_CUSTOM > + * @train_set: Voltage/preemphasis settings > + * @bw_code: Bandwidth code for the link > + * @link_cnt: Number of lanes > + */ > +struct zynqmp_dp_test { > + enum test_pattern pattern; > + bool enhanced, downspread, active; > + u8 custom[10]; > + u8 train_set[ZYNQMP_DP_MAX_LANES]; > + u8 bw_code; > + u8 link_cnt; > +}; > + > +/** > + * struct zynqmp_dp_train_set_priv - Private data for train_set debugfs files > + *
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
Hi Sean, kernel test robot noticed the following build errors: [auto build test ERROR on v6.8] [cannot apply to drm-misc/drm-misc-next linus/master next-20240315] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208 base: v6.8 patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20240316/202403161837.76okcezn-...@intel.com/config) compiler: m68k-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403161837.76okcezn-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202403161837.76okcezn-...@intel.com/ All errors (new ones prefixed by >>): m68k-linux-ld: drivers/gpu/drm/xlnx/zynqmp_dp.o: in function `zynqmp_dp_rate_set': >> zynqmp_dp.c:(.text+0x1a7a): undefined reference to `__udivdi3' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
Hi Sean, kernel test robot noticed the following build warnings: [auto build test WARNING on v6.8] [cannot apply to drm-misc/drm-misc-next linus/master next-20240315] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208 base: v6.8 patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing config: microblaze-allmodconfig (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/config) compiler: microblaze-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403161704.achjdsjg-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202403161704.achjdsjg-...@intel.com/ All warnings (new ones prefixed by >>): drivers/gpu/drm/xlnx/zynqmp_dp.c: In function 'zynqmp_dp_bridge_debugfs_init': >> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a >> terminating nul past the end of the destination [-Wformat-overflow=] 2168 | sprintf(name, fmt, i); | ^~~ drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between 18 and 20 bytes into a destination of size 19 2168 | sprintf(name, fmt, i); | ^ vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c 2136 2137 DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get, 2138 zynqmp_dp_rate_set, "%llu\n"); 2139 2140 static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge, 2141struct dentry *root) 2142 { 2143 struct zynqmp_dp *dp = bridge_to_dp(bridge); 2144 struct dentry *test; 2145 int i; 2146 2147 dp->test.bw_code = DP_LINK_BW_5_4; 2148 dp->test.link_cnt = dp->num_lanes; 2149 2150 test = debugfs_create_dir("test", root); 2151 #define CREATE_FILE(name) \ 2152 debugfs_create_file(#name, 0600, test, dp, _zynqmp_dp_##name) 2153 CREATE_FILE(pattern); 2154 CREATE_FILE(enhanced); 2155 CREATE_FILE(downspread); 2156 CREATE_FILE(active); 2157 CREATE_FILE(custom); 2158 CREATE_FILE(rate); 2159 CREATE_FILE(lanes); 2160 2161 for (i = 0; i < dp->num_lanes; i++) { 2162 static const char fmt[] = "lane%d_preemphasis"; 2163 char name[sizeof(fmt)]; 2164 2165 dp->debugfs_train_set[i].dp = dp; 2166 dp->debugfs_train_set[i].lane = i; 2167 > 2168 sprintf(name, fmt, i); 2169 debugfs_create_file(name, 0600, test, 2170 >debugfs_train_set[i], 2171 _zynqmp_dp_preemphasis); 2172 2173 sprintf(name, "lane%d_swing", i); 2174 debugfs_create_file(name, 0600, test, 2175 >debugfs_train_set[i], 2176 _zynqmp_dp_swing); 2177 } 2178 } 2179 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
Add a debugfs interface for exercising the various test modes supported by the DisplayPort controller. This allows performing compliance testing, or performing signal integrity measurements on a failing link. At the moment, we do not support sink-driven link quality testing, although such support would be fairly easy to add. Signed-off-by: Sean Anderson --- drivers/gpu/drm/xlnx/zynqmp_dp.c | 591 ++- 1 file changed, 590 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c index 040f7b88ee51..57032186e1ca 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c @@ -18,7 +18,9 @@ #include #include +#include #include +#include #include #include #include @@ -50,6 +52,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)"); #define ZYNQMP_DP_LANE_COUNT_SET 0x4 #define ZYNQMP_DP_ENHANCED_FRAME_EN0x8 #define ZYNQMP_DP_TRAINING_PATTERN_SET 0xc +#define ZYNQMP_DP_LINK_QUAL_PATTERN_SET0x10 #define ZYNQMP_DP_SCRAMBLING_DISABLE 0x14 #define ZYNQMP_DP_DOWNSPREAD_CTL 0x18 #define ZYNQMP_DP_SOFTWARE_RESET 0x1c @@ -63,6 +66,9 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)"); ZYNQMP_DP_SOFTWARE_RESET_STREAM3 | \ ZYNQMP_DP_SOFTWARE_RESET_STREAM4 | \ ZYNQMP_DP_SOFTWARE_RESET_AUX) +#define ZYNQMP_DP_COMP_PATTERN_80BIT_1 0x20 +#define ZYNQMP_DP_COMP_PATTERN_80BIT_2 0x24 +#define ZYNQMP_DP_COMP_PATTERN_80BIT_3 0x28 /* Core enable registers */ #define ZYNQMP_DP_TRANSMITTER_ENABLE 0x80 @@ -206,6 +212,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)"); #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_2 BIT(2) #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_3 BIT(3) #define ZYNQMP_DP_TX_PHY_POWER_DOWN_ALL0xf +#define ZYNQMP_DP_TRANSMIT_PRBS7 0x230 #define ZYNQMP_DP_PHY_PRECURSOR_LANE_0 0x23c #define ZYNQMP_DP_PHY_PRECURSOR_LANE_1 0x240 #define ZYNQMP_DP_PHY_PRECURSOR_LANE_2 0x244 @@ -273,6 +280,69 @@ struct zynqmp_dp_config { u8 bpp; }; +/** + * enum test_pattern - Test patterns for test testing + * TEST_VIDEO: Use regular video input + * TEST_SYMBOL_ERROR: Symbol error measurement pattern + * TEST_PRBS7: Output of the PRBS7 (x^7 + x^6 + 1) polynomial + * TEST_80BIT_CUSTOM: A custom 80-bit pattern + * TEST_CP2520: HBR2 compliance eye pattern + * TEST_TPS1: Link training symbol pattern TPS1 (/D10.2/) + * TEST_TPS2: Link training symbol pattern TPS2 + * TEST_TPS3: Link training symbol pattern TPS3 (for HBR2) + */ +enum test_pattern { + TEST_VIDEO, + TEST_TPS1, + TEST_TPS2, + TEST_TPS3, + TEST_SYMBOL_ERROR, + TEST_PRBS7, + TEST_80BIT_CUSTOM, + TEST_CP2520, +}; + +static const char *const test_pattern_str[] = { + [TEST_VIDEO] = "video", + [TEST_TPS1] = "tps1", + [TEST_TPS2] = "tps2", + [TEST_TPS3] = "tps3", + [TEST_SYMBOL_ERROR] = "symbol-error", + [TEST_PRBS7] = "prbs7", + [TEST_80BIT_CUSTOM] = "80bit-custom", + [TEST_CP2520] = "cp2520", +}; + +/** + * struct zynqmp_dp_test - Configuration for test mode + * @pattern: The test pattern + * @enhanced: Use enhanced framing + * @downspread: Use SSC + * @active: Whether test mode is active + * @custom: Custom pattern for %TEST_80BIT_CUSTOM + * @train_set: Voltage/preemphasis settings + * @bw_code: Bandwidth code for the link + * @link_cnt: Number of lanes + */ +struct zynqmp_dp_test { + enum test_pattern pattern; + bool enhanced, downspread, active; + u8 custom[10]; + u8 train_set[ZYNQMP_DP_MAX_LANES]; + u8 bw_code; + u8 link_cnt; +}; + +/** + * struct zynqmp_dp_train_set_priv - Private data for train_set debugfs files + * @dp: DisplayPort IP core structure + * @lane: The lane for this file + */ +struct zynqmp_dp_train_set_priv { + struct zynqmp_dp *dp; + int lane; +}; + /** * struct zynqmp_dp - Xilinx DisplayPort core * @dev: device structure @@ -283,6 +353,7 @@ struct zynqmp_dp_config { * @irq: irq * @bridge: DRM bridge for the DP encoder * @next_bridge: The downstream bridge + * @test: Configuration for test mode * @config: IP core configuration from DTS * @aux: aux channel * @phy: PHY handles for DP lanes @@ -294,6 +365,7 @@ struct zynqmp_dp_config { * @link_config: common link configuration between IP core and sink device * @mode: current mode between IP core and sink device * @train_set: set