Message ID | 1485352137-29367-1-git-send-email-w.bumiller@proxmox.com |
---|---|
State | New |
Headers | show |
Hi, Your series seems to have some coding style problems. See output below for more information: Type: series Subject: [Qemu-devel] [PATCH v2] cirrus: handle negative pitch in cirrus_invalidate_region() Message-id: 1485352137-29367-1-git-send-email-w.bumiller@proxmox.com === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 # Useful git options git config --local diff.renamelimit 0 git config --local diff.renames True commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/1485352082-16830-1-git-send-email-groug@kaod.org -> patchew/1485352082-16830-1-git-send-email-groug@kaod.org * [new tag] patchew/1485352137-29367-1-git-send-email-w.bumiller@proxmox.com -> patchew/1485352137-29367-1-git-send-email-w.bumiller@proxmox.com - [tag update] patchew/20170124110151.937-1-berrange@redhat.com -> patchew/20170124110151.937-1-berrange@redhat.com - [tag update] patchew/20170124180420.12430-1-pbonzini@redhat.com -> patchew/20170124180420.12430-1-pbonzini@redhat.com Switched to a new branch 'test' 93e5129 cirrus: handle negative pitch in cirrus_invalidate_region() === OUTPUT BEGIN === Checking PATCH 1/1: cirrus: handle negative pitch in cirrus_invalidate_region()... ERROR: spaces required around that '-' (ctx:VxV) #31: FILE: hw/display/cirrus_vga.c:665: + off_begin -= bytesperline-1; ^ ERROR: code indent should never use tabs #37: FILE: hw/display/cirrus_vga.c:671: +^Iassert(off_cur_end >= off_cur);$ total: 2 errors, 0 warnings, 14 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@freelists.org
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index b1a0773..755110f 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -670,9 +670,14 @@ static void cirrus_invalidate_region(CirrusVGAState * s, int off_begin, int off_cur; int off_cur_end; + if (off_pitch < 0) { + off_begin -= bytesperline-1; + } + for (y = 0; y < lines; y++) { off_cur = off_begin; off_cur_end = (off_cur + bytesperline) & s->cirrus_addr_mask; + assert(off_cur_end >= off_cur); memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - off_cur); off_begin += off_pitch; }
cirrus_invalidate_region() calls memory_region_set_dirty() on a per-line basis, always ranging from off_begin to off_begin+bytesperline. With a negative pitch off_begin marks the top most used address and thus we need to do an initial shift backwards by a line for negative pitches of backward blits, otherwise the first iteration covers the line going from the start offset forwards instead of backwards. Additionally since the start address is inclusive, if we shift by a full `bytesperline` we move to the first address *not* included in the blit, so we only shift by one less than bytesperline. Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> --- Changes to v1: * Subtract only bytesperline-1 since the original address should be included while the byte at `begin-bytesperline` is exclusive. * Added an assertion to ensure we don't call memory_region_set_dirty() with a negative size since it takes unsigned parameters. @Gerd: Still unclear: is -1 enough or should this somehow deal with multiple depths? As far as I can see the backward blit functions are all only byte based, as opposed to the patternfill functions in cirrus_vga_rop2.h which have PUTPIXEL() macros for 8/16/24/32 bpp. CIRRUS_BLTMODE_BACKWARDS only references cirrus_bkwd_rop[16] and cirrus_bkwd_transp_rop[16][2] which only contain the bkwd rops from cirrus_vga_rop.h. hw/display/cirrus_vga.c | 5 +++++ 1 file changed, 5 insertions(+)