[PULL 00/21] Vga 20200528 patches

Gerd Hoffmann posted 21 patches 3 years, 11 months ago
Test docker-mingw@fedora passed
Test checkpatch failed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200528123609.27362-1-kraxel@redhat.com
Maintainers: BALATON Zoltan <balaton@eik.bme.hu>
include/hw/display/edid.h    |   1 +
hw/display/cg3.c             |  14 +-
hw/display/cirrus_vga.c      | 119 ++++++-------
hw/display/dpcd.c            |  20 +--
hw/display/exynos4210_fimd.c |  46 +++--
hw/display/omap_dss.c        |   2 +-
hw/display/pxa2xx_lcd.c      |  26 +--
hw/display/sm501.c           | 313 ++++++++++++++++++-----------------
hw/display/vmware_vga.c      |  18 +-
hw/display/xlnx_dp.c         |  14 +-
hw/display/trace-events      |  10 ++
11 files changed, 301 insertions(+), 282 deletions(-)
[PULL 00/21] Vga 20200528 patches
Posted by Gerd Hoffmann 3 years, 11 months ago
The following changes since commit 06539ebc76b8625587aa78d646a9d8d5fddf84f3:

  Merge remote-tracking branch 'remotes/philmd-gitlab/tags/mips-hw-next-20200526' into staging (2020-05-26 20:25:06 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/vga-20200528-pull-request

for you to fetch changes up to fa0013a1bc5f6011a1017e0e655740403e5555d9:

  sm501: Remove obsolete changelog and todo comment (2020-05-28 11:38:57 +0200)

----------------------------------------------------------------
hw/dispaly/sm501: bugfixes, add sanity checks.
hw/display: use tracepoints, misc cleanups.

----------------------------------------------------------------

BALATON Zoltan (7):
  sm501: Convert printf + abort to qemu_log_mask
  sm501: Shorten long variable names in sm501_2d_operation
  sm501: Use BIT(x) macro to shorten constant
  sm501: Clean up local variables in sm501_2d_operation
  sm501: Replace hand written implementation with pixman where possible
  sm501: Optimize small overlapping blits
  sm501: Remove obsolete changelog and todo comment

Philippe Mathieu-Daudé (14):
  hw/display/edid: Add missing 'qdev-properties.h' header
  hw/display/cg3: Convert debug printf()s to trace events
  hw/display/cirrus_vga: Convert debug printf() to trace event
  hw/display/cirrus_vga: Use qemu_log_mask(UNIMP) instead of debug
    printf
  hw/display/cirrus_vga: Use qemu_log_mask(ERROR) instead of debug
    printf
  hw/display/cirrus_vga: Convert debug printf() to trace event
  hw/display/dpcd: Fix memory region size
  hw/display/dpcd: Convert debug printf()s to trace events
  hw/display/xlnx_dp: Replace disabled DPRINTF() by error_report()
  hw/display/vmware_vga: Replace printf() calls by qemu_log_mask(ERROR)
  hw/display/vmware_vga: Let the PCI device own its I/O MemoryRegion
  hw/display/exynos4210_fimd: Use qemu_log_mask(GUEST_ERROR)
  hw/display/omap_dss: Replace fprintf() call by
    qemu_log_mask(LOG_UNIMP)
  hw/display/pxa2xx_lcd: Replace printf() call by qemu_log_mask()

 include/hw/display/edid.h    |   1 +
 hw/display/cg3.c             |  14 +-
 hw/display/cirrus_vga.c      | 119 ++++++-------
 hw/display/dpcd.c            |  20 +--
 hw/display/exynos4210_fimd.c |  46 +++--
 hw/display/omap_dss.c        |   2 +-
 hw/display/pxa2xx_lcd.c      |  26 +--
 hw/display/sm501.c           | 313 ++++++++++++++++++-----------------
 hw/display/vmware_vga.c      |  18 +-
 hw/display/xlnx_dp.c         |  14 +-
 hw/display/trace-events      |  10 ++
 11 files changed, 301 insertions(+), 282 deletions(-)

-- 
2.18.4


Re: [PULL 00/21] Vga 20200528 patches
Posted by no-reply@patchew.org 3 years, 11 months ago
Patchew URL: https://patchew.org/QEMU/20200528123609.27362-1-kraxel@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200528123609.27362-1-kraxel@redhat.com
Subject: [PULL 00/21] Vga 20200528 patches
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200528153742.274164-1-kwolf@redhat.com -> patchew/20200528153742.274164-1-kwolf@redhat.com
Switched to a new branch 'test'
9f2de8c sm501: Remove obsolete changelog and todo comment
40c6179 sm501: Optimize small overlapping blits
a57f549 sm501: Replace hand written implementation with pixman where possible
757eb94 sm501: Clean up local variables in sm501_2d_operation
8d8e3be sm501: Use BIT(x) macro to shorten constant
e7baa0b sm501: Shorten long variable names in sm501_2d_operation
b038590 sm501: Convert printf + abort to qemu_log_mask
a524b11 hw/display/pxa2xx_lcd: Replace printf() call by qemu_log_mask()
41000f1 hw/display/omap_dss: Replace fprintf() call by qemu_log_mask(LOG_UNIMP)
dad32f3 hw/display/exynos4210_fimd: Use qemu_log_mask(GUEST_ERROR)
98b6d1b hw/display/vmware_vga: Let the PCI device own its I/O MemoryRegion
f1eadac hw/display/vmware_vga: Replace printf() calls by qemu_log_mask(ERROR)
6ad1e39 hw/display/xlnx_dp: Replace disabled DPRINTF() by error_report()
3cffc59 hw/display/dpcd: Convert debug printf()s to trace events
e359084 hw/display/dpcd: Fix memory region size
0e19973 hw/display/cirrus_vga: Convert debug printf() to trace event
97f369f hw/display/cirrus_vga: Use qemu_log_mask(ERROR) instead of debug printf
0758566 hw/display/cirrus_vga: Use qemu_log_mask(UNIMP) instead of debug printf
46f4782 hw/display/cirrus_vga: Convert debug printf() to trace event
4e2d47f hw/display/cg3: Convert debug printf()s to trace events
60a4146 hw/display/edid: Add missing 'qdev-properties.h' header

=== OUTPUT BEGIN ===
1/21 Checking commit 60a4146dff34 (hw/display/edid: Add missing 'qdev-properties.h' header)
2/21 Checking commit 4e2d47f82c1e (hw/display/cg3: Convert debug printf()s to trace events)
3/21 Checking commit 46f47822efea (hw/display/cirrus_vga: Convert debug printf() to trace event)
4/21 Checking commit 0758566b5a09 (hw/display/cirrus_vga: Use qemu_log_mask(UNIMP) instead of debug printf)
5/21 Checking commit 97f369f2479d (hw/display/cirrus_vga: Use qemu_log_mask(ERROR) instead of debug printf)
ERROR: suspect code indent for conditional statements (16, 12)
#34: FILE: hw/display/cirrus_vga.c:1038:
                if (s->cirrus_blt_pixelwidth > 2) {
+            qemu_log_mask(LOG_GUEST_ERROR,

total: 1 errors, 0 warnings, 156 lines checked

Patch 5/21 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/21 Checking commit 0e199736615e (hw/display/cirrus_vga: Convert debug printf() to trace event)
7/21 Checking commit e3590842cc63 (hw/display/dpcd: Fix memory region size)
8/21 Checking commit 3cffc590ddc1 (hw/display/dpcd: Convert debug printf()s to trace events)
9/21 Checking commit 6ad1e398b3a1 (hw/display/xlnx_dp: Replace disabled DPRINTF() by error_report())
10/21 Checking commit f1eadacbf17b (hw/display/vmware_vga: Replace printf() calls by qemu_log_mask(ERROR))
11/21 Checking commit 98b6d1b284b1 (hw/display/vmware_vga: Let the PCI device own its I/O MemoryRegion)
12/21 Checking commit dad32f3b2d7a (hw/display/exynos4210_fimd: Use qemu_log_mask(GUEST_ERROR))
13/21 Checking commit 41000f19872c (hw/display/omap_dss: Replace fprintf() call by qemu_log_mask(LOG_UNIMP))
14/21 Checking commit a524b11a2cb3 (hw/display/pxa2xx_lcd: Replace printf() call by qemu_log_mask())
15/21 Checking commit b03859016467 (sm501: Convert printf + abort to qemu_log_mask)
16/21 Checking commit e7baa0b4bdde (sm501: Shorten long variable names in sm501_2d_operation)
17/21 Checking commit 8d8e3be40796 (sm501: Use BIT(x) macro to shorten constant)
18/21 Checking commit 757eb94bd007 (sm501: Clean up local variables in sm501_2d_operation)
19/21 Checking commit a57f549846a1 (sm501: Replace hand written implementation with pixman where possible)
20/21 Checking commit 40c6179925f0 (sm501: Optimize small overlapping blits)
21/21 Checking commit 9f2de8c702a3 (sm501: Remove obsolete changelog and todo comment)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200528123609.27362-1-kraxel@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PULL 00/21] Vga 20200528 patches
Posted by Peter Maydell 3 years, 11 months ago
On Thu, 28 May 2020 at 13:36, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit 06539ebc76b8625587aa78d646a9d8d5fddf84f3:
>
>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/mips-hw-next-20200526' into staging (2020-05-26 20:25:06 +0100)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/vga-20200528-pull-request
>
> for you to fetch changes up to fa0013a1bc5f6011a1017e0e655740403e5555d9:
>
>   sm501: Remove obsolete changelog and todo comment (2020-05-28 11:38:57 +0200)
>
> ----------------------------------------------------------------
> hw/dispaly/sm501: bugfixes, add sanity checks.
> hw/display: use tracepoints, misc cleanups.
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

Could somebody send a followup patch to fix the indentation
error checkpatch notices, please?

5/21 Checking commit 97f369f2479d (hw/display/cirrus_vga: Use
qemu_log_mask(ERROR) instead of debug printf)
ERROR: suspect code indent for conditional statements (16, 12)
#34: FILE: hw/display/cirrus_vga.c:1038:
                if (s->cirrus_blt_pixelwidth > 2) {
+            qemu_log_mask(LOG_GUEST_ERROR,

-- PMM

Re: [PULL 00/21] Vga 20200528 patches
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
Hi Peter,

On 5/29/20 12:29 PM, Peter Maydell wrote:
> On Thu, 28 May 2020 at 13:36, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>> The following changes since commit 06539ebc76b8625587aa78d646a9d8d5fddf84f3:
>>
>>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/mips-hw-next-20200526' into staging (2020-05-26 20:25:06 +0100)
>>
>> are available in the Git repository at:
>>
>>   git://git.kraxel.org/qemu tags/vga-20200528-pull-request
>>
>> for you to fetch changes up to fa0013a1bc5f6011a1017e0e655740403e5555d9:
>>
>>   sm501: Remove obsolete changelog and todo comment (2020-05-28 11:38:57 +0200)
>>
>> ----------------------------------------------------------------
>> hw/dispaly/sm501: bugfixes, add sanity checks.
>> hw/display: use tracepoints, misc cleanups.
>>
> 
> 
> Applied, thanks.
> 
> Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
> for any user-visible changes.
> 
> Could somebody send a followup patch to fix the indentation
> error checkpatch notices, please?

If this is part of your scripts, this is a nice feature :)

> 
> 5/21 Checking commit 97f369f2479d (hw/display/cirrus_vga: Use
> qemu_log_mask(ERROR) instead of debug printf)
> ERROR: suspect code indent for conditional statements (16, 12)
> #34: FILE: hw/display/cirrus_vga.c:1038:
>                 if (s->cirrus_blt_pixelwidth > 2) {
> +            qemu_log_mask(LOG_GUEST_ERROR,

I explained on the patches:

  False positive.
  Checkpatch is confused by the mis-indented code
  previous to this line.

https://www.mail-archive.com/qemu-devel@nongnu.org/msg706364.html

> 
> -- PMM
> 


Re: [PULL 00/21] Vga 20200528 patches
Posted by Peter Maydell 3 years, 11 months ago
On Fri, 29 May 2020 at 17:15, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 5/29/20 12:29 PM, Peter Maydell wrote:
> > Could somebody send a followup patch to fix the indentation
> > error checkpatch notices, please?
>
> If this is part of your scripts, this is a nice feature :)

No, I just noticed the patchew email.

> >
> > 5/21 Checking commit 97f369f2479d (hw/display/cirrus_vga: Use
> > qemu_log_mask(ERROR) instead of debug printf)
> > ERROR: suspect code indent for conditional statements (16, 12)
> > #34: FILE: hw/display/cirrus_vga.c:1038:
> >                 if (s->cirrus_blt_pixelwidth > 2) {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
>
> I explained on the patches:
>
>   False positive.

The code is
            if (s->cirrus_blt_mode & CIRRUS_BLTMODE_TRANSPARENTCOMP) {
                if (s->cirrus_blt_pixelwidth > 2) {
            qemu_log_mask(LOG_GUEST_ERROR,
                          "cirrus: src transparent without colorexpand "
                          "must be 8bpp or 16bpp\n");
                    goto bitblt_ignore;
                }

checkpatch seems correct; the qemu_log_mask line is misindented,
and looking at the commit this is a misindent introduced in
commit 2b55f4d3504a9f34 "hw/display/cirrus_vga: Use
qemu_log_mask(ERROR) instead of debug printf". The old
fprintf() line was using indent of tab+tab+4 spaces, but
the new qemu_log_mask line is indented by 12 spaces, not 20.
(Tabs are always 8 spaces equivalent.)

Some days I wonder whether we should just do a bulk detabify
of the QEMU sources.

thanks
-- PMM

Re: [PULL 00/21] Vga 20200528 patches
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 5/29/20 6:36 PM, Peter Maydell wrote:
> On Fri, 29 May 2020 at 17:15, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> On 5/29/20 12:29 PM, Peter Maydell wrote:
>>> Could somebody send a followup patch to fix the indentation
>>> error checkpatch notices, please?
>>
>> If this is part of your scripts, this is a nice feature :)
> 
> No, I just noticed the patchew email.
> 
>>>
>>> 5/21 Checking commit 97f369f2479d (hw/display/cirrus_vga: Use
>>> qemu_log_mask(ERROR) instead of debug printf)
>>> ERROR: suspect code indent for conditional statements (16, 12)
>>> #34: FILE: hw/display/cirrus_vga.c:1038:
>>>                 if (s->cirrus_blt_pixelwidth > 2) {
>>> +            qemu_log_mask(LOG_GUEST_ERROR,
>>
>> I explained on the patches:
>>
>>   False positive.
> 
> The code is
>             if (s->cirrus_blt_mode & CIRRUS_BLTMODE_TRANSPARENTCOMP) {
>                 if (s->cirrus_blt_pixelwidth > 2) {
>             qemu_log_mask(LOG_GUEST_ERROR,
>                           "cirrus: src transparent without colorexpand "
>                           "must be 8bpp or 16bpp\n");
>                     goto bitblt_ignore;
>                 }
> 
> checkpatch seems correct; the qemu_log_mask line is misindented,
> and looking at the commit this is a misindent introduced in
> commit 2b55f4d3504a9f34 "hw/display/cirrus_vga: Use
> qemu_log_mask(ERROR) instead of debug printf". The old
> fprintf() line was using indent of tab+tab+4 spaces, but
> the new qemu_log_mask line is indented by 12 spaces, not 20.
> (Tabs are always 8 spaces equivalent.)

OK now I understand, I use "set ts=4 sw=4" in my .vimrc and see this
file completely un-indented (and the qemu_log_mask call well placed).

I'll send a cleanup patch. Sorry and thanks for noticing this.

> 
> Some days I wonder whether we should just do a bulk detabify
> of the QEMU sources.
> 
> thanks
> -- PMM
> 

Re: [PULL 00/21] Vga 20200528 patches
Posted by Gerd Hoffmann 3 years, 11 months ago
  Hi,

> Some days I wonder whether we should just do a bulk detabify
> of the QEMU sources.

git & patch utils have switches to ignore whitespace changes, so I'd
expect such a bulk change shouldn't be too disruptive in terms of
conflicts.  A one-time "git rebase --ignore-whitespace" for WIP patches
should handle it.

cheers,
  Gerd