Re: [PATCH 5/5] tests: Test suspend/resume on active pipelines
Hi Kieran, On Friday 25 Nov 2016 18:40:34 Kieran Bingham wrote: > On 25/11/16 17:10, Laurent Pinchart wrote: > > On Friday 25 Nov 2016 13:59:16 Kieran Bingham wrote: > >> From: Kieran Bingham> >> > >> Provide a test to verify the hardware completes a functional test whilst > >> performing a suspend resume cycle in parallel. Make use of the > >> /sys/power/pm_test functionality provided by CONFIG_PM_DEBUG to perform > >> the testing > >> > >> Signed-off-by: Kieran Bingham > >> --- > >> > >> tests/vsp-unit-test-0020.sh | 86 +++ > >> 1 file changed, 86 insertions(+) > >> create mode 100755 tests/vsp-unit-test-0020.sh > >> > >> diff --git a/tests/vsp-unit-test-0020.sh b/tests/vsp-unit-test-0020.sh > >> new file mode 100755 > >> index ..5838295139b2 > >> --- /dev/null > >> +++ b/tests/vsp-unit-test-0020.sh > >> @@ -0,0 +1,86 @@ > >> +#!/bin/sh > >> + > >> +# > >> +# Test power-management suspend/resume whilst pipelines are active > >> +# > >> +# Utilise the basic RPF->WPF packing test case as a measure that the > >> hardware > >> +# is operable while we perform test suspend and resume, and verify that > >> it is > >> +# still successful even with a suspend resume cycle in the middle of the > >> test. > >> +# > >> +# Format iteration loops are maintained, even with only one format so > >> that this > >> +# test can be easily extended to try further formats if needed in the > >> future. > >> +# > > > > Given that this test will suspend during the first iteration only I don't > > think it's very useful to keep the loop. > > Agreed. Can you tell this test started it's life as > vsp-unit-test-0019.sh ? :D > > >> + > >> +source vsp-lib.sh > >> + > >> +features="rpf.0 wpf.0" > >> +formats="RGB24" > >> + > >> +# These can be extracted from /sys/power/pm_test > >> +suspend_modes="freezer devices platform processors core" > > > > How about extracting them from that file then ? :-) Or maybe just > > verifying they are available before trying to use them ? > > I thought about that - and looked at it. > > Annoyingly - /sys/power/pm_test contains none which would need to be > filtered out, and also the active mode is surrounded by square brackets, > which would also need filtering. > > # cat /sys/power/pm_test > none core processors platform [devices] freezer > > They are also in the reverse order of that which I wanted the tests to run. > > All solvable, but I hadn't bothered as I thought it was possibly > overkill. Of course extracting automatically does future proof us > against different modes and changing mode-names - so it does sound like > a valuable thing to do. You're right that it might be overkill, it's up to you. If you want to keep the list of modes here to select the order, you could do a grep -q $mode /sys/power/pm_test || continue (or something similar possibly with a message being printed to inform that the test is skipped) Even that might be overkill though. > >> +# This extended function performs the same > >> +# as it's non-extended name-sake - but runs the pipeline > >> +# for 300 frames. The suspend action occurs between frame #150~#200 > >> +test_extended_wpf_packing() { > >> + test_start "Verify WPF packing in $format during suspend:$mode" > >> + > >> + pipe_configure rpf-wpf 0 0 > >> + format_configure rpf-wpf 0 0 ARGB32 1024x768 $format > >> + > >> + vsp_runner rpf.0 --count=300 & > >> + vsp_runner wpf.0 --count=300 --skip=297 > >> + > >> + local result=$(compare_frames) > >> + > >> + test_complete $result > >> +} > >> + > >> +test_hw_pipe() { > >> + local format > >> + > >> + for format in $formats ; do > >> + test_extended_wpf_packing $format > >> + done > >> +} > >> + > >> +test_suspend_resume() { > >> + local test_results=0 > > > > This variable doesn't seem to be used. > > Another left over from when I was experimenting to get the return values > before I was blocked by $(compare_frames) returning a string. > > >> + local test_pid > >> + > >> + # Test the hardware each side of suspend resume > >> + test_hw_pipe & > >> + test_pid=$! > >> + > >> + # Make sure the pipeline has time to start > >> + sleep 1 > >> + > >> + # Set the test mode > >> + echo $mode > /sys/power/pm_test > >> + > >> + # Comence suspend > > > > Commence ? > > Yes > Where can I get a terminal emulator with spell-check highlighting :D > > >> + # The pm_test framework will automatically resume after 5 seconds > >> + echo mem > /sys/power/state > >> + > >> + # Wait for the pipeline to complete > >> + wait $test_pid > > > > It would be nice to add a timeout here. Maybe something like the following > > (untested) ? > > > > (sleep 30 ; kill $test_pid) & > > kill_pid=$! > > wait $test_pid > > kill $kill_pid > > > > test_complete should be called here based on both whether the frames > > compared successfully and whether the test timed
Re: [PATCH 1/5] scripts: Test suite runner
On 25/11/16 18:21, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Friday 25 Nov 2016 13:59:12 Kieran Bingham wrote: >> From: Kieran Bingham>> >> Provide a utility script to execute all vsp unit tests, as well >> as the option to execute multiple iterations of the suite. >> >> Signed-off-by: Kieran Bingham >> --- >> scripts/vsp-tests.sh | 46 ++ >> 1 file changed, 46 insertions(+) >> create mode 100755 scripts/vsp-tests.sh >> >> diff --git a/scripts/vsp-tests.sh b/scripts/vsp-tests.sh >> new file mode 100755 >> index ..e5ffa0ec4236 >> --- /dev/null >> +++ b/scripts/vsp-tests.sh >> @@ -0,0 +1,46 @@ >> +#!/bin/sh >> + >> +## >> +## VSP Tests runner >> +## >> +## Automatically execute all vsp-unit tests >> +## Move test failure results to a specific folder for >> +## the running kernel version >> +## >> +## An argument can be provided to specify the number of >> +## iterations to perform >> +## >> +## usage: >> +## ./vsp-tests.sh >> +## >> +## n: Number of iterations to execute test suite >> +## >> + >> +KERNEL_VERSION=`uname -r` >> + >> +run_test() { >> +echo $1 >> +./$1 >> + >> +if [ $(ls *.bin 2>/dev/null | wc -l) != 0 ]; >> +then >> +RESULTS_DIR=$KERNEL_VERSION/test-$1/$2/ >> + >> +echo "Moving *.bin to test-$1/$2/"; > > You can remove the ; at the end of the lines. > >> +mkdir -p $RESULTS_DIR; >> +echo "mv *.bin $RESULTS_DIR"; > > Do we really need this message ? We certainly don't need two :D. I'll combine the best bits of them both :D echo "Moving test failures to $RESULTS_DIR"; And in fact, now that bin2png.sh works on the target - this could also generate the PNG files immediately which would be useful. >> +mv *.bin $RESULTS_DIR; >> +fi; >> +} >> + >> +run_suite() { >> +echo "Test loop $1" >> + >> +for test in vsp-unit-test*.sh; do >> +run_test $test $1; >> +done; >> +} >> + >> +for loop in `seq 1 1 $1`; do >> +run_suite $loop >> +done; > -- Regards Kieran Bingham
Re: [PATCH 5/5] tests: Test suspend/resume on active pipelines
Hi Laurent, On 25/11/16 17:10, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Friday 25 Nov 2016 13:59:16 Kieran Bingham wrote: >> From: Kieran Bingham>> >> Provide a test to verify the hardware completes a functional test whilst >> performing a suspend resume cycle in parallel. Make use of the >> /sys/power/pm_test functionality provided by CONFIG_PM_DEBUG to perform >> the testing >> >> Signed-off-by: Kieran Bingham >> --- >> tests/vsp-unit-test-0020.sh | 86 ++ >> 1 file changed, 86 insertions(+) >> create mode 100755 tests/vsp-unit-test-0020.sh >> >> diff --git a/tests/vsp-unit-test-0020.sh b/tests/vsp-unit-test-0020.sh >> new file mode 100755 >> index ..5838295139b2 >> --- /dev/null >> +++ b/tests/vsp-unit-test-0020.sh >> @@ -0,0 +1,86 @@ >> +#!/bin/sh >> + >> +# >> +# Test power-management suspend/resume whilst pipelines are active >> +# >> +# Utilise the basic RPF->WPF packing test case as a measure that the >> hardware >> +# is operable while we perform test suspend and resume, and verify that it >> is >> +# still successful even with a suspend resume cycle in the middle of the >> test. >> +# >> +# Format iteration loops are maintained, even with only one format so that >> this >> +# test can be easily extended to try further formats if needed in the >> future. >> +# > > Given that this test will suspend during the first iteration only I don't > think it's very useful to keep the loop. > Agreed. Can you tell this test started it's life as vsp-unit-test-0019.sh ? :D >> + >> +source vsp-lib.sh >> + >> +features="rpf.0 wpf.0" >> +formats="RGB24" >> + >> +# These can be extracted from /sys/power/pm_test >> +suspend_modes="freezer devices platform processors core" > > How about extracting them from that file then ? :-) Or maybe just verifying > they are available before trying to use them ? I thought about that - and looked at it. Annoyingly - /sys/power/pm_test contains none which would need to be filtered out, and also the active mode is surrounded by square brackets, which would also need filtering. # cat /sys/power/pm_test none core processors platform [devices] freezer They are also in the reverse order of that which I wanted the tests to run. All solvable, but I hadn't bothered as I thought it was possibly overkill. Of course extracting automatically does future proof us against different modes and changing mode-names - so it does sound like a valuable thing to do. >> +# This extended function performs the same >> +# as it's non-extended name-sake - but runs the pipeline >> +# for 300 frames. The suspend action occurs between frame #150~#200 >> +test_extended_wpf_packing() { >> +test_start "Verify WPF packing in $format during suspend:$mode" >> + >> +pipe_configure rpf-wpf 0 0 >> +format_configure rpf-wpf 0 0 ARGB32 1024x768 $format >> + >> +vsp_runner rpf.0 --count=300 & >> +vsp_runner wpf.0 --count=300 --skip=297 >> + >> +local result=$(compare_frames) >> + >> +test_complete $result >> +} >> + >> +test_hw_pipe() { >> +local format >> + >> +for format in $formats ; do >> +test_extended_wpf_packing $format >> +done >> +} >> + >> +test_suspend_resume() { >> +local test_results=0 > > This variable doesn't seem to be used. > Another left over from when I was experimenting to get the return values before I was blocked by $(compare_frames) returning a string. >> +local test_pid >> + >> +# Test the hardware each side of suspend resume >> +test_hw_pipe & >> +test_pid=$! >> + >> +# Make sure the pipeline has time to start >> +sleep 1 >> + >> +# Set the test mode >> +echo $mode > /sys/power/pm_test >> + >> +# Comence suspend > > Commence ? Yes Where can I get a terminal emulator with spell-check highlighting :D > >> +# The pm_test framework will automatically resume after 5 seconds >> +echo mem > /sys/power/state >> + >> +# Wait for the pipeline to complete >> +wait $test_pid > > It would be nice to add a timeout here. Maybe something like the following > (untested) ? > > (sleep 30 ; kill $test_pid) & > kill_pid=$! > wait $test_pid > kill $kill_pid > > test_complete should be called here based on both whether the frames compared > successfully and whether the test timed out. > I agree that the test should have a timeout, and result determined, but scripting this in shell ... feels ugh :S Its a shame we don't have gnu-timeout - http://man7.org/linux/man-pages/man1/timeout.1.html(man 1 timeout) >> +} >> + >> +test_main() { >> +local mode; > > No need for the trailing ;. > Ack. >> +local suspend_test_failures > > This variable doesn't seem to be used. Hrm .. maybe I forgot to do a fresh git-format-patch after I fixed these up :D > >> + >> +# Check for pm-suspend test option
Re: [PATCH 3/5] logger: Log to the FTrace buffer if tracing is enabled
Hi Kieran, On Friday 25 Nov 2016 18:10:10 Kieran Bingham wrote: > On 25/11/16 17:40, Laurent Pinchart wrote: > > On Friday 25 Nov 2016 13:59:14 Kieran Bingham wrote: > >> From: Kieran Bingham> >> > >> Extend the logger such that it will detect the tracing system, and also > >> append print statement to this ring buffer. > >> > >> This provides the relevant logging output interspersed in the ftrace > >> logs for an effective solution to identifying the actions that caused > >> the traces to occur > >> > >> Signed-off-by: Kieran Bingham > >> --- > >> > >> scripts/logger.sh | 13 - > >> 1 file changed, 12 insertions(+), 1 deletion(-) > >> > >> diff --git a/scripts/logger.sh b/scripts/logger.sh > >> index 8123f0c9f6e3..8412b0ba9a08 100755 > >> --- a/scripts/logger.sh > >> +++ b/scripts/logger.sh > >> @@ -6,6 +6,17 @@ now() { > >> > >> label=${1:+ [$1]} > >> > >> +TRACE_MARKER=/sys/kernel/debug/tracing/trace_marker > >> +if [ -e $TRACE_MARKER ]; then > >> + extra_log_files=$TRACE_MARKER > >> +fi > >> + > >> > >> while read line ; do > >> > >> - echo "$(now)$label $line" > >> + newline="$(now)$label $line" > >> + > >> + echo "$newline" > >> + > >> + for f in $extra_log_files; do > >> + echo "$newline" >> $f; > > > > As the tracer adds a timestamp, should you echo "$label $line" only here ? > > Hrm, yes it is possibly a little bit redundant... > > My only argument would be that it will be a 'different' timestamp to the > one logged by logger.sh. > > Inspection of a recent log shows a difference of around 40-50 ms, which > will be the latency between capturing the time in $(now) and passing the > buffer to the kernel. > > At this level though, the logger.sh is already susceptible to scheduler > jitter anyway, so I'm not too worried about 40 ms. But anyone reading > the logs will have to be aware of that extra latency. If you think we should keep the timestamp I'm fine with that. > >> + done; > >> > >> done -- Regards, Laurent Pinchart
Re: [PATCH 1/5] scripts: Test suite runner
Hi Kieran, Thank you for the patch. On Friday 25 Nov 2016 13:59:12 Kieran Bingham wrote: > From: Kieran Bingham> > Provide a utility script to execute all vsp unit tests, as well > as the option to execute multiple iterations of the suite. > > Signed-off-by: Kieran Bingham > --- > scripts/vsp-tests.sh | 46 ++ > 1 file changed, 46 insertions(+) > create mode 100755 scripts/vsp-tests.sh > > diff --git a/scripts/vsp-tests.sh b/scripts/vsp-tests.sh > new file mode 100755 > index ..e5ffa0ec4236 > --- /dev/null > +++ b/scripts/vsp-tests.sh > @@ -0,0 +1,46 @@ > +#!/bin/sh > + > +## > +## VSP Tests runner > +## > +## Automatically execute all vsp-unit tests > +## Move test failure results to a specific folder for > +## the running kernel version > +## > +## An argument can be provided to specify the number of > +## iterations to perform > +## > +## usage: > +## ./vsp-tests.sh > +## > +## n: Number of iterations to execute test suite > +## > + > +KERNEL_VERSION=`uname -r` > + > +run_test() { > + echo $1 > + ./$1 > + > + if [ $(ls *.bin 2>/dev/null | wc -l) != 0 ]; > + then > + RESULTS_DIR=$KERNEL_VERSION/test-$1/$2/ > + > + echo "Moving *.bin to test-$1/$2/"; You can remove the ; at the end of the lines. > + mkdir -p $RESULTS_DIR; > + echo "mv *.bin $RESULTS_DIR"; Do we really need this message ? > + mv *.bin $RESULTS_DIR; > + fi; > +} > + > +run_suite() { > + echo "Test loop $1" > + > + for test in vsp-unit-test*.sh; do > + run_test $test $1; > + done; > +} > + > +for loop in `seq 1 1 $1`; do > + run_suite $loop > +done; -- Regards, Laurent Pinchart
Re: [PATCH 3/5] logger: Log to the FTrace buffer if tracing is enabled
Hi Laurent, On 25/11/16 17:40, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Friday 25 Nov 2016 13:59:14 Kieran Bingham wrote: >> From: Kieran Bingham>> >> Extend the logger such that it will detect the tracing system, and also >> append print statement to this ring buffer. >> >> This provides the relevant logging output interspersed in the ftrace >> logs for an effective solution to identifying the actions that caused >> the traces to occur >> >> Signed-off-by: Kieran Bingham >> --- >> scripts/logger.sh | 13 - >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/scripts/logger.sh b/scripts/logger.sh >> index 8123f0c9f6e3..8412b0ba9a08 100755 >> --- a/scripts/logger.sh >> +++ b/scripts/logger.sh >> @@ -6,6 +6,17 @@ now() { >> >> label=${1:+ [$1]} >> >> +TRACE_MARKER=/sys/kernel/debug/tracing/trace_marker >> +if [ -e $TRACE_MARKER ]; then >> +extra_log_files=$TRACE_MARKER >> +fi >> + >> while read line ; do >> -echo "$(now)$label $line" >> +newline="$(now)$label $line" >> + >> +echo "$newline" >> + >> +for f in $extra_log_files; do >> +echo "$newline" >> $f; > > As the tracer adds a timestamp, should you echo "$label $line" only here ? Hrm, yes it is possibly a little bit redundant... My only argument would be that it will be a 'different' timestamp to the one logged by logger.sh. Inspection of a recent log shows a difference of around 40-50 ms, which will be the latency between capturing the time in $(now) and passing the buffer to the kernel. At this level though, the logger.sh is already susceptible to scheduler jitter anyway, so I'm not too worried about 40 ms. But anyone reading the logs will have to be aware of that extra latency. > >> +done; >> done >
Re: [PATCH 4/5] tests: Test suspend/resume on idle pipelines
On 25/11/16 17:13, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Friday 25 Nov 2016 13:59:15 Kieran Bingham wrote: >> From: Kieran Bingham>> >> Provide a test to verify the hardware is functional both before and >> after entering a suspend / resume cycle. Make use of the >> /sys/power/pm_test functionality provided by CONFIG_PM_DEBUG to perform >> the testing >> >> Signed-off-by: Kieran Bingham >> --- >> tests/vsp-unit-test-0019.sh | 77 ++ >> 1 file changed, 77 insertions(+) >> create mode 100755 tests/vsp-unit-test-0019.sh >> >> diff --git a/tests/vsp-unit-test-0019.sh b/tests/vsp-unit-test-0019.sh >> new file mode 100755 >> index ..e7b94996c1aa >> --- /dev/null >> +++ b/tests/vsp-unit-test-0019.sh >> @@ -0,0 +1,77 @@ >> +#!/bin/sh >> + >> +# >> +# Test power-management suspend/resume whilst pipelines are idle >> +# >> +# Utilise the basic RPF->WPF packing test case as a measure that the >> hardware >> +# is operable while we perform test suspend and resume, and verify that it >> is >> +# still operable after resume. >> +# >> +# Format iteration loops are maintained, even with only one format so that >> this >> +# test can be easily extended to try further formats if needed in the >> future. >> +# > > I don't think testing multiple formats is needed, but I would like the > test_wpf_packing() function to be called twice, to verify that stop -> start > works fine after resume. You could specify two formats to achieve that (with > a > comment explaining why at least two formats should be specified). > That sounds like a good plan. >> +source vsp-lib.sh >> + >> +features="rpf.0 wpf.0" >> +formats="RGB24" >> + >> +# These can be extracted from /sys/power/pm_test >> +suspend_modes="freezer devices platform processors core" >> + >> +test_wpf_packing() { >> +test_start "Verify WPF packing in $format before/after suspend:$mode" >> + >> +pipe_configure rpf-wpf 0 0 >> +format_configure rpf-wpf 0 0 ARGB32 1024x768 $format >> + >> +vsp_runner rpf.0 & >> +vsp_runner wpf.0 >> + >> +local result=$(compare_frames) >> + >> +test_complete $result >> +} >> + >> +test_hw_pipe() { >> +local format >> + >> +for format in $formats ; do >> +test_wpf_packing $format >> +done >> +} >> + >> +test_suspend_resume() { >> +local test_results=0 > > This variable is unused. Bah, I missed some. I thought I'd cleaned these out before posting. :D >> + >> +# Test the hardware each side of suspend resume >> +test_hw_pipe >> + >> +# Set the test mode >> +echo $mode > /sys/power/pm_test >> + >> +# Comence suspend >> +# The pm_test framework will automatically resume after 5 seconds >> +echo mem > /sys/power/state >> + >> +# Verify the hardware is still operational >> +test_hw_pipe > > Given that the goal is to test proper operation after resume, it would be > better to call test_start and test_complete in this function, based on > whether > all the tests passed or not. Yes, I did go down this route - but stalled. Mainly in getting sensible return values from the $(compare_frames) which can be tracked through the functions. I think it looked like I'd have to string match against, pass, skip, fail etc ... without a nice easy return value I can add to an integer. This can be re-examined. >> +} >> + >> +test_main() { >> +local mode; > > No need for the trailing ;. I think my hands are preprogrammed in 'c' coding style. Tough to get out of sometimes :D >> + >> +# Check for pm-suspend test option >> +if [ ! -e /sys/power/pm_test ] ; then >> +echo "$0: Suspend Resume testing requires CONFIG_PM_DEBUG" >> +test_complete skip >> +return >> +fi; > > Ditto. Ack. > >> + >> +for mode in $suspend_modes ; do >> +test_suspend_resume $mode >> +done; > > Ditto. > Ack. >> +} >> + >> +test_init $0 "$features" >> +test_run >
Re: [PATCH 2/5] scripts: Provide bin2png.sh helper
Hi Laurent, Thanks for the review :D On 25/11/16 17:55, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Friday 25 Nov 2016 13:59:13 Kieran Bingham wrote: >> From: Kieran Bingham>> >> Identify the size and format from the test output filename, and pass >> to raw2rgbpnm for conversion to a PNM file. >> >> From there we can convert easily to a PNG output file. >> >> Signed-off-by: Kieran Bingham >> --- >> scripts/Makefile | 2 +- >> scripts/bin2png.sh | 34 ++ >> 2 files changed, 35 insertions(+), 1 deletion(-) >> create mode 100755 scripts/bin2png.sh >> >> diff --git a/scripts/Makefile b/scripts/Makefile >> index 8c452f4c54ce..6586b2989aed 100644 >> --- a/scripts/Makefile >> +++ b/scripts/Makefile >> @@ -1,4 +1,4 @@ >> -SCRIPTS=logger.sh vsp-lib.sh >> +SCRIPTS=$(wildcard *.sh) >> >> all: >> >> diff --git a/scripts/bin2png.sh b/scripts/bin2png.sh >> new file mode 100755 >> index ..527c5546514d >> --- /dev/null >> +++ b/scripts/bin2png.sh >> @@ -0,0 +1,34 @@ >> +#!/bin/sh >> + >> +FILE="$1" >> + >> +fmt=$(echo $FILE | sed -e >> 's/.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin/\1/') >> +size=$(echo $FILE | >> sed -e 's/.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin/\2/') >> + >> +case $fmt in >> +yuv410m|yvu410m|yuv411m|yuv420m|yvu420m|yuv422m|yvu422m|yuv444m| > yvu444m) >> +fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` >> +fmt=`echo $fmt | tr 'M' 'P'` >> +;; >> +nv12m|nv21m|nv16m|nv61m) >> +fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` >> +fmt=`echo $fmt | tr -d 'M'` >> +;; >> +argb555|xrgb555) >> +fmt=RGB555X >> +;; >> +argb32|xrgb32) >> +fmt=RGB32 >> +;; >> +abgr32|xbgr32) >> +fmt=BGR32 >> +;; >> +*) >> +fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` >> +;; >> +esac >> + >> +# Only run pnmtopng if the conversion to PNM succeeds >> +raw2rgbpnm -s $size -f $fmt $FILE $FILE.pnm && \ > > How about stripping the .bin suffix to avoid calling the output file > *.bin.png > ? Yeah, we can probably manage this :D >> +pnmtopng $FILE.pnm > $FILE.png > > We already have a dependency on ImageMagick (although on the target side, not > the host side). Shoould we use convert instead of pnmtopng ? Ahh - excellent plan. Actually I'd been using this script on the host only, using convert makes it easy to run this on the target as well. That means in fact that I can convert 'bin' files from tmpfs storage and output small png's without hammering the network on NFS usage. Sounds like a win to me ! > >> +rm $FILE.pnm >
Re: [PATCH 2/5] scripts: Provide bin2png.sh helper
Hi Kieran, Thank you for the patch. On Friday 25 Nov 2016 13:59:13 Kieran Bingham wrote: > From: Kieran Bingham> > Identify the size and format from the test output filename, and pass > to raw2rgbpnm for conversion to a PNM file. > > From there we can convert easily to a PNG output file. > > Signed-off-by: Kieran Bingham > --- > scripts/Makefile | 2 +- > scripts/bin2png.sh | 34 ++ > 2 files changed, 35 insertions(+), 1 deletion(-) > create mode 100755 scripts/bin2png.sh > > diff --git a/scripts/Makefile b/scripts/Makefile > index 8c452f4c54ce..6586b2989aed 100644 > --- a/scripts/Makefile > +++ b/scripts/Makefile > @@ -1,4 +1,4 @@ > -SCRIPTS=logger.sh vsp-lib.sh > +SCRIPTS=$(wildcard *.sh) > > all: > > diff --git a/scripts/bin2png.sh b/scripts/bin2png.sh > new file mode 100755 > index ..527c5546514d > --- /dev/null > +++ b/scripts/bin2png.sh > @@ -0,0 +1,34 @@ > +#!/bin/sh > + > +FILE="$1" > + > +fmt=$(echo $FILE | sed -e > 's/.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin/\1/') > +size=$(echo $FILE | > sed -e 's/.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin/\2/') > + > +case $fmt in > + yuv410m|yvu410m|yuv411m|yuv420m|yvu420m|yuv422m|yvu422m|yuv444m| yvu444m) > + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` > + fmt=`echo $fmt | tr 'M' 'P'` > + ;; > + nv12m|nv21m|nv16m|nv61m) > + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` > + fmt=`echo $fmt | tr -d 'M'` > + ;; > + argb555|xrgb555) > + fmt=RGB555X > + ;; > + argb32|xrgb32) > + fmt=RGB32 > + ;; > + abgr32|xbgr32) > + fmt=BGR32 > + ;; > + *) > + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` > + ;; > +esac > + > +# Only run pnmtopng if the conversion to PNM succeeds > +raw2rgbpnm -s $size -f $fmt $FILE $FILE.pnm && \ How about stripping the .bin suffix to avoid calling the output file *.bin.png ? > + pnmtopng $FILE.pnm > $FILE.png We already have a dependency on ImageMagick (although on the target side, not the host side). Shoould we use convert instead of pnmtopng ? > +rm $FILE.pnm -- Regards, Laurent Pinchart
Re: [PATCH 3/5] logger: Log to the FTrace buffer if tracing is enabled
Hi Kieran, Thank you for the patch. On Friday 25 Nov 2016 13:59:14 Kieran Bingham wrote: > From: Kieran Bingham> > Extend the logger such that it will detect the tracing system, and also > append print statement to this ring buffer. > > This provides the relevant logging output interspersed in the ftrace > logs for an effective solution to identifying the actions that caused > the traces to occur > > Signed-off-by: Kieran Bingham > --- > scripts/logger.sh | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/scripts/logger.sh b/scripts/logger.sh > index 8123f0c9f6e3..8412b0ba9a08 100755 > --- a/scripts/logger.sh > +++ b/scripts/logger.sh > @@ -6,6 +6,17 @@ now() { > > label=${1:+ [$1]} > > +TRACE_MARKER=/sys/kernel/debug/tracing/trace_marker > +if [ -e $TRACE_MARKER ]; then > + extra_log_files=$TRACE_MARKER > +fi > + > while read line ; do > - echo "$(now)$label $line" > + newline="$(now)$label $line" > + > + echo "$newline" > + > + for f in $extra_log_files; do > + echo "$newline" >> $f; As the tracer adds a timestamp, should you echo "$label $line" only here ? > + done; > done -- Regards, Laurent Pinchart
Re: [PATCH 4/5] tests: Test suspend/resume on idle pipelines
Hi Kieran, Thank you for the patch. On Friday 25 Nov 2016 13:59:15 Kieran Bingham wrote: > From: Kieran Bingham> > Provide a test to verify the hardware is functional both before and > after entering a suspend / resume cycle. Make use of the > /sys/power/pm_test functionality provided by CONFIG_PM_DEBUG to perform > the testing > > Signed-off-by: Kieran Bingham > --- > tests/vsp-unit-test-0019.sh | 77 ++ > 1 file changed, 77 insertions(+) > create mode 100755 tests/vsp-unit-test-0019.sh > > diff --git a/tests/vsp-unit-test-0019.sh b/tests/vsp-unit-test-0019.sh > new file mode 100755 > index ..e7b94996c1aa > --- /dev/null > +++ b/tests/vsp-unit-test-0019.sh > @@ -0,0 +1,77 @@ > +#!/bin/sh > + > +# > +# Test power-management suspend/resume whilst pipelines are idle > +# > +# Utilise the basic RPF->WPF packing test case as a measure that the > hardware > +# is operable while we perform test suspend and resume, and verify that it > is > +# still operable after resume. > +# > +# Format iteration loops are maintained, even with only one format so that > this > +# test can be easily extended to try further formats if needed in the > future. > +# I don't think testing multiple formats is needed, but I would like the test_wpf_packing() function to be called twice, to verify that stop -> start works fine after resume. You could specify two formats to achieve that (with a comment explaining why at least two formats should be specified). > +source vsp-lib.sh > + > +features="rpf.0 wpf.0" > +formats="RGB24" > + > +# These can be extracted from /sys/power/pm_test > +suspend_modes="freezer devices platform processors core" > + > +test_wpf_packing() { > + test_start "Verify WPF packing in $format before/after suspend:$mode" > + > + pipe_configure rpf-wpf 0 0 > + format_configure rpf-wpf 0 0 ARGB32 1024x768 $format > + > + vsp_runner rpf.0 & > + vsp_runner wpf.0 > + > + local result=$(compare_frames) > + > + test_complete $result > +} > + > +test_hw_pipe() { > + local format > + > + for format in $formats ; do > + test_wpf_packing $format > + done > +} > + > +test_suspend_resume() { > + local test_results=0 This variable is unused. > + > + # Test the hardware each side of suspend resume > + test_hw_pipe > + > + # Set the test mode > + echo $mode > /sys/power/pm_test > + > + # Comence suspend > + # The pm_test framework will automatically resume after 5 seconds > + echo mem > /sys/power/state > + > + # Verify the hardware is still operational > + test_hw_pipe Given that the goal is to test proper operation after resume, it would be better to call test_start and test_complete in this function, based on whether all the tests passed or not. > +} > + > +test_main() { > + local mode; No need for the trailing ;. > + > + # Check for pm-suspend test option > + if [ ! -e /sys/power/pm_test ] ; then > + echo "$0: Suspend Resume testing requires CONFIG_PM_DEBUG" > + test_complete skip > + return > + fi; Ditto. > + > + for mode in $suspend_modes ; do > + test_suspend_resume $mode > + done; Ditto. > +} > + > +test_init $0 "$features" > +test_run -- Regards, Laurent Pinchart
Re: [PATCH 5/5] tests: Test suspend/resume on active pipelines
Hi Kieran, Thank you for the patch. On Friday 25 Nov 2016 13:59:16 Kieran Bingham wrote: > From: Kieran Bingham> > Provide a test to verify the hardware completes a functional test whilst > performing a suspend resume cycle in parallel. Make use of the > /sys/power/pm_test functionality provided by CONFIG_PM_DEBUG to perform > the testing > > Signed-off-by: Kieran Bingham > --- > tests/vsp-unit-test-0020.sh | 86 ++ > 1 file changed, 86 insertions(+) > create mode 100755 tests/vsp-unit-test-0020.sh > > diff --git a/tests/vsp-unit-test-0020.sh b/tests/vsp-unit-test-0020.sh > new file mode 100755 > index ..5838295139b2 > --- /dev/null > +++ b/tests/vsp-unit-test-0020.sh > @@ -0,0 +1,86 @@ > +#!/bin/sh > + > +# > +# Test power-management suspend/resume whilst pipelines are active > +# > +# Utilise the basic RPF->WPF packing test case as a measure that the > hardware > +# is operable while we perform test suspend and resume, and verify that it > is > +# still successful even with a suspend resume cycle in the middle of the > test. > +# > +# Format iteration loops are maintained, even with only one format so that > this > +# test can be easily extended to try further formats if needed in the > future. > +# Given that this test will suspend during the first iteration only I don't think it's very useful to keep the loop. > + > +source vsp-lib.sh > + > +features="rpf.0 wpf.0" > +formats="RGB24" > + > +# These can be extracted from /sys/power/pm_test > +suspend_modes="freezer devices platform processors core" How about extracting them from that file then ? :-) Or maybe just verifying they are available before trying to use them ? > +# This extended function performs the same > +# as it's non-extended name-sake - but runs the pipeline > +# for 300 frames. The suspend action occurs between frame #150~#200 > +test_extended_wpf_packing() { > + test_start "Verify WPF packing in $format during suspend:$mode" > + > + pipe_configure rpf-wpf 0 0 > + format_configure rpf-wpf 0 0 ARGB32 1024x768 $format > + > + vsp_runner rpf.0 --count=300 & > + vsp_runner wpf.0 --count=300 --skip=297 > + > + local result=$(compare_frames) > + > + test_complete $result > +} > + > +test_hw_pipe() { > + local format > + > + for format in $formats ; do > + test_extended_wpf_packing $format > + done > +} > + > +test_suspend_resume() { > + local test_results=0 This variable doesn't seem to be used. > + local test_pid > + > + # Test the hardware each side of suspend resume > + test_hw_pipe & > + test_pid=$! > + > + # Make sure the pipeline has time to start > + sleep 1 > + > + # Set the test mode > + echo $mode > /sys/power/pm_test > + > + # Comence suspend Commence ? > + # The pm_test framework will automatically resume after 5 seconds > + echo mem > /sys/power/state > + > + # Wait for the pipeline to complete > + wait $test_pid It would be nice to add a timeout here. Maybe something like the following (untested) ? (sleep 30 ; kill $test_pid) & kill_pid=$! wait $test_pid kill $kill_pid test_complete should be called here based on both whether the frames compared successfully and whether the test timed out. > +} > + > +test_main() { > + local mode; No need for the trailing ;. > + local suspend_test_failures This variable doesn't seem to be used. > + > + # Check for pm-suspend test option > + if [ ! -e /sys/power/pm_test ] ; then > + echo "$0: Suspend Resume testing requires CONFIG_PM_DEBUG" > + test_complete skip > + return > + fi; > + > + for mode in $suspend_modes ; do > + test_suspend_resume $mode > + done; > +} > + > +test_init $0 "$features" > +test_run -- Regards, Laurent Pinchart
[RFC] New Device Tree property - "bonding"
Hello DT maintainers, In one of the Renesas SoCs we have a device called DRIF (Digital Radio Interface) controller. A DRIF channel contains 4 external pins - SCK, SYNC, Data pins D0 & D1. Internally a DRIF channel is made up of two SPI slave devices (also called sub-channels here) that share common CLK & SYNC signals but have their own resource set. The DRIF channel can have either one of the sub-channel active at a time or both. When both sub-channels are active, they need to be managed together as one device as they share same CLK & SYNC. We plan to tie these two sub-channels together with a new property called "renesas,bonding". We would appreciate your feedback on this regard. Below is an example usage of this property. SoC common dtsi file: - "renesas,bonding" - phandles to similar devices that are part of the bond. A bond between devices implies that they need to be managed together and cannot operate independently when more than one member of the bond needs to be active. drif00: rif@e6f4 { compatible = "renesas,r8a7795-drif", "renesas,rcar-gen3-drif"; reg = <0 0xe6f4 0 0x64>; interrupts = ; clocks = < CPG_MOD 515>; clock-names = "fck"; dmas = < 0x20>, < 0x20>; dma-names = "rx", "rx"; renesas,bonding = <>; power-domains = < R8A7795_PD_ALWAYS_ON>; status = "disabled"; }; drif01: rif@e6f5 { compatible = "renesas,r8a7795-drif", "renesas,rcar-gen3-drif"; reg = <0 0xe6f5 0 0x64>; interrupts = ; clocks = < CPG_MOD 514>; clock-names = "fck"; dmas = < 0x22>, < 0x22>; dma-names = "rx", "rx"; renesas,bonding = <>; power-domains = < R8A7795_PD_ALWAYS_ON>; status = "disabled"; }; Board specific dts file: - - When more than one member of the "bonding" property are available, another keyword ("primary" or "bonded" or ??) is expected on any one of the members. This "primary" member controls/accepts properties common to all members. - When only a single member of the bond needs to be active, the "bonding" property has no effect. Note: "primary" keyword is found in "http://elinux.org/images/c/cf/Power_ePAPR_APPROVED_v1.1.pdf; Page 103, Line 10. It looks like a generic property but documentation is not available. E.g. 1) Both channels enabled case: { pinctrl-0 = <_pins>; pinctrl-names = "default"; renesas,syncac-pol-high; status = "okay"; primary; /* This can be changed to "renesas,primary or renesas,bonded or ??" */ port { drif0_ep: endpoint { remote-endpoint = <_ep>; }; }; }; { status = "okay"; }; 2) Single channel enabled case: --- { pinctrl-0 = <_pins>; pinctrl-names = "default"; status = "okay"; port { drif0_ep: endpoint { remote-endpoint = <_ep>; }; }; }; Thanks, Ramesh. Note 1: Below is a pictorial representation of E.g. (1) and (2) A DRIF channel can interface with a Master device like this 1) Master with two data pins - both sub-channels used +-++-+ | |-SCK--->|CLK | | Master|-SS>|SYNC DRIFn (slave) | | |-SD0--->|D0 | | |-SD1--->|D1 | +-++-+ 2) Master with one data pin - sub-channel0 or 1 can be used based on the board. Below is e.g. where sub-channel1 alone is used. D0 pin is unused. +-++-+ | |-SCK--->|CLK | | Master|-SS>|SYNC DRIFn (slave) | | |-SD0--->|D1 | | || | +-++-+ Note 2: This topic is discussed as part of the driver submission. The latest on that is here - https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg09037.html None of the models discussed are convincing. This new property proposal is closest to
[PATCH 4/5] tests: Test suspend/resume on idle pipelines
From: Kieran BinghamProvide a test to verify the hardware is functional both before and after entering a suspend / resume cycle. Make use of the /sys/power/pm_test functionality provided by CONFIG_PM_DEBUG to perform the testing Signed-off-by: Kieran Bingham --- tests/vsp-unit-test-0019.sh | 77 + 1 file changed, 77 insertions(+) create mode 100755 tests/vsp-unit-test-0019.sh diff --git a/tests/vsp-unit-test-0019.sh b/tests/vsp-unit-test-0019.sh new file mode 100755 index ..e7b94996c1aa --- /dev/null +++ b/tests/vsp-unit-test-0019.sh @@ -0,0 +1,77 @@ +#!/bin/sh + +# +# Test power-management suspend/resume whilst pipelines are idle +# +# Utilise the basic RPF->WPF packing test case as a measure that the hardware +# is operable while we perform test suspend and resume, and verify that it is +# still operable after resume. +# +# Format iteration loops are maintained, even with only one format so that this +# test can be easily extended to try further formats if needed in the future. +# + +source vsp-lib.sh + +features="rpf.0 wpf.0" +formats="RGB24" + +# These can be extracted from /sys/power/pm_test +suspend_modes="freezer devices platform processors core" + +test_wpf_packing() { + test_start "Verify WPF packing in $format before/after suspend:$mode" + + pipe_configure rpf-wpf 0 0 + format_configure rpf-wpf 0 0 ARGB32 1024x768 $format + + vsp_runner rpf.0 & + vsp_runner wpf.0 + + local result=$(compare_frames) + + test_complete $result +} + +test_hw_pipe() { + local format + + for format in $formats ; do + test_wpf_packing $format + done +} + +test_suspend_resume() { + local test_results=0 + + # Test the hardware each side of suspend resume + test_hw_pipe + + # Set the test mode + echo $mode > /sys/power/pm_test + + # Comence suspend + # The pm_test framework will automatically resume after 5 seconds + echo mem > /sys/power/state + + # Verify the hardware is still operational + test_hw_pipe +} + +test_main() { + local mode; + + # Check for pm-suspend test option + if [ ! -e /sys/power/pm_test ] ; then + echo "$0: Suspend Resume testing requires CONFIG_PM_DEBUG" + test_complete skip + return + fi; + + for mode in $suspend_modes ; do + test_suspend_resume $mode + done; +} + +test_init $0 "$features" +test_run -- 2.7.4
[PATCH 2/5] scripts: Provide bin2png.sh helper
From: Kieran BinghamIdentify the size and format from the test output filename, and pass to raw2rgbpnm for conversion to a PNM file. >From there we can convert easily to a PNG output file. Signed-off-by: Kieran Bingham --- scripts/Makefile | 2 +- scripts/bin2png.sh | 34 ++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100755 scripts/bin2png.sh diff --git a/scripts/Makefile b/scripts/Makefile index 8c452f4c54ce..6586b2989aed 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -1,4 +1,4 @@ -SCRIPTS=logger.sh vsp-lib.sh +SCRIPTS=$(wildcard *.sh) all: diff --git a/scripts/bin2png.sh b/scripts/bin2png.sh new file mode 100755 index ..527c5546514d --- /dev/null +++ b/scripts/bin2png.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +FILE="$1" + +fmt=$(echo $FILE | sed -e 's/.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin/\1/') +size=$(echo $FILE | sed -e 's/.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin/\2/') + +case $fmt in + yuv410m|yvu410m|yuv411m|yuv420m|yvu420m|yuv422m|yvu422m|yuv444m|yvu444m) + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` + fmt=`echo $fmt | tr 'M' 'P'` + ;; + nv12m|nv21m|nv16m|nv61m) + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` + fmt=`echo $fmt | tr -d 'M'` + ;; + argb555|xrgb555) + fmt=RGB555X + ;; + argb32|xrgb32) + fmt=RGB32 + ;; + abgr32|xbgr32) + fmt=BGR32 + ;; + *) + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'` + ;; +esac + +# Only run pnmtopng if the conversion to PNM succeeds +raw2rgbpnm -s $size -f $fmt $FILE $FILE.pnm && \ + pnmtopng $FILE.pnm > $FILE.png +rm $FILE.pnm -- 2.7.4
[PATCH 3/5] logger: Log to the FTrace buffer if tracing is enabled
From: Kieran BinghamExtend the logger such that it will detect the tracing system, and also append print statement to this ring buffer. This provides the relevant logging output interspersed in the ftrace logs for an effective solution to identifying the actions that caused the traces to occur Signed-off-by: Kieran Bingham --- scripts/logger.sh | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/scripts/logger.sh b/scripts/logger.sh index 8123f0c9f6e3..8412b0ba9a08 100755 --- a/scripts/logger.sh +++ b/scripts/logger.sh @@ -6,6 +6,17 @@ now() { label=${1:+ [$1]} +TRACE_MARKER=/sys/kernel/debug/tracing/trace_marker +if [ -e $TRACE_MARKER ]; then + extra_log_files=$TRACE_MARKER +fi + while read line ; do - echo "$(now)$label $line" + newline="$(now)$label $line" + + echo "$newline" + + for f in $extra_log_files; do + echo "$newline" >> $f; + done; done -- 2.7.4
[PATCH 1/5] scripts: Test suite runner
From: Kieran BinghamProvide a utility script to execute all vsp unit tests, as well as the option to execute multiple iterations of the suite. Signed-off-by: Kieran Bingham --- scripts/vsp-tests.sh | 46 ++ 1 file changed, 46 insertions(+) create mode 100755 scripts/vsp-tests.sh diff --git a/scripts/vsp-tests.sh b/scripts/vsp-tests.sh new file mode 100755 index ..e5ffa0ec4236 --- /dev/null +++ b/scripts/vsp-tests.sh @@ -0,0 +1,46 @@ +#!/bin/sh + +## +## VSP Tests runner +## +## Automatically execute all vsp-unit tests +## Move test failure results to a specific folder for +## the running kernel version +## +## An argument can be provided to specify the number of +## iterations to perform +## +## usage: +## ./vsp-tests.sh +## +## n: Number of iterations to execute test suite +## + +KERNEL_VERSION=`uname -r` + +run_test() { + echo $1 + ./$1 + + if [ $(ls *.bin 2>/dev/null | wc -l) != 0 ]; + then + RESULTS_DIR=$KERNEL_VERSION/test-$1/$2/ + + echo "Moving *.bin to test-$1/$2/"; + mkdir -p $RESULTS_DIR; + echo "mv *.bin $RESULTS_DIR"; + mv *.bin $RESULTS_DIR; + fi; +} + +run_suite() { + echo "Test loop $1" + + for test in vsp-unit-test*.sh; do + run_test $test $1; + done; +} + +for loop in `seq 1 1 $1`; do + run_suite $loop +done; -- 2.7.4
[PATCH 5/5] tests: Test suspend/resume on active pipelines
From: Kieran BinghamProvide a test to verify the hardware completes a functional test whilst performing a suspend resume cycle in parallel. Make use of the /sys/power/pm_test functionality provided by CONFIG_PM_DEBUG to perform the testing Signed-off-by: Kieran Bingham --- tests/vsp-unit-test-0020.sh | 86 + 1 file changed, 86 insertions(+) create mode 100755 tests/vsp-unit-test-0020.sh diff --git a/tests/vsp-unit-test-0020.sh b/tests/vsp-unit-test-0020.sh new file mode 100755 index ..5838295139b2 --- /dev/null +++ b/tests/vsp-unit-test-0020.sh @@ -0,0 +1,86 @@ +#!/bin/sh + +# +# Test power-management suspend/resume whilst pipelines are active +# +# Utilise the basic RPF->WPF packing test case as a measure that the hardware +# is operable while we perform test suspend and resume, and verify that it is +# still successful even with a suspend resume cycle in the middle of the test. +# +# Format iteration loops are maintained, even with only one format so that this +# test can be easily extended to try further formats if needed in the future. +# + +source vsp-lib.sh + +features="rpf.0 wpf.0" +formats="RGB24" + +# These can be extracted from /sys/power/pm_test +suspend_modes="freezer devices platform processors core" + +# This extended function performs the same +# as it's non-extended name-sake - but runs the pipeline +# for 300 frames. The suspend action occurs between frame #150~#200 +test_extended_wpf_packing() { + test_start "Verify WPF packing in $format during suspend:$mode" + + pipe_configure rpf-wpf 0 0 + format_configure rpf-wpf 0 0 ARGB32 1024x768 $format + + vsp_runner rpf.0 --count=300 & + vsp_runner wpf.0 --count=300 --skip=297 + + local result=$(compare_frames) + + test_complete $result +} + +test_hw_pipe() { + local format + + for format in $formats ; do + test_extended_wpf_packing $format + done +} + +test_suspend_resume() { + local test_results=0 + local test_pid + + # Test the hardware each side of suspend resume + test_hw_pipe & + test_pid=$! + + # Make sure the pipeline has time to start + sleep 1 + + # Set the test mode + echo $mode > /sys/power/pm_test + + # Comence suspend + # The pm_test framework will automatically resume after 5 seconds + echo mem > /sys/power/state + + # Wait for the pipeline to complete + wait $test_pid +} + +test_main() { + local mode; + local suspend_test_failures + + # Check for pm-suspend test option + if [ ! -e /sys/power/pm_test ] ; then + echo "$0: Suspend Resume testing requires CONFIG_PM_DEBUG" + test_complete skip + return + fi; + + for mode in $suspend_modes ; do + test_suspend_resume $mode + done; +} + +test_init $0 "$features" +test_run -- 2.7.4
[PATCH 0/5] VSP-Tests: Add suspend resume tests and helpers
From: Kieran BinghamProvide two tests for suspend/resume cycles. One will verify the VSP1 is functional with a test before and after a suspend cycle. The other will maintain an active pipeline which must succeed despite a suspend resume cycle occuring in the middle of the test. Along side these tests are a couple of helpers that I have locally and thought they might be useful to others, so I'm posting for review. A test suite runner simplifies executing all vsp-unit tests, and provides the ability to easily repeat the test suite (for example to run an overnight longevity test). 'bin2png.sh' is a wrapper around the existing tools that helps convert the test files generated by VSP-Tests into png files for easy viewing. Finally, I have extended 'logger.sh' to also log to the FTrace buffer. As I have been making greater use of ftrace for register write capture, and driver flow - this addition helps separate multiple tests from the ftrace kernelshark view. Kieran Bingham (5): scripts: Test suite runner scripts: Provide bin2png.sh helper logger: Log to the FTrace buffer if tracing is enabled tests: Test suspend/resume on idle pipelines tests: Test suspend/resume on active pipelines scripts/Makefile| 2 +- scripts/bin2png.sh | 34 scripts/logger.sh | 13 ++- scripts/vsp-tests.sh| 46 ++ tests/vsp-unit-test-0019.sh | 84 tests/vsp-unit-test-0020.sh | 94 + 6 files changed, 271 insertions(+), 2 deletions(-) create mode 100755 scripts/bin2png.sh create mode 100755 scripts/vsp-tests.sh create mode 100755 tests/vsp-unit-test-0019.sh create mode 100755 tests/vsp-unit-test-0020.sh -- 2.7.4
Re: [PATCH] Media: platform: vsp1: - Do not forget to call
Hi Shailendra, Thank you for the patch. The subject line is missing something (and has an extra -), I would phrase it as "v4l: vsp1: Clean up file handle in open() error path" (Mauro's scripts will add the "[media]" prefix when applying, so there's no need to add it manually) The same comment holds for all other patches in this series. On Friday 25 Nov 2016 10:37:57 Shailendra Verma wrote: > v4l2_fh_init is already done.So call the v4l2_fh_exit in error condition > before returing from the function. > > Signed-off-by: Shailendra Verma> --- > drivers/media/platform/vsp1/vsp1_video.c |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/platform/vsp1/vsp1_video.c > b/drivers/media/platform/vsp1/vsp1_video.c index d351b9c..cc58163 100644 > --- a/drivers/media/platform/vsp1/vsp1_video.c > +++ b/drivers/media/platform/vsp1/vsp1_video.c > @@ -1044,6 +1044,7 @@ static int vsp1_video_open(struct file *file) > ret = vsp1_device_get(video->vsp1); > if (ret < 0) { > v4l2_fh_del(vfh); > + v4l2_fh_exit(vfh); > kfree(vfh); > } -- Regards, Laurent Pinchart
Re: [PATCH mmc/next] mmc: sh_mobile_sdhi: remove support for sh7372
On 24 November 2016 at 11:48, Simon Hormanwrote: > Remove documentation of support for the SH7372 (SH-Mobile AP4) from the MMC > driver. The driver itself appears to have no SH7372 specific code. > > Commit edf4100906044225 ("ARM: shmobile: sh7372 dtsi: Remove Legacy file") > removes this SoC from the kernel in v4.1. > > Signed-off-by: Simon Horman Thanks, applied for next! Kind regards Uffe > --- > Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > index 13df9c2399c3..1db9e74bb9c1 100644 > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > @@ -11,7 +11,6 @@ optional bindings can be used. > > Required properties: > - compatible: "renesas,sdhi-shmobile" - a generic sh-mobile SDHI unit > - "renesas,sdhi-sh7372" - SDHI IP on SH7372 SoC > "renesas,sdhi-sh73a0" - SDHI IP on SH73A0 SoC > "renesas,sdhi-r8a73a4" - SDHI IP on R8A73A4 SoC > "renesas,sdhi-r8a7740" - SDHI IP on R8A7740 SoC > -- > 2.7.0.rc3.207.g0ac5344 >
Re: [PATCH mmc/next] mmc: sh_mmcif: Document r8a73a4, r8a7778 and sh73a0 DT bindings
On 25 November 2016 at 08:56, Simon Hormanwrote: > Simply document new compatibility strings as the driver is already > activated using a fallback compatibility string. > > These compat strings are in keeping with those for all other > Renesas ARM based SoCs with sh_mmcif enabled in mainline. > > Signed-off-by: Simon Horman Thanks, applied for next! Kind regards Uffe > --- > Reposted with r8a7778 instead of r8a7779 in subject > > I have also posted patches to use these new compat strings > to bring the DT files of the SoCs in question in-line with those > for other Renesas ARM based SoCs with sh_mmcif enabled in mainline. > --- > Documentation/devicetree/bindings/mmc/renesas,mmcif.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt > b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt > index ff611fa66871..e4ba92aa035e 100644 > --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt > +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt > @@ -8,11 +8,14 @@ Required properties: > > - compatible: should be "renesas,mmcif-", "renesas,sh-mmcif" as a >fallback. Examples with are: > + - "renesas,mmcif-r8a73a4" for the MMCIF found in r8a73a4 SoCs > - "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs > + - "renesas,mmcif-r8a7778" for the MMCIF found in r8a7778 SoCs > - "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs > - "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs > - "renesas,mmcif-r8a7793" for the MMCIF found in r8a7793 SoCs > - "renesas,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs > + - "renesas,mmcif-sh73a0" for the MMCIF found in sh73a0 SoCs > > - clocks: reference to the functional clock > > -- > 2.7.0.rc3.207.g0ac5344 >
Re: [PATCH] devicetree: bindings: Add vendor prefix for Oki
On Fri, Nov 25, 2016 at 11:15:45AM +0100, Geert Uytterhoeven wrote: > Already in use for "oki,ml86v7667". > > Signed-off-by: Geert UytterhoevenReviewed-by: Simon Horman
RE: renesas-drivers-2016-11-22-v4.9-rc6
Hi Geert-san, > From: geert.uytterhoe...@gmail.com [mailto:geert.uytterhoe...@gmail.com] On > Behalf Of Geert Uytterhoeven > Sent: Friday, November 25, 2016 4:54 PM > > Hi Shimoda-san, > > On Fri, Nov 25, 2016 at 7:55 AM, Yoshihiro Shimoda >wrote: > >> As we're getting close to the opening of the merge window for v4.10, I > >> have also pushed renesas-drivers-next-2016-11-22-v4.9-rc6 to > >> https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git > > > > I don't know why, but I could not fetch this tag on my local repo. > > Morimoto-san is also the same. Do you know why? > > The following works fine for me: > > git fetch > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > renesas-drivers-next-2016-11-22-v4.9-rc6 Thank you for the reply. The command works file for me too! > What are you using? It doesn't work with the https URL above, as that one > is meant for your web brouwer. I did just "git fetch" on my local renesas-drivers.git repo. The local repo has other tags (e.g. renesas-drivers-2016-11-22-v4.9-rc6). And renesas-drivers-next-2016-11-22-v4.9-rc6 didn't exist. But, I tried "git fetch" now, and I could get the "next" tag ;) I don't know why, but thank you for your support! Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH renesas-next 1/3] ARM: dts: r8a73a4: Use SoC-specific compat string for mmcif
On Thu, Nov 24, 2016 at 9:15 PM, Simon Hormanwrote: > Use the SoC-specific compat string for mmcif in DT for the r8a73a4 SoC. > This is in keeping with the use of compat strings for mmcif for other > Renesas ARM based SoCs. > > Signed-off-by: Simon Horman Acked-by: Geert Uytterhoeven Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
[PATCH] devicetree: bindings: Add vendor prefix for Oki
Already in use for "oki,ml86v7667". Signed-off-by: Geert Uytterhoeven--- Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index dc7d5998d0abc632..2868be624c0e3f75 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -198,6 +198,7 @@ nuvoton Nuvoton Technology Corporation nvidia NVIDIA nxpNXP Semiconductors okaya Okaya Electric America, Inc. +okiOki Electric Industry Co., Ltd. olimex OLIMEX Ltd. onion Onion Corporation onnn ON Semiconductor Corp. -- 1.9.1
Re: [PATCH mmc/next] mmc: sh_mmcif: Document r8a73a4, r8a7778 and sh73a0 DT bindings
On Fri, Nov 25, 2016 at 8:56 AM, Simon Hormanwrote: > Simply document new compatibility strings as the driver is already > activated using a fallback compatibility string. > > These compat strings are in keeping with those for all other > Renesas ARM based SoCs with sh_mmcif enabled in mainline. > > Signed-off-by: Simon Horman Acked-by: Geert Uytterhoeven Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
Hi Magnus, (this time with CC kept) On Fri, Nov 25, 2016 at 9:16 AM, Magnus Dammwrote: > On Mon, Nov 21, 2016 at 8:09 PM, Geert Uytterhoeven > wrote: >> On Thu, Nov 17, 2016 at 9:51 AM, Magnus Damm wrote: >>> On Thu, Nov 17, 2016 at 5:31 PM, Geert Uytterhoeven >>> wrote: On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm wrote: > First of all, we might have slightly different view of the hardware, > so this might need some further discussions. Also, this topic in my > mind is mainly about DT integration code merge ordering for r8a7796 to > enable >4GB memory access early on without introducing any potential > issues. Note that >4GB memory is already enabled on r8a7796 by U-Boot. >>> >>> Right, can you remind me - did we get any conclusion about how to >>> handle the memory ranges in DTS and the ones from u-boot? >>> >>> It would be good with a consistent plan how to handle such. >> >> I think we should just apply "arm64: dts: r8a7796: salvator-x: Update memory >> node to 4 GiB map". > > I guess that we cannot control what u-boot will pass to us, but with > the patch above at least we have some chance of having a consistent > memory map. Indeed. >> IMHO it doesn't make much sense to pretend the memory is not present nor >> enabled. > > Enabling all the memory makes sense at this point, but I'd like us to > keep considering performance of bounce buffers and/or IPMMU when > merging different on-chip devices that may not support addressing of > the full physical memory space. > > We earlier had issues with "enable-and-forget" development approach in > case of USB host over on-chip PCI for the R-Car Gen2 family of > devices. In that case the hardware was unable to do bus mastering to > all the memory but this was not considered as part of the upstreaming > plan and instead came as a nasty surprise later on. So for each device > that we develop code for and integrate i would like to make sure that > we understand how memory handling is supposed to work and what > potential workarounds we may have to use. Sure. All existing devices in r8a7796.dtsi in Simon's tree either use PIO, or DMA through SYS-DMAC. The latter supports 64-bit addressing hardware-wise, but the software side needs a patch to enable that, cfr. "[PATCH/RFC 5/5] dmaengine: rcar-dmac: Widen DMA mask to 40 bits". Without that, it still works, but using swiotlb bounce buffers. Once that's fixed, there are no performance deteriorations due to bounce buffers, with the current set of enabled devices. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH renesas-next 2/3] ARM: dts: r8a7778: Use SoC-specific compat string for mmcif
On Thu, Nov 24, 2016 at 9:15 PM, Simon Hormanwrote: > Use the SoC-specific compat string for mmcif in DT for the r8a7778 SoC. > This is in keeping with the use of compat strings for mmcif for other > Renesas ARM based SoCs. > > Signed-off-by: Simon Horman Acked-by: Geert Uytterhoeven Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH/RFC 5/5] dmaengine: rcar-dmac: Widen DMA mask to 40 bits
Hi Geert, On Fri, Nov 25, 2016 at 6:00 PM, Geert Uytterhoevenwrote: > On Mon, Oct 31, 2016 at 6:08 PM, Geert Uytterhoeven > wrote: >> By default, the DMA mask covers only the low 32-bit address space, which >> causes SWIOTLB on arm64 to fall back to a bounce buffer for DMA >> transfers involving memory outside the 32-bit address space. >> >> The R-Car DMA controller hardware supports a 40-bit address space, hence >> widen the DMA mask to 40 bits to actually make use of this feature. >> >> Signed-off-by: Geert Uytterhoeven > > Any comments? Thanks! > >> --- >> drivers/dma/sh/rcar-dmac.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c >> index 2e441d0ccd79a37a..93a69b992a51a7aa 100644 >> --- a/drivers/dma/sh/rcar-dmac.c >> +++ b/drivers/dma/sh/rcar-dmac.c >> @@ -1716,6 +1716,7 @@ static int rcar_dmac_probe(struct platform_device >> *pdev) >> >> dmac->dev = >dev; >> platform_set_drvdata(pdev, dmac); >> + dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40)); This makes sense to me since the hardware and the driver both can access more than 32-bits of physical address space. Cheers, / magnus
Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
Hi Geert, On Mon, Nov 21, 2016 at 8:09 PM, Geert Uytterhoevenwrote: > Hi Magnus, > > On Thu, Nov 17, 2016 at 9:51 AM, Magnus Damm wrote: >> On Thu, Nov 17, 2016 at 5:31 PM, Geert Uytterhoeven >> wrote: >>> On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm wrote: First of all, we might have slightly different view of the hardware, so this might need some further discussions. Also, this topic in my mind is mainly about DT integration code merge ordering for r8a7796 to enable >4GB memory access early on without introducing any potential issues. >>> >>> Note that >4GB memory is already enabled on r8a7796 by U-Boot. >> >> Right, can you remind me - did we get any conclusion about how to >> handle the memory ranges in DTS and the ones from u-boot? >> >> It would be good with a consistent plan how to handle such. > > I think we should just apply "arm64: dts: r8a7796: salvator-x: Update memory > node to 4 GiB map". I guess that we cannot control what u-boot will pass to us, but with the patch above at least we have some chance of having a consistent memory map. > IMHO it doesn't make much sense to pretend the memory is not present nor > enabled. Enabling all the memory makes sense at this point, but I'd like us to keep considering performance of bounce buffers and/or IPMMU when merging different on-chip devices that may not support addressing of the full physical memory space. We earlier had issues with "enable-and-forget" development approach in case of USB host over on-chip PCI for the R-Car Gen2 family of devices. In that case the hardware was unable to do bus mastering to all the memory but this was not considered as part of the upstreaming plan and instead came as a nasty surprise later on. So for each device that we develop code for and integrate i would like to make sure that we understand how memory handling is supposed to work and what potential workarounds we may have to use. Thanks! / magnus
Re: ALSA analog audio loopback test tool (atest)
Hi Ulrich, On Thu, Nov 17, 2016 at 6:27 PM, Ulrich Hechtwrote: > On Thu, Nov 17, 2016 at 6:05 AM, Magnus Damm wrote: >> Thanks for your efforts! Quick question, the dependency on zlib seems >> to come from using adler32() for checksum comparison. In the code you >> seem to compare the total amount of data anyway, so I guess the >> checksum seems to be there to get some early notification about >> potential mismatch. The potential error will be discovered using the >> full-data-comparison later on anyway. I'm not sure if that's the way >> the code works, but if so then it should be easy to reduce the number >> of dependencies by simply dropping the checksum code. Does that make >> sense to you? > > The checksum is merely there because it's part of the Q15X25 packet > format. Not having to comply with any standard, we might as well drop > it. Thanks, I agree! FYI, last week I did some limited testing myself but I could not get your test app to work on my remote boards. Probably because the analogue loop back cables were missing! Since then I've seen some positive result on one local board with a cable attached, so I intend to get a bunch of analogue loop back cables for the different boards in the remote farm in the not so distant future. Cheers, / magnus