[Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device

Mark Cave-Ayland posted 12 patches 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
Test checkpatch failed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppcbe passed
Test ppcle passed
Test s390x passed
Makefile.objs                   |   1 +
default-configs/ppc-softmmu.mak |   1 +
hw/misc/Makefile.objs           |   3 +
hw/misc/macio/cuda.c            | 666 +++++++++++-----------------------------
hw/misc/macio/macio.c           |   3 +-
hw/misc/macio/trace-events      |  11 +
hw/misc/mos6522.c               | 505 ++++++++++++++++++++++++++++++
hw/misc/trace-events            |   7 +
hw/ppc/mac.h                    |  76 +----
include/hw/misc/macio/cuda.h    | 107 +++++++
include/hw/misc/mos6522.h       | 152 +++++++++
11 files changed, 963 insertions(+), 569 deletions(-)
create mode 100644 hw/misc/macio/trace-events
create mode 100644 hw/misc/mos6522.c
create mode 100644 include/hw/misc/macio/cuda.h
create mode 100644 include/hw/misc/mos6522.h
[Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device
Posted by Mark Cave-Ayland 6 years, 1 month ago
The Mac CUDA device (also known as via-cuda) consists of two parts: a 6522 VIA
acting as an interface chip and the CUDA device itself.

Currently there are at least 2 other upcoming Mac devices that include their
own 6522 VIA implementations: Ben's via-pmu device and Laurent's mac_via
device with their own hacks based upon the existing CUDA device. So rather than
keep duplicating this same functionality with slightly different hacks, let's
move the 6522 into its own separate device.

Patches 1 converts CUDA away from using old_mmio accesses, whilst patches 2-3
came about from studying Laurent and Ben's implementations (and appear to have
no ill effects in my testing so far).

Patches 4-8 do the majority of the work which is to isolate the hacks required
by MacOS into separate functions so that a separate 6522 device can operate in
a standard way, yet allow a CUDA implementation to hook into the right places.

Patch 9 introduces a new mos6522 device for the shared functionality with the
relevant hook points for delayed SR interrupt delivery and MacOS timer
calibration hacks implemented as device class methods.

Patch 10 switches CUDA over from using its own 6522 implementation over
to using an embedded mos6522 device, allowing the duplicate code to be removed.

Finally patches 11-12 perform some additional tidy-up by moving the CUDA
definitions into a new cuda.h file and converting CUDA over to use trace-events.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


v2:
- Add R-B tags from Laurent and Philippe
- Add David to CC as PPC maintainer and qemu-ppc for further comment
- Use DEVICE_BIG_ENDIAN in patch 1 as suggested by Laurent
- Rename *_INT constants in patch 9 to match those used in Ben's PMU patches
- Add patch 11 to move CUDA definitions into a new cuda.h file
- Add patch 12 to convert CUDA over to use trace-events


Mark Cave-Ayland (12):
  cuda: do not use old_mmio accesses
  cuda: don't allow writes to port output pins
  cuda: don't call cuda_update() when writing to ACR register
  cuda: introduce CUDAState parameter to get_counter()
  cuda: rename frequency property to tb_frequency
  cuda: minor cosmetic tidy-ups to get_next_irq_time()
  cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
  cuda: factor out timebase-derived counter value and load time
  misc: introduce new mos6522 VIA device and enable it for ppc builds
  cuda: convert to use the shared mos6522 device
  ppc: move CUDAState and other CUDA-related definitions into separate
    cuda.h file
  cuda: convert to trace-events

 Makefile.objs                   |   1 +
 default-configs/ppc-softmmu.mak |   1 +
 hw/misc/Makefile.objs           |   3 +
 hw/misc/macio/cuda.c            | 666 +++++++++++-----------------------------
 hw/misc/macio/macio.c           |   3 +-
 hw/misc/macio/trace-events      |  11 +
 hw/misc/mos6522.c               | 505 ++++++++++++++++++++++++++++++
 hw/misc/trace-events            |   7 +
 hw/ppc/mac.h                    |  76 +----
 include/hw/misc/macio/cuda.h    | 107 +++++++
 include/hw/misc/mos6522.h       | 152 +++++++++
 11 files changed, 963 insertions(+), 569 deletions(-)
 create mode 100644 hw/misc/macio/trace-events
 create mode 100644 hw/misc/mos6522.c
 create mode 100644 include/hw/misc/macio/cuda.h
 create mode 100644 include/hw/misc/mos6522.h

-- 
2.11.0


Re: [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device
Posted by no-reply@patchew.org 6 years, 1 month ago
Hi,

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

Type: series
Message-id: 20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
Subject: [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

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/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk -> patchew/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
Switched to a new branch 'test'
8ffc09de07 cuda: convert to trace-events
e2e6817bf3 ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file
a49d2800aa cuda: convert to use the shared mos6522 device
783d299115 misc: introduce new mos6522 VIA device and enable it for ppc builds
467b07e22d cuda: factor out timebase-derived counter value and load time
1bf294c5a4 cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
fdd2899c88 cuda: minor cosmetic tidy-ups to get_next_irq_time()
1b9a70ecf8 cuda: rename frequency property to tb_frequency
f80a0ed8ee cuda: introduce CUDAState parameter to get_counter()
1d814a15da cuda: don't call cuda_update() when writing to ACR register
c37f165538 cuda: don't allow writes to port output pins
167b6d50f8 cuda: do not use old_mmio accesses

=== OUTPUT BEGIN ===
Checking PATCH 1/12: cuda: do not use old_mmio accesses...
Checking PATCH 2/12: cuda: don't allow writes to port output pins...
Checking PATCH 3/12: cuda: don't call cuda_update() when writing to ACR register...
Checking PATCH 4/12: cuda: introduce CUDAState parameter to get_counter()...
Checking PATCH 5/12: cuda: rename frequency property to tb_frequency...
Checking PATCH 6/12: cuda: minor cosmetic tidy-ups to get_next_irq_time()...
Checking PATCH 7/12: cuda: set timer 1 frequency property to CUDA_TIMER_FREQ...
Checking PATCH 8/12: cuda: factor out timebase-derived counter value and load time...
Checking PATCH 9/12: misc: introduce new mos6522 VIA device and enable it for ppc builds...
Checking PATCH 10/12: cuda: convert to use the shared mos6522 device...
ERROR: do not use C99 // comments
#31: FILE: hw/misc/macio/cuda.c:36:
+//#define DEBUG_CUDA_PACKET

total: 1 errors, 0 warnings, 897 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.

Checking PATCH 11/12: ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file...
Checking PATCH 12/12: cuda: convert to trace-events...
=== 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
Re: [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device
Posted by Mark Cave-Ayland 6 years, 1 month ago
On 09/02/18 19:12, no-reply@patchew.org wrote:

> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
> Subject: [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> 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/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk -> patchew/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
> Switched to a new branch 'test'
> 8ffc09de07 cuda: convert to trace-events
> e2e6817bf3 ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file
> a49d2800aa cuda: convert to use the shared mos6522 device
> 783d299115 misc: introduce new mos6522 VIA device and enable it for ppc builds
> 467b07e22d cuda: factor out timebase-derived counter value and load time
> 1bf294c5a4 cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
> fdd2899c88 cuda: minor cosmetic tidy-ups to get_next_irq_time()
> 1b9a70ecf8 cuda: rename frequency property to tb_frequency
> f80a0ed8ee cuda: introduce CUDAState parameter to get_counter()
> 1d814a15da cuda: don't call cuda_update() when writing to ACR register
> c37f165538 cuda: don't allow writes to port output pins
> 167b6d50f8 cuda: do not use old_mmio accesses
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/12: cuda: do not use old_mmio accesses...
> Checking PATCH 2/12: cuda: don't allow writes to port output pins...
> Checking PATCH 3/12: cuda: don't call cuda_update() when writing to ACR register...
> Checking PATCH 4/12: cuda: introduce CUDAState parameter to get_counter()...
> Checking PATCH 5/12: cuda: rename frequency property to tb_frequency...
> Checking PATCH 6/12: cuda: minor cosmetic tidy-ups to get_next_irq_time()...
> Checking PATCH 7/12: cuda: set timer 1 frequency property to CUDA_TIMER_FREQ...
> Checking PATCH 8/12: cuda: factor out timebase-derived counter value and load time...
> Checking PATCH 9/12: misc: introduce new mos6522 VIA device and enable it for ppc builds...
> Checking PATCH 10/12: cuda: convert to use the shared mos6522 device...
> ERROR: do not use C99 // comments
> #31: FILE: hw/misc/macio/cuda.c:36:
> +//#define DEBUG_CUDA_PACKET
> 
> total: 1 errors, 0 warnings, 897 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.
> 
> Checking PATCH 11/12: ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file...
> Checking PATCH 12/12: cuda: convert to trace-events...
> === OUTPUT END ===
> 
> Test command exited with code: 1

As before, this is a false positive (especially since v2 patch 12 
converts CUDA to use trace-events and so these lines disappear anyhow).


ATB,

Mark.