qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V2 6/7] Introduce xilinx dp.


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH V2 6/7] Introduce xilinx dp.
Date: Wed, 24 Jun 2015 01:21:28 -0700

On Mon, Jun 15, 2015 at 8:15 AM,  <address@hidden> wrote:
> From: KONRAD Frederic <address@hidden>
>
> This is the implementation of the DisplayPort.
>
> It has an aux-bus to access dpcd and edid needed for the driver to complete.
>

No need to reference the driver.

> Graphic plane is connected to the channel 3.
> Video plane is connected to the channel 0.
> Audio stream are connected to the channels 4 and 5.
>
> Signed-off-by: KONRAD Frederic <address@hidden>
> ---
>  hw/display/Makefile.objs |    2 +-
>  hw/display/xilinx_dp.c   | 1427 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  hw/display/xilinx_dp.h   |  129 +++++
>  3 files changed, 1557 insertions(+), 1 deletion(-)
>  create mode 100644 hw/display/xilinx_dp.c
>  create mode 100644 hw/display/xilinx_dp.h
>
> diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
> index f75094f..f418dbc 100644
> --- a/hw/display/Makefile.objs
> +++ b/hw/display/Makefile.objs
> @@ -36,4 +36,4 @@ obj-$(CONFIG_VGA) += vga.o
>  common-obj-$(CONFIG_QXL) += qxl.o qxl-logger.o qxl-render.o
>
>  obj-$(CONFIG_VIRTIO) += virtio-gpu.o
> -obj-$(CONFIG_XLNX_ZYNQMP) += dpcd.o
> +obj-$(CONFIG_XLNX_ZYNQMP) += dpcd.o xilinx_dp.o

Give each file its own line. It makes Makefile.objs diffs easier reading.

Same comments as before about name stems and filenames.

> diff --git a/hw/display/xilinx_dp.c b/hw/display/xilinx_dp.c
> new file mode 100644
> index 0000000..cf48d8b
> --- /dev/null
> +++ b/hw/display/xilinx_dp.c
> @@ -0,0 +1,1427 @@
> +/*
> + * xilinx_dp.c
> + *
> + *  Copyright (C) 2015 : GreenSocs Ltd
> + *      http://www.greensocs.com/ , email: address@hidden
> + *
> + *  Developed by :
> + *  Frederic Konrad   <address@hidden>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option)any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "xilinx_dp.h"
> +
> +#ifndef DEBUG_DP
> +#define DEBUG_DP 0
> +#endif
> +
> +#define DPRINTF(fmt, ...) do {                                               
>   \
> +    if (DEBUG_DP) {                                                          
>   \
> +        qemu_log("xilinx_dp: " fmt , ## __VA_ARGS__);                        
>   \
> +    }                                                                        
>   \
> +} while (0);
> +
> +/*
> + * Register offset for DP.
> + */

Blank lines between logical groupings.

> +#define DP_LINK_BW_SET                      (0x0000 >> 2)
> +#define DP_LANE_COUNT_SET                   (0x0004 >> 2)
> +#define DP_ENHANCED_FRAME_EN                (0x0008 >> 2)
> +#define DP_TRAINING_PATTERN_SET             (0x000C >> 2)
> +#define DP_LINK_QUAL_PATTERN_SET            (0x0010 >> 2)
> +#define DP_SCRAMBLING_DISABLE               (0x0014 >> 2)
> +#define DP_DOWNSPREAD_CTRL                  (0x0018 >> 2)
> +#define DP_SOFTWARE_RESET                   (0x001C >> 2)
> +#define DP_TRANSMITTER_ENABLE               (0x0080 >> 2)
> +#define DP_MAIN_STREAM_ENABLE               (0x0084 >> 2)
> +#define DP_FORCE_SCRAMBLER_RESET            (0x00C0 >> 2)
> +#define DP_VERSION_REGISTER                 (0x00F8 >> 2)
> +#define DP_CORE_ID                          (0x00FC >> 2)
> +#define DP_AUX_COMMAND_REGISTER             (0x0100 >> 2)
> +#define AUX_ADDR_ONLY_MASK                  (0x1000)
> +#define AUX_COMMAND_MASK                    (0x0F00)
> +#define AUX_COMMAND_SHIFT                   (8)
> +#define AUX_COMMAND_NBYTES                  (0x000F)
> +#define DP_AUX_WRITE_FIFO                   (0x0104 >> 2)
> +#define DP_AUX_ADDRESS                      (0x0108 >> 2)
> +#define DP_AUX_CLOCK_DIVIDER                (0x010C >> 2)
> +#define DP_TX_USER_FIFO_OVERFLOW            (0x0110 >> 2)
> +#define DP_INTERRUPT_SIGNAL_STATE           (0x0130 >> 2)
> +#define DP_AUX_REPLY_DATA                   (0x0134 >> 2)
> +#define DP_AUX_REPLY_CODE                   (0x0138 >> 2)
> +#define DP_AUX_REPLY_COUNT                  (0x013C >> 2)
> +#define DP_REPLY_DATA_COUNT                 (0x0148 >> 2)
> +#define DP_REPLY_STATUS                     (0x014C >> 2)
> +#define DP_HPD_DURATION                     (0x0150 >> 2)
> +#define DP_MAIN_STREAM_HTOTAL               (0x0180 >> 2)
> +#define DP_MAIN_STREAM_VTOTAL               (0x0184 >> 2)
> +#define DP_MAIN_STREAM_POLARITY             (0x0188 >> 2)
> +#define DP_MAIN_STREAM_HSWIDTH              (0x018C >> 2)
> +#define DP_MAIN_STREAM_VSWIDTH              (0x0190 >> 2)
> +#define DP_MAIN_STREAM_HRES                 (0x0194 >> 2)
> +#define DP_MAIN_STREAM_VRES                 (0x0198 >> 2)
> +#define DP_MAIN_STREAM_HSTART               (0x019C >> 2)
> +#define DP_MAIN_STREAM_VSTART               (0x01A0 >> 2)
> +#define DP_MAIN_STREAM_MISC0                (0x01A4 >> 2)
> +#define DP_MAIN_STREAM_MISC1                (0x01A8 >> 2)
> +#define DP_MAIN_STREAM_M_VID                (0x01AC >> 2)
> +#define DP_MSA_TRANSFER_UNIT_SIZE           (0x01B0 >> 2)
> +#define DP_MAIN_STREAM_N_VID                (0x01B4 >> 2)
> +#define DP_USER_DATA_COUNT_PER_LANE         (0x01BC >> 2)
> +#define DP_MIN_BYTES_PER_TU                 (0x01C4 >> 2)
> +#define DP_FRAC_BYTES_PER_TU                (0x01C8 >> 2)
> +#define DP_INIT_WAIT                        (0x01CC >> 2)
> +#define DP_PHY_RESET                        (0x0200 >> 2)
> +#define DP_PHY_VOLTAGE_DIFF_LANE_0          (0x0220 >> 2)
> +#define DP_PHY_VOLTAGE_DIFF_LANE_1          (0x0224 >> 2)
> +#define DP_TRANSMIT_PRBS7                   (0x0230 >> 2)
> +#define DP_PHY_CLOCK_SELECT                 (0x0234 >> 2)
> +#define DP_TX_PHY_POWER_DOWN                (0x0238 >> 2)
> +#define DP_PHY_PRECURSOR_LANE_0             (0x023C >> 2)
> +#define DP_PHY_PRECURSOR_LANE_1             (0x0240 >> 2)
> +#define DP_PHY_POSTCURSOR_LANE_0            (0x024C >> 2)
> +#define DP_PHY_POSTCURSOR_LANE_1            (0x0250 >> 2)
> +#define DP_PHY_STATUS                       (0x0280 >> 2)
> +#define DP_TX_AUDIO_CONTROL                 (0x0300 >> 2)
> +#define DP_TX_AUDIO_CHANNELS                (0x0304 >> 2)
> +#define DP_TX_AUDIO_INFO_DATA0              (0x0308 >> 2)
> +#define DP_TX_AUDIO_INFO_DATA1              (0x030C >> 2)
> +#define DP_TX_AUDIO_INFO_DATA2              (0x0310 >> 2)
> +#define DP_TX_AUDIO_INFO_DATA3              (0x0314 >> 2)
> +#define DP_TX_AUDIO_INFO_DATA4              (0x0318 >> 2)
> +#define DP_TX_AUDIO_INFO_DATA5              (0x031C >> 2)
> +#define DP_TX_AUDIO_INFO_DATA6              (0x0320 >> 2)
> +#define DP_TX_AUDIO_INFO_DATA7              (0x0324 >> 2)

Define once and index in on N.

> +#define DP_TX_M_AUD                         (0x0328 >> 2)
> +#define DP_TX_N_AUD                         (0x032C >> 2)
> +#define DP_TX_AUDIO_EXT_DATA0               (0x0330 >> 2)
> +#define DP_TX_AUDIO_EXT_DATA1               (0x0334 >> 2)
> +#define DP_TX_AUDIO_EXT_DATA2               (0x0338 >> 2)
> +#define DP_TX_AUDIO_EXT_DATA3               (0x033C >> 2)
> +#define DP_TX_AUDIO_EXT_DATA4               (0x0340 >> 2)
> +#define DP_TX_AUDIO_EXT_DATA5               (0x0344 >> 2)
> +#define DP_TX_AUDIO_EXT_DATA6               (0x0348 >> 2)
> +#define DP_TX_AUDIO_EXT_DATA7               (0x034C >> 2)
> +#define DP_TX_AUDIO_EXT_DATA8               (0x0350 >> 2)

Index.

> +#define DP_INT_STATUS                       (0x03A0 >> 2)
> +#define DP_INT_MASK                         (0x03A4 >> 2)
> +#define DP_INT_EN                           (0x03A8 >> 2)
> +#define DP_INT_DS                           (0x03AC >> 2)
> +
> +/*
> + * Registers offset for Audio Video Buffer configuration.
> + */
> +#define V_BLEND_OFFSET                      (0xA000)
> +#define V_BLEND_BG_CLR_0                    (0x0000 >> 2)
> +#define V_BLEND_BG_CLR_1                    (0x0004 >> 2)
> +#define V_BLEND_BG_CLR_2                    (0x0008 >> 2)
> +#define V_BLEND_SET_GLOBAL_ALPHA_REG        (0x000C >> 2)
> +#define V_BLEND_OUTPUT_VID_FORMAT           (0x0014 >> 2)
> +#define V_BLEND_LAYER0_CONTROL              (0x0018 >> 2)
> +#define V_BLEND_LAYER1_CONTROL              (0x001C >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF0            (0x0020 >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF1            (0x0024 >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF2            (0x0028 >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF3            (0x002C >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF4            (0x0030 >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF5            (0x0034 >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF6            (0x0038 >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF7            (0x003C >> 2)
> +#define V_BLEND_RGB2YCBCR_COEFF8            (0x0040 >> 2)
> +#define V_BLEND_IN1CSC_COEFF0               (0x0044 >> 2)
> +#define V_BLEND_IN1CSC_COEFF1               (0x0048 >> 2)
> +#define V_BLEND_IN1CSC_COEFF2               (0x004C >> 2)
> +#define V_BLEND_IN1CSC_COEFF3               (0x0050 >> 2)
> +#define V_BLEND_IN1CSC_COEFF4               (0x0054 >> 2)
> +#define V_BLEND_IN1CSC_COEFF5               (0x0058 >> 2)
> +#define V_BLEND_IN1CSC_COEFF6               (0x005C >> 2)
> +#define V_BLEND_IN1CSC_COEFF7               (0x0060 >> 2)
> +#define V_BLEND_IN1CSC_COEF8               (0x0064 >> 2)

Index.

Missing "F" on #8?

> +#define V_BLEND_LUMA_IN1CSC_OFFSET          (0x0068 >> 2)
> +#define V_BLEND_CR_IN1CSC_OFFSET            (0x006C >> 2)
> +#define V_BLEND_CB_IN1CSC_OFFSET            (0x0070 >> 2)
> +#define V_BLEND_LUMA_OUTCSC_OFFSET          (0x0074 >> 2)
> +#define V_BLEND_CR_OUTCSC_OFFSET            (0x0078 >> 2)
> +#define V_BLEND_CB_OUTCSC_OFFSET            (0x007C >> 2)
> +#define V_BLEND_IN2CSC_COEFF0               (0x0080 >> 2)
> +#define V_BLEND_IN2CSC_COEFF1               (0x0084 >> 2)
> +#define V_BLEND_IN2CSC_COEFF2               (0x0088 >> 2)
> +#define V_BLEND_IN2CSC_COEFF3               (0x008C >> 2)
> +#define V_BLEND_IN2CSC_COEFF4               (0x0090 >> 2)
> +#define V_BLEND_IN2CSC_COEFF5               (0x0094 >> 2)
> +#define V_BLEND_IN2CSC_COEFF6               (0x0098 >> 2)
> +#define V_BLEND_IN2CSC_COEFF7               (0x009C >> 2)
> +#define V_BLEND_IN2CSC_COEFF8               (0x00A0 >> 2)

Index.

> +#define V_BLEND_LUMA_IN2CSC_OFFSET          (0x00A4 >> 2)
> +#define V_BLEND_CR_IN2CSC_OFFSET            (0x00A8 >> 2)
> +#define V_BLEND_CB_IN2CSC_OFFSET            (0x00AC >> 2)
> +#define V_BLEND_CHROMA_KEY_ENABLE           (0x01D0 >> 2)
> +#define V_BLEND_CHROMA_KEY_COMP1            (0x01D4 >> 2)
> +#define V_BLEND_CHROMA_KEY_COMP2            (0x01D8 >> 2)
> +#define V_BLEND_CHROMA_KEY_COMP3            (0x01DC >> 2)
> +
> +/*
> + * Registers offset for Audio Video Buffer configuration.
> + */
> +#define AV_BUF_MANAGER_OFFSET               (0xB000)
> +#define AV_BUF_FORMAT                       (0x0000 >> 2)
> +#define AV_BUF_NON_LIVE_LATENCY             (0x0008 >> 2)
> +#define AV_CHBUF0                           (0x0010 >> 2)
> +#define AV_CHBUF1                           (0x0014 >> 2)
> +#define AV_CHBUF2                           (0x0018 >> 2)
> +#define AV_CHBUF3                           (0x001C >> 2)
> +#define AV_CHBUF4                           (0x0020 >> 2)
> +#define AV_CHBUF5                           (0x0024 >> 2)
> +#define AV_BUF_STC_CONTROL                  (0x002C >> 2)
> +#define AV_BUF_STC_INIT_VALUE0              (0x0030 >> 2)
> +#define AV_BUF_STC_INIT_VALUE1              (0x0034 >> 2)
> +#define AV_BUF_STC_ADJ                      (0x0038 >> 2)
> +#define AV_BUF_STC_VIDEO_VSYNC_TS_REG0      (0x003C >> 2)
> +#define AV_BUF_STC_VIDEO_VSYNC_TS_REG1      (0x0040 >> 2)
> +#define AV_BUF_STC_EXT_VSYNC_TS_REG0        (0x0044 >> 2)
> +#define AV_BUF_STC_EXT_VSYNC_TS_REG1        (0x0048 >> 2)
> +#define AV_BUF_STC_CUSTOM_EVENT_TS_REG0     (0x004C >> 2)
> +#define AV_BUF_STC_CUSTOM_EVENT_TS_REG1     (0x0050 >> 2)
> +#define AV_BUF_STC_CUSTOM_EVENT2_TS_REG0    (0x0054 >> 2)
> +#define AV_BUF_STC_CUSTOM_EVENT2_TS_REG1    (0x0058 >> 2)
> +#define AV_BUF_STC_SNAPSHOT0                (0x0060 >> 2)
> +#define AV_BUF_STC_SNAPSHOT1                (0x0064 >> 2)
> +#define AV_BUF_OUTPUT_AUDIO_VIDEO_SELECT    (0x0070 >> 2)
> +#define AV_BUF_HCOUNT_VCOUNT_INT0           (0x0074 >> 2)
> +#define AV_BUF_HCOUNT_VCOUNT_INT1           (0x0078 >> 2)
> +#define AV_BUF_DITHER_CONFIG                (0x007C >> 2)
> +#define AV_BUF_DITHER_CONFIG_MAX            (0x008C >> 2)
> +#define AV_BUF_DITHER_CONFIG_MIN            (0x0090 >> 2)
> +#define AV_BUF_PATTERN_GEN_SELECT           (0x0100 >> 2)
> +#define AV_BUF_AUD_VID_CLK_SOURCE           (0x0120 >> 2)
> +#define AV_BUF_SRST_REG                     (0x0124 >> 2)
> +#define AV_BUF_AUDIO_RDY_INTERVAL           (0x0128 >> 2)
> +#define AV_BUF_AUDIO_CH_CONFIG              (0x012C >> 2)
> +#define AV_BUF_GRAPHICS_COMP0_SCALE_FACTOR  (0x0200 >> 2)
> +#define AV_BUF_GRAPHICS_COMP1_SCALE_FACTOR  (0x0204 >> 2)
> +#define AV_BUF_GRAPHICS_COMP2_SCALE_FACTOR  (0x0208 >> 2)
> +#define AV_BUF_VIDEO_COMP0_SCALE_FACTOR     (0x020C >> 2)
> +#define AV_BUF_VIDEO_COMP1_SCALE_FACTOR     (0x0210 >> 2)
> +#define AV_BUF_VIDEO_COMP2_SCALE_FACTOR     (0x0214 >> 2)
> +#define AV_BUF_LIVE_VIDEO_COMP0_SF          (0x0218 >> 2)
> +#define AV_BUF_LIVE_VIDEO_COMP1_SF          (0x021C >> 2)
> +#define AV_BUF_LIVE_VIDEO_COMP2_SF          (0x0220 >> 2)
> +#define AV_BUF_LIVE_VID_CONFIG              (0x0224 >> 2)
> +#define AV_BUF_LIVE_GFX_COMP0_SF            (0x0228 >> 2)
> +#define AV_BUF_LIVE_GFX_COMP1_SF            (0x022C >> 2)
> +#define AV_BUF_LIVE_GFX_COMP2_SF            (0x0230 >> 2)

I think these COMP things can be indexed too.

> +#define AV_BUF_LIVE_GFX_CONFIG              (0x0234 >> 2)
> +
> +#define AUDIO_MIXER_REGISTER_OFFSET         (0xC000)
> +#define AUDIO_MIXER_VOLUME_CONTROL          (0x0000 >> 2)
> +#define AUDIO_MIXER_META_DATA               (0x0004 >> 2)
> +#define AUD_CH_STATUS_REG0                  (0x0008 >> 2)
> +#define AUD_CH_STATUS_REG1                  (0x000C >> 2)
> +#define AUD_CH_STATUS_REG2                  (0x0010 >> 2)
> +#define AUD_CH_STATUS_REG3                  (0x0014 >> 2)
> +#define AUD_CH_STATUS_REG4                  (0x0018 >> 2)
> +#define AUD_CH_STATUS_REG5                  (0x001C >> 2)
> +#define AUD_CH_A_DATA_REG0                  (0x0020 >> 2)
> +#define AUD_CH_A_DATA_REG1                  (0x0024 >> 2)
> +#define AUD_CH_A_DATA_REG2                  (0x0028 >> 2)
> +#define AUD_CH_A_DATA_REG3                  (0x002C >> 2)
> +#define AUD_CH_A_DATA_REG4                  (0x0030 >> 2)
> +#define AUD_CH_A_DATA_REG5                  (0x0034 >> 2)
> +#define AUD_CH_B_DATA_REG0                  (0x0038 >> 2)
> +#define AUD_CH_B_DATA_REG1                  (0x003C >> 2)
> +#define AUD_CH_B_DATA_REG2                  (0x0040 >> 2)
> +#define AUD_CH_B_DATA_REG3                  (0x0044 >> 2)
> +#define AUD_CH_B_DATA_REG4                  (0x0048 >> 2)
> +#define AUD_CH_B_DATA_REG5                  (0x004C >> 2)
> +

Index.

> +typedef enum dp_graphic_fmt {

DPGraphicFmt

> +    DP_GRAPHIC_RGBA8888 = 0 << 8,
> +    DP_GRAPHIC_ABGR8888 = 1 << 8,
> +    DP_GRAPHIC_RGB888 = 2 << 8,
> +    DP_GRAPHIC_BGR888 = 3 << 8,
> +    DP_GRAPHIC_RGBA5551 = 4 << 8,
> +    DP_GRAPHIC_RGBA4444 = 5 << 8,
> +    DP_GRAPHIC_RGB565 = 6 << 8,
> +    DP_GRAPHIC_8BPP = 7 << 8,
> +    DP_GRAPHIC_4BPP = 8 << 8,
> +    DP_GRAPHIC_2BPP = 9 << 8,
> +    DP_GRAPHIC_1BPP = 10 << 8,
> +    DP_GRAPHIC_MASK = 0xF << 8

Is it better to define them without the shift and the user handles shifting?

> +} dp_graphic_fmt;
> +
> +typedef enum dp_video_fmt {

DPVideoFmt

> +    DP_NL_VID_CB_Y0_CR_Y1 = 0,
> +    DP_NL_VID_CR_Y0_CB_Y1 = 1,
> +    DP_NL_VID_Y0_CR_Y1_CB = 2,
> +    DP_NL_VID_Y0_CB_Y1_CR = 3,
> +    DP_NL_VID_YV16 = 4,
> +    DP_NL_VID_YV24 = 5,
> +    DP_NL_VID_YV16CL = 6,
> +    DP_NL_VID_MONO = 7,
> +    DP_NL_VID_YV16CL2 = 8,
> +    DP_NL_VID_YUV444 = 9,
> +    DP_NL_VID_RGB888 = 10,
> +    DP_NL_VID_RGBA8880 = 11,
> +    DP_NL_VID_RGB888_10BPC = 12,
> +    DP_NL_VID_YUV444_10BPC = 13,
> +    DP_NL_VID_YV16CL2_10BPC = 14,
> +    DP_NL_VID_YV16CL_10BPC = 15,
> +    DP_NL_VID_YV16_10BPC = 16,
> +    DP_NL_VID_YV24_10BPC = 17,
> +    DP_NL_VID_Y_ONLY_10BPC = 18,
> +    DP_NL_VID_YV16_420 = 19,
> +    DP_NL_VID_YV16CL_420 = 20,
> +    DP_NL_VID_YV16CL2_420 = 21,
> +    DP_NL_VID_YV16_420_10BPC = 22,
> +    DP_NL_VID_YV16CL_420_10BPC = 23,
> +    DP_NL_VID_YV16CL2_420_10BPC = 24,
> +    DP_NL_VID_FMT_MASK = 0x1F
> +} dp_video_fmt;
> +
> +static const VMStateDescription vmstate_dp = {
> +    .name = TYPE_XILINX_DP,
> +    .version_id = 1,
> +    .fields = (VMStateField[]){
> +

Same comment.

> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void xilinx_dp_update_irq(XilinxDPState *s);
> +
> +static uint64_t xilinx_dp_audio_read(void *opaque, hwaddr offset, unsigned 
> size)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +
> +    offset = offset >> 2;
> +
> +    switch (offset) {
> +    default:

drop switch.

> +        return s->audio_registers[offset];
> +    }
> +}
> +
> +static void xilinx_dp_audio_write(void *opaque, hwaddr offset, uint64_t 
> value,
> +                                  unsigned size)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +
> +    offset = offset >> 2;
> +
> +    switch (offset) {
> +    case AUDIO_MIXER_META_DATA:
> +        s->audio_registers[offset] = value & 0x00000001;
> +    break;
> +    default:
> +        s->audio_registers[offset] = value;
> +    break;
> +    }
> +}
> +
> +static const MemoryRegionOps audio_ops = {
> +    .read = xilinx_dp_audio_read,
> +    .write = xilinx_dp_audio_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static inline uint32_t xilinx_dp_audio_get_volume(XilinxDPState *s,
> +                                                  uint8_t channel)
> +{
> +    switch (channel) {
> +    case 0:
> +        return s->audio_registers[AUDIO_MIXER_VOLUME_CONTROL] & 0xFFFF;

extract32

> +    case 1:
> +        return (s->audio_registers[AUDIO_MIXER_VOLUME_CONTROL] >> 16) & 
> 0xFFFF;

extract32

> +    default:
> +        return 0;
> +    }
> +}
> +
> +static inline void xilinx_dp_audio_activate(XilinxDPState *s)
> +{
> +    bool activated =
> +                ((s->core_registers[DP_TX_AUDIO_CONTROL] & 0x00000001) != 0);

Mask should be macro'd.

> +    AUD_set_active_out(s->amixer_output_stream, activated);
> +    xilinx_dpdma_set_host_data_location(s->dpdma, 4, &s->audio_buffer_0);
> +    xilinx_dpdma_set_host_data_location(s->dpdma, 5, &s->audio_buffer_1);

4, 5 should be macrod. These are the defs of the DMA channels right?

> +}
> +
> +static inline void xilinx_dp_audio_mix_buffer(XilinxDPState *s)
> +{
> +    /*
> +     * Audio packets are signed and have this shape:
> +     * | 16 | 16 | 16 | 16 | 16 | 16 | 16 | 16 |
> +     * | R3 | L3 | R2 | L2 | R1 | L1 | R0 | L0 |
> +     *
> +     * Output audio is 16bits saturated.
> +     */
> +    int i;
> +
> +    if ((s->audio_data_available[0]) && (xilinx_dp_audio_get_volume(s, 0))) {
> +        for (i = 0; i < s->audio_data_available[0] / 2; i++) {
> +            s->temp_buffer[i] = (int64_t)(s->audio_buffer_0[i])
> +                              * xilinx_dp_audio_get_volume(s, 0) / 8192;
> +        }
> +        s->byte_left = s->audio_data_available[0];
> +    } else {
> +        memset(s->temp_buffer, 0, s->audio_data_available[1] / 2);
> +    }
> +
> +    if ((s->audio_data_available[1]) && (xilinx_dp_audio_get_volume(s, 1))) {
> +        if ((s->audio_data_available[0] == 0)
> +        || (s->audio_data_available[1] == s->audio_data_available[0])) {
> +            for (i = 0; i < s->audio_data_available[1] / 2; i++) {
> +                s->temp_buffer[i] += (int64_t)(s->audio_buffer_1[i])
> +                                   * xilinx_dp_audio_get_volume(s, 1) / 8192;
> +            }
> +            s->byte_left = s->audio_data_available[1];
> +        }
> +    }
> +
> +    for (i = 0; i < s->byte_left / 2; i++) {
> +        s->out_buffer[i] = s->temp_buffer[i];
> +        if (s->temp_buffer[i] < -32767) {
> +            s->out_buffer[i] = -32767;
> +        }

Strange that -32768 is not valid. This means the saturation arithmatic
is using one less than the full 64k range.

> +        if (s->temp_buffer[i] > 32767) {
> +            s->out_buffer[i] = 32767;
> +        }

MIN and MAX macros remove need for ifs.

> +    }
> +
> +    s->data_ptr = 0;
> +}
> +
> +static void xilinx_dp_audio_callback(void *opaque, int avail)
> +{
> +    /*
> +     * Get some data from the DPDMA and compute them. Then wait QEMU's audio
> +     * subsystem to call this callback.

I think "them" needs more qaulification. Are you reffering to the data?

"wait for" or "wait on".

> +     */
> +    XilinxDPState *s = XILINX_DP(opaque);
> +    size_t written = 0;
> +    static uint8_t buffer;
> +
> +    /* If there are already some data don't get more data. */
> +    if (s->byte_left == 0) {
> +        buffer++;
> +        s->audio_data_available[0] = xilinx_dpdma_start_operation(s->dpdma, 
> 4,
> +                                                                  true);
> +        s->audio_data_available[1] = xilinx_dpdma_start_operation(s->dpdma, 
> 5,
> +                                                                  true);
> +        xilinx_dp_audio_mix_buffer(s);
> +    }
> +
> +    /* Send the buffer through the audio. */
> +    if (s->byte_left <= MAX_QEMU_BUFFER_SIZE) {
> +        if (s->byte_left != 0) {
> +            written = AUD_write(s->amixer_output_stream,
> +                                &s->out_buffer[s->data_ptr], s->byte_left);
> +        } else {
> +            /*
> +             * There is nothing to play.. We don't have any data! Fill the
> +             * buffer with zero's and send it.
> +             */
> +            written = 0;
> +            memset(s->out_buffer, 0, 1024);
> +            AUD_write(s->amixer_output_stream, s->out_buffer, 1024);
> +        }
> +    } else {
> +        written = AUD_write(s->amixer_output_stream,
> +                            &s->out_buffer[s->data_ptr], 
> MAX_QEMU_BUFFER_SIZE);
> +    }
> +    s->byte_left -= written;
> +    s->data_ptr += written;
> +}
> +
> +/*
> + * AUX channel related function.
> + */
> +static void xilinx_dp_aux_clear_rx_fifo(XilinxDPState *s)
> +{
> +    fifo8_reset(&s->rx_fifo);
> +}
> +
> +static void xilinx_dp_aux_push_rx_fifo(XilinxDPState *s, uint8_t *buf,
> +                                       size_t len)
> +{

fifo8_push_all to remove loop. May obsolete the need for this function
and can just do from caller.

> +    int i;
> +
> +    DPRINTF("Push %u data in rx_fifo\n", (unsigned)len);
> +    for (i = 0; i < len; i++) {
> +        if (fifo8_is_full(&s->rx_fifo)) {
> +            DPRINTF("rx_fifo overflow..\n");
> +            abort();
> +        }
> +        fifo8_push(&s->rx_fifo, buf[i]);
> +    }
> +}
> +
> +static uint8_t xilinx_dp_aux_pop_rx_fifo(XilinxDPState *s)
> +{
> +    uint8_t ret;
> +
> +    if (fifo8_is_empty(&s->rx_fifo)) {
> +        DPRINTF("rx_fifo underflow..\n");
> +        abort();
> +    }
> +    ret = fifo8_pop(&s->rx_fifo);
> +    DPRINTF("pop 0x%2.2X from rx_fifo.\n", ret);

PRIx8

> +    return ret;
> +}
> +
> +static void xilinx_dp_aux_clear_tx_fifo(XilinxDPState *s)
> +{
> +    fifo8_reset(&s->tx_fifo);
> +}
> +
> +static void xilinx_dp_aux_push_tx_fifo(XilinxDPState *s, uint8_t *buf,
> +                                       size_t len)
> +{

same.

> +    int i;
> +
> +    DPRINTF("Push %u data in tx_fifo\n", (unsigned)len);
> +    for (i = 0; i < len; i++) {
> +        if (fifo8_is_full(&s->tx_fifo)) {
> +            DPRINTF("tx_fifo overflow..\n");
> +            abort();
> +        }
> +        fifo8_push(&s->tx_fifo, buf[i]);
> +    }
> +}
> +
> +static uint8_t xilinx_dp_aux_pop_tx_fifo(XilinxDPState *s)
> +{
> +    uint8_t ret;
> +
> +    if (fifo8_is_empty(&s->tx_fifo)) {
> +        DPRINTF("tx_fifo underflow..\n");
> +        abort();
> +    }
> +    ret = fifo8_pop(&s->tx_fifo);
> +    DPRINTF("pop 0x%2.2X from tx_fifo.\n", ret);
> +    return ret;
> +}
> +
> +static uint32_t xilinx_dp_aux_get_address(XilinxDPState *s)
> +{
> +    return s->core_registers[DP_AUX_ADDRESS];
> +}
> +
> +static uint8_t xilinx_dp_aux_get_data(XilinxDPState *s)
> +{
> +    return xilinx_dp_aux_pop_rx_fifo(s);
> +}
> +
> +static void xilinx_dp_aux_set_data(XilinxDPState *s, uint8_t value)
> +{
> +    xilinx_dp_aux_push_tx_fifo(s, &value, 1);
> +}
> +
> +/*
> + * Get command from the register.
> + */
> +static void xilinx_dp_aux_set_command(XilinxDPState *s, uint32_t value)
> +{
> +    bool address_only = (value & AUX_ADDR_ONLY_MASK) != 0;
> +    aux_command cmd = (value & AUX_COMMAND_MASK) >> AUX_COMMAND_SHIFT;
> +    uint8_t nbytes = (value & AUX_COMMAND_NBYTES) + 1;
> +    uint8_t buf[16];
> +    int i;
> +
> +    /*
> +     * When an address_only command is executed nothing happen to the fifo, 
> so
> +     * just make nbytes = 0.
> +     */
> +    if (address_only) {
> +        nbytes = 0;
> +    }
> +
> +    switch (cmd) {
> +    case READ_AUX:
> +    case READ_I2C:
> +    case READ_I2C_MOT:
> +        s->core_registers[DP_AUX_REPLY_CODE] = aux_request(s->aux_bus, cmd,
> +                                               xilinx_dp_aux_get_address(s),
> +                                               nbytes, buf);
> +        s->core_registers[DP_REPLY_DATA_COUNT] = nbytes;
> +
> +        if (s->core_registers[DP_AUX_REPLY_CODE] == AUX_I2C_ACK) {
> +            xilinx_dp_aux_push_rx_fifo(s, buf, nbytes);
> +        }
> +    break;
> +    case WRITE_AUX:
> +    case WRITE_I2C:
> +    case WRITE_I2C_MOT:
> +        for (i = 0; i < nbytes; i++) {
> +            buf[i] = xilinx_dp_aux_pop_tx_fifo(s);
> +        }
> +        s->core_registers[DP_AUX_REPLY_CODE] = aux_request(s->aux_bus, cmd,
> +                                               xilinx_dp_aux_get_address(s),
> +                                               nbytes, buf);
> +        xilinx_dp_aux_clear_tx_fifo(s);
> +    break;
> +    case WRITE_I2C_STATUS:
> +    default:
> +        abort();
> +    break;
> +    }
> +
> +    s->core_registers[DP_INTERRUPT_SIGNAL_STATE] |= 0x04;
> +}
> +
> +static void xilinx_dp_set_dpdma(Object *obj, const char *name, Object *val,
> +                                Error **errp)
> +{
> +    XilinxDPState *s = XILINX_DP(obj);
> +    if (s->console) {
> +        DisplaySurface *surface = qemu_console_surface(s->console);
> +        XilinxDPDMAState *dma = XILINX_DPDMA(val);
> +        xilinx_dpdma_set_host_data_location(dma, 3, surface_data(surface));
> +    }
> +}
> +
> +static inline uint8_t xilinx_dp_global_alpha_value(XilinxDPState *s)
> +{
> +    return (s->vblend_registers[V_BLEND_SET_GLOBAL_ALPHA_REG] & 0x1FE) >> 1;
> +}
> +
> +static inline bool xilinx_dp_global_alpha_enabled(XilinxDPState *s)
> +{
> +    /*
> +     * If the alpha is totally opaque (255) we don't consider the alpha is
> +     * disabled to reduce CPU consumption.

Double negative.

> +     */
> +    return ((xilinx_dp_global_alpha_value(s) != 0xFF) &&
> +           ((s->vblend_registers[V_BLEND_SET_GLOBAL_ALPHA_REG] & 0x01) != 
> 0));
> +}
> +
> +static void xilinx_dp_recreate_surface(XilinxDPState *s)
> +{
> +    /*
> +     * Two possibilities, if blending is enabled the console display 
> bout_plane,

"displays"

> +     * if not g_plane is displayed.
> +     */
> +    uint16_t width = s->core_registers[DP_MAIN_STREAM_HRES];
> +    uint16_t height = s->core_registers[DP_MAIN_STREAM_VRES];
> +    DisplaySurface *current_console_surface = 
> qemu_console_surface(s->console);
> +
> +    if ((width != 0) && (height != 0)) {
> +        /*
> +         * As dpy_gfx_replace_surface calls qemu_free_displaysurface on the
> +         * surface we need to be carefull and don't free the surface 
> associated
> +         * to the console or double free will happen.
> +         */
> +        if (s->bout_plane.surface != current_console_surface) {
> +            qemu_free_displaysurface(s->bout_plane.surface);
> +        }
> +        if (s->v_plane.surface != current_console_surface) {
> +            qemu_free_displaysurface(s->v_plane.surface);
> +        }
> +        if (s->g_plane.surface != current_console_surface) {
> +            qemu_free_displaysurface(s->g_plane.surface);
> +
> +        }
> +
> +        s->g_plane.surface
> +                = qemu_create_displaysurface_from(width, height,
> +                                                  s->g_plane.format, 0, 
> NULL);
> +        s->v_plane.surface
> +                = qemu_create_displaysurface_from(width, height,
> +                                                  s->v_plane.format, 0, 
> NULL);
> +        if (xilinx_dp_global_alpha_enabled(s)) {
> +            s->bout_plane.surface =
> +                            qemu_create_displaysurface_from(width,
> +                                                            height,
> +                                                            
> s->g_plane.format,
> +                                                            0, NULL);
> +            dpy_gfx_replace_surface(s->console, s->bout_plane.surface);
> +        } else {
> +            s->bout_plane.surface = NULL;
> +            dpy_gfx_replace_surface(s->console, s->g_plane.surface);
> +        }
> +
> +        xilinx_dpdma_set_host_data_location(s->dpdma, 3,
> +                                            
> surface_data(s->g_plane.surface));
> +        xilinx_dpdma_set_host_data_location(s->dpdma, 0,
> +                                            
> surface_data(s->v_plane.surface));
> +    }
> +}
> +
> +/*
> + * Change the graphic format of the surface.
> + * XXX: To be completed.
> + */
> +static void xilinx_dp_change_graphic_fmt(XilinxDPState *s)
> +{
> +    switch (s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK) {
> +    case DP_GRAPHIC_RGBA8888:
> +        s->g_plane.format = PIXMAN_r8g8b8a8;
> +        break;
> +    case DP_GRAPHIC_ABGR8888:
> +        s->g_plane.format = PIXMAN_a8b8g8r8;
> +        break;
> +    case DP_GRAPHIC_RGB565:
> +        s->g_plane.format = PIXMAN_r5g6b5;
> +        break;
> +    case DP_GRAPHIC_RGB888:
> +        s->g_plane.format = PIXMAN_r8g8b8;
> +        break;
> +    case DP_GRAPHIC_BGR888:
> +        s->g_plane.format = PIXMAN_b8g8r8;
> +        break;
> +    default:
> +        DPRINTF("error: unsupported graphic format %u.\n",
> +                s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> +        abort();
> +        break;
> +    }
> +
> +    switch (s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK) {
> +    case 0:
> +        s->v_plane.format = PIXMAN_r8g8b8a8;
> +        break;
> +    case DP_NL_VID_RGBA8880:
> +        s->v_plane.format = PIXMAN_r8g8b8a8;
> +        break;
> +    default:
> +        DPRINTF("error: unsupported video format %u.\n",
> +                s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
> +        abort();
> +        break;
> +    }
> +
> +    xilinx_dp_recreate_surface(s);
> +}
> +
> +static void xilinx_dp_update_irq(XilinxDPState *s)
> +{
> +    uint32_t flags;
> +
> +    flags = s->core_registers[DP_INT_STATUS] & 
> ~s->core_registers[DP_INT_MASK];
> +    DPRINTF("update IRQ value = %" PRIx32 "\n", flags);
> +    qemu_set_irq(s->irq, flags != 0);
> +}
> +
> +static uint64_t xilinx_dp_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +    uint64_t ret = 0;
> +
> +    offset = offset >> 2;
> +
> +    switch (offset) {
> +    /*
> +     * Trying to read a write only register.
> +     */
> +    case DP_TX_USER_FIFO_OVERFLOW:
> +        ret = s->core_registers[DP_TX_USER_FIFO_OVERFLOW];
> +        s->core_registers[DP_TX_USER_FIFO_OVERFLOW] = 0;
> +    break;
> +    case DP_AUX_WRITE_FIFO:
> +        ret = 0;
> +    break;
> +    case DP_AUX_REPLY_DATA:
> +        ret = xilinx_dp_aux_get_data(s);
> +    break;
> +    case DP_INTERRUPT_SIGNAL_STATE:
> +        /*
> +         * XXX: Not sure it is the right thing to do actually.
> +         * The register is not written by the device driver so it's stuck
> +         * to 0x04.
> +         */
> +        ret = s->core_registers[DP_INTERRUPT_SIGNAL_STATE];
> +        s->core_registers[DP_INTERRUPT_SIGNAL_STATE] &= ~0x04;
> +    break;
> +    case DP_TX_AUDIO_INFO_DATA0:
> +    case DP_TX_AUDIO_INFO_DATA1:
> +    case DP_TX_AUDIO_INFO_DATA2:
> +    case DP_TX_AUDIO_INFO_DATA3:
> +    case DP_TX_AUDIO_INFO_DATA4:
> +    case DP_TX_AUDIO_INFO_DATA5:
> +    case DP_TX_AUDIO_INFO_DATA6:
> +    case DP_TX_AUDIO_INFO_DATA7:
> +    case DP_TX_AUDIO_EXT_DATA0:
> +    case DP_TX_AUDIO_EXT_DATA1:
> +    case DP_TX_AUDIO_EXT_DATA2:
> +    case DP_TX_AUDIO_EXT_DATA3:
> +    case DP_TX_AUDIO_EXT_DATA4:
> +    case DP_TX_AUDIO_EXT_DATA5:
> +    case DP_TX_AUDIO_EXT_DATA6:
> +    case DP_TX_AUDIO_EXT_DATA7:
> +    case DP_TX_AUDIO_EXT_DATA8:
> +        /* write only registers */
> +        ret = 0;
> +    break;
> +    default:
> +        assert(offset <= (0x3AC >> 2));
> +        ret = s->core_registers[offset];
> +    break;
> +    }
> +
> +    DPRINTF("core read @%" PRIx64 " = 0x%8.8lX\n", offset << 2, ret);
> +    return ret;
> +}
> +
> +static void xilinx_dp_write(void *opaque, hwaddr offset, uint64_t value,
> +                            unsigned size)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +
> +    DPRINTF("core write @%" PRIx64 " = 0x%8.8lX\n", offset, value);
> +
> +    offset = offset >> 2;
> +
> +    switch (offset) {
> +    /*
> +     * Only special write case are handled.
> +     */
> +    case DP_LINK_BW_SET:
> +        s->core_registers[offset] = value & 0x000000FF;
> +    break;
> +    case DP_LANE_COUNT_SET:
> +    case DP_MAIN_STREAM_MISC0:
> +        s->core_registers[offset] = value & 0x0000000F;
> +    break;
> +    case DP_TRAINING_PATTERN_SET:
> +    case DP_LINK_QUAL_PATTERN_SET:
> +    case DP_MAIN_STREAM_POLARITY:
> +    case DP_PHY_VOLTAGE_DIFF_LANE_0:
> +    case DP_PHY_VOLTAGE_DIFF_LANE_1:
> +        s->core_registers[offset] = value & 0x00000003;
> +    break;
> +    case DP_ENHANCED_FRAME_EN:
> +    case DP_SCRAMBLING_DISABLE:
> +    case DP_DOWNSPREAD_CTRL:
> +    case DP_MAIN_STREAM_ENABLE:
> +    case DP_TRANSMIT_PRBS7:
> +        s->core_registers[offset] = value & 0x00000001;
> +    break;
> +    case DP_PHY_CLOCK_SELECT:
> +        s->core_registers[offset] = value & 0x00000007;
> +    case DP_SOFTWARE_RESET:
> +        /*
> +         * No need to update this bit as it's read '0'.
> +         */
> +        /*
> +         * TODO: reset IP.
> +         */
> +    break;
> +    case DP_TRANSMITTER_ENABLE:
> +        s->core_registers[offset] = value & 0x01;
> +    break;
> +    case DP_FORCE_SCRAMBLER_RESET:
> +        /*
> +         * No need to update this bit as it's read '0'.
> +         */
> +        /*
> +         * TODO: force a scrambler reset??
> +         */
> +    break;
> +    case DP_AUX_COMMAND_REGISTER:
> +        s->core_registers[offset] = value & 0x00001F0F;
> +        xilinx_dp_aux_set_command(s, s->core_registers[offset]);
> +    break;
> +    case DP_MAIN_STREAM_HTOTAL:
> +    case DP_MAIN_STREAM_VTOTAL:
> +    case DP_MAIN_STREAM_HSTART:
> +    case DP_MAIN_STREAM_VSTART:
> +        s->core_registers[offset] = value & 0x0000FFFF;
> +    break;
> +    case DP_MAIN_STREAM_HRES:
> +    case DP_MAIN_STREAM_VRES:
> +        s->core_registers[offset] = value & 0x0000FFFF;
> +        xilinx_dp_recreate_surface(s);
> +    break;
> +    case DP_MAIN_STREAM_HSWIDTH:
> +    case DP_MAIN_STREAM_VSWIDTH:
> +        s->core_registers[offset] = value & 0x00007FFF;
> +    break;
> +    case DP_MAIN_STREAM_MISC1:
> +        s->core_registers[offset] = value & 0x00000086;
> +    break;
> +    case DP_MAIN_STREAM_M_VID:
> +    case DP_MAIN_STREAM_N_VID:
> +        s->core_registers[offset] = value & 0x00FFFFFF;
> +    break;
> +    case DP_MSA_TRANSFER_UNIT_SIZE:
> +    case DP_MIN_BYTES_PER_TU:
> +    case DP_INIT_WAIT:
> +        s->core_registers[offset] = value & 0x00000007;
> +    break;
> +    case DP_USER_DATA_COUNT_PER_LANE:
> +        s->core_registers[offset] = value & 0x0003FFFF;
> +    break;
> +    case DP_FRAC_BYTES_PER_TU:
> +        s->core_registers[offset] = value & 0x000003FF;
> +    break;
> +    case DP_PHY_RESET:
> +        s->core_registers[offset] = value & 0x00010003;
> +        /*
> +         * TODO: Reset something?
> +         */
> +    break;
> +    case DP_TX_PHY_POWER_DOWN:
> +        s->core_registers[offset] = value & 0x0000000F;
> +        /*
> +         * TODO: Power down things?
> +         */
> +    break;
> +    case DP_AUX_WRITE_FIFO:
> +        xilinx_dp_aux_set_data(s, value & 0x0000000F);
> +    break;
> +    case DP_AUX_CLOCK_DIVIDER:
> +    break;
> +    case DP_AUX_REPLY_COUNT:
> +        /*
> +         * Writing to this register clear the counter.
> +         */
> +        s->core_registers[offset] = 0x00000000;
> +    break;
> +    case DP_AUX_ADDRESS:
> +        s->core_registers[offset] = value & 0x000FFFFF;
> +    break;
> +    case DP_VERSION_REGISTER:
> +    case DP_CORE_ID:
> +    case DP_TX_USER_FIFO_OVERFLOW:
> +    case DP_AUX_REPLY_DATA:
> +    case DP_AUX_REPLY_CODE:
> +    case DP_REPLY_DATA_COUNT:
> +    case DP_REPLY_STATUS:
> +    case DP_HPD_DURATION:
> +        /*
> +         * Write to read only location..
> +         */
> +    break;
> +    case DP_TX_AUDIO_CONTROL:
> +        s->core_registers[offset] = value & 0x00000001;
> +        xilinx_dp_audio_activate(s);
> +    break;
> +    case DP_TX_AUDIO_CHANNELS:
> +        s->core_registers[offset] = value & 0x00000007;
> +        xilinx_dp_audio_activate(s);
> +    break;
> +    case DP_INT_STATUS:
> +        s->core_registers[DP_INT_STATUS] &= ~value;
> +        xilinx_dp_update_irq(s);
> +    break;
> +    case DP_INT_EN:
> +        s->core_registers[DP_INT_MASK] &= ~value;
> +        xilinx_dp_update_irq(s);
> +    break;
> +    case DP_INT_DS:
> +        s->core_registers[DP_INT_MASK] |= ~value;
> +        xilinx_dp_update_irq(s);
> +    break;
> +    default:
> +        assert(offset <= (0x504C >> 2));
> +        s->core_registers[offset] = value;
> +    break;
> +    }
> +}
> +
> +static const MemoryRegionOps dp_ops = {
> +    .read = xilinx_dp_read,
> +    .write = xilinx_dp_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +/*
> + * This is to handle Read/Write to the Video Blender.
> + */
> +static void xilinx_dp_vblend_write(void *opaque, hwaddr offset,
> +                                   uint64_t value, unsigned size)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +    bool alpha_was_enabled;
> +    assert(size == 4);
> +    assert((offset % 4) == 0);

Let memory API enforce aligns and sizes.

> +
> +    DPRINTF("vblend: write @%" PRIx64 " = 0x%8.8lX\n", offset, value);
> +
> +    offset = offset >> 2;
> +
> +    switch (offset) {
> +    case V_BLEND_BG_CLR_0:
> +    case V_BLEND_BG_CLR_1:
> +    case V_BLEND_BG_CLR_2:
> +        s->vblend_registers[offset] = value & 0x00000FFF;
> +    break;
> +    case V_BLEND_SET_GLOBAL_ALPHA_REG:
> +        /*
> +         * A write to this register can enable or disable blending. Thus we 
> need
> +         * to recreate the surfaces.
> +         */
> +        alpha_was_enabled = xilinx_dp_global_alpha_enabled(s);
> +        s->vblend_registers[offset] = value & 0x000001FF;
> +        if (xilinx_dp_global_alpha_enabled(s) != alpha_was_enabled) {
> +            xilinx_dp_recreate_surface(s);
> +        }
> +    break;
> +    case V_BLEND_OUTPUT_VID_FORMAT:
> +        s->vblend_registers[offset] = value & 0x00000017;
> +    break;
> +    case V_BLEND_LAYER0_CONTROL:
> +    case V_BLEND_LAYER1_CONTROL:
> +        s->vblend_registers[offset] = value & 0x00000103;
> +    break;
> +    case V_BLEND_RGB2YCBCR_COEFF0:
> +    case V_BLEND_RGB2YCBCR_COEFF1:
> +    case V_BLEND_RGB2YCBCR_COEFF2:
> +    case V_BLEND_RGB2YCBCR_COEFF3:
> +    case V_BLEND_RGB2YCBCR_COEFF4:
> +    case V_BLEND_RGB2YCBCR_COEFF5:
> +    case V_BLEND_RGB2YCBCR_COEFF6:
> +    case V_BLEND_RGB2YCBCR_COEFF7:
> +    case V_BLEND_RGB2YCBCR_COEFF8:
> +    case V_BLEND_IN1CSC_COEFF0:
> +    case V_BLEND_IN1CSC_COEFF1:
> +    case V_BLEND_IN1CSC_COEFF2:
> +    case V_BLEND_IN1CSC_COEFF3:
> +    case V_BLEND_IN1CSC_COEFF4:
> +    case V_BLEND_IN1CSC_COEFF5:
> +    case V_BLEND_IN1CSC_COEFF6:
> +    case V_BLEND_IN1CSC_COEFF7:
> +    case V_BLEND_IN1CSC_COEFF8:
> +    case V_BLEND_IN2CSC_COEFF0:
> +    case V_BLEND_IN2CSC_COEFF1:
> +    case V_BLEND_IN2CSC_COEFF2:
> +    case V_BLEND_IN2CSC_COEFF3:
> +    case V_BLEND_IN2CSC_COEFF4:
> +    case V_BLEND_IN2CSC_COEFF5:
> +    case V_BLEND_IN2CSC_COEFF6:
> +    case V_BLEND_IN2CSC_COEFF7:
> +    case V_BLEND_IN2CSC_COEFF8:
> +        s->vblend_registers[offset] = value & 0x0000FFFF;
> +    break;
> +    case V_BLEND_LUMA_IN1CSC_OFFSET:
> +    case V_BLEND_CR_IN1CSC_OFFSET:
> +    case V_BLEND_CB_IN1CSC_OFFSET:
> +    case V_BLEND_LUMA_IN2CSC_OFFSET:
> +    case V_BLEND_CR_IN2CSC_OFFSET:
> +    case V_BLEND_CB_IN2CSC_OFFSET:
> +    case V_BLEND_LUMA_OUTCSC_OFFSET:
> +    case V_BLEND_CR_OUTCSC_OFFSET:
> +    case V_BLEND_CB_OUTCSC_OFFSET:
> +        s->vblend_registers[offset] = value & 0x3FFF7FFF;
> +    break;
> +    case V_BLEND_CHROMA_KEY_ENABLE:
> +        s->vblend_registers[offset] = value & 0x00000003;
> +    break;
> +    case V_BLEND_CHROMA_KEY_COMP1:
> +    case V_BLEND_CHROMA_KEY_COMP2:
> +    case V_BLEND_CHROMA_KEY_COMP3:
> +        s->vblend_registers[offset] = value & 0x0FFF0FFF;
> +    break;
> +    default:
> +        s->vblend_registers[offset] = value;
> +    break;
> +    }
> +}
> +
> +static uint64_t xilinx_dp_vblend_read(void *opaque, hwaddr offset,
> +                                      unsigned size)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +    uint32_t ret;
> +
> +    offset = offset >> 2;
> +
> +    ret = s->vblend_registers[offset];
> +    DPRINTF("vblend: read @%" PRIx64 " = 0x%8.8X\n", offset << 2, ret);
> +    return ret;
> +}
> +
> +static const MemoryRegionOps vblend_ops = {
> +    .read = xilinx_dp_vblend_read,
> +    .write = xilinx_dp_vblend_write,

Access restrictions can be defined here.

> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +/*
> + * This is to handle Read/Write to the Audio Video buffer manager.
> + */
> +static void xilinx_dp_avbufm_write(void *opaque, hwaddr offset, uint64_t 
> value,
> +                                   unsigned size)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +

Missing DPRINTF?

> +    offset = offset >> 2;
> +
> +    switch (offset) {
> +    case AV_BUF_FORMAT:
> +        s->avbufm_registers[offset] = value & 0x00000FFF;
> +        xilinx_dp_change_graphic_fmt(s);
> +    break;
> +    case AV_CHBUF0:
> +    case AV_CHBUF1:
> +    case AV_CHBUF2:
> +    case AV_CHBUF3:
> +    case AV_CHBUF4:
> +    case AV_CHBUF5:
> +        s->avbufm_registers[offset] = value & 0x0000007F;
> +    break;
> +    case AV_BUF_OUTPUT_AUDIO_VIDEO_SELECT:
> +        s->avbufm_registers[offset] = value & 0x0000007F;
> +    break;
> +    case AV_BUF_DITHER_CONFIG:
> +        s->avbufm_registers[offset] = value & 0x000007FF;
> +    break;
> +    case AV_BUF_DITHER_CONFIG_MAX:
> +    case AV_BUF_DITHER_CONFIG_MIN:
> +        s->avbufm_registers[offset] = value & 0x00000FFF;
> +    break;
> +    case AV_BUF_PATTERN_GEN_SELECT:
> +        s->avbufm_registers[offset] = value & 0xFFFFFF03;
> +    break;
> +    case AV_BUF_AUD_VID_CLK_SOURCE:
> +        s->avbufm_registers[offset] = value & 0x00000007;
> +    break;
> +    case AV_BUF_SRST_REG:
> +        s->avbufm_registers[offset] = value & 0x00000002;
> +    break;
> +    case AV_BUF_AUDIO_CH_CONFIG:
> +        s->avbufm_registers[offset] = value & 0x00000003;
> +    break;
> +    case AV_BUF_GRAPHICS_COMP0_SCALE_FACTOR:
> +    case AV_BUF_GRAPHICS_COMP1_SCALE_FACTOR:
> +    case AV_BUF_GRAPHICS_COMP2_SCALE_FACTOR:
> +    case AV_BUF_VIDEO_COMP0_SCALE_FACTOR:
> +    case AV_BUF_VIDEO_COMP1_SCALE_FACTOR:
> +    case AV_BUF_VIDEO_COMP2_SCALE_FACTOR:
> +        s->avbufm_registers[offset] = value & 0x0000FFFF;
> +    break;
> +    case AV_BUF_LIVE_VIDEO_COMP0_SF:
> +    case AV_BUF_LIVE_VIDEO_COMP1_SF:
> +    case AV_BUF_LIVE_VIDEO_COMP2_SF:
> +    case AV_BUF_LIVE_VID_CONFIG:
> +    case AV_BUF_LIVE_GFX_COMP0_SF:
> +    case AV_BUF_LIVE_GFX_COMP1_SF:
> +    case AV_BUF_LIVE_GFX_COMP2_SF:
> +    case AV_BUF_LIVE_GFX_CONFIG:
> +    case AV_BUF_NON_LIVE_LATENCY:
> +    case AV_BUF_STC_CONTROL:
> +    case AV_BUF_STC_INIT_VALUE0:
> +    case AV_BUF_STC_INIT_VALUE1:
> +    case AV_BUF_STC_ADJ:
> +    case AV_BUF_STC_VIDEO_VSYNC_TS_REG0:
> +    case AV_BUF_STC_VIDEO_VSYNC_TS_REG1:
> +    case AV_BUF_STC_EXT_VSYNC_TS_REG0:
> +    case AV_BUF_STC_EXT_VSYNC_TS_REG1:
> +    case AV_BUF_STC_CUSTOM_EVENT_TS_REG0:
> +    case AV_BUF_STC_CUSTOM_EVENT_TS_REG1:
> +    case AV_BUF_STC_CUSTOM_EVENT2_TS_REG0:
> +    case AV_BUF_STC_CUSTOM_EVENT2_TS_REG1:
> +    case AV_BUF_STC_SNAPSHOT0:
> +    case AV_BUF_STC_SNAPSHOT1:
> +    case AV_BUF_HCOUNT_VCOUNT_INT0:
> +    case AV_BUF_HCOUNT_VCOUNT_INT1:
> +        /*
> +         * Non implemented.
> +         */

qemu_log_mask(LOG_UNIMP,

> +    break;
> +    default:
> +        s->avbufm_registers[offset] = value;
> +    break;
> +    }
> +}
> +
> +static uint64_t xilinx_dp_avbufm_read(void *opaque, hwaddr offset,
> +                                      unsigned size)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +    assert(size == 4);
> +    assert((offset % 4) == 0);
> +
> +    offset = offset >> 2;
> +
> +    return s->avbufm_registers[offset];
> +}
> +
> +static const MemoryRegionOps avbufm_ops = {
> +    .read = xilinx_dp_avbufm_read,
> +    .write = xilinx_dp_avbufm_write,

same comments WRT align and size.

> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +/*
> + * This is a global alpha blending using pixman.
> + * Both graphic and video planes are multiplied with the global alpha
> + * coefficient and added.
> + */
> +static inline void xilinx_dp_blend_surface(XilinxDPState *s)
> +{
> +    pixman_fixed_t alpha1[] = { pixman_double_to_fixed(1),
> +                                pixman_double_to_fixed(1),
> +                                pixman_double_to_fixed(1.0) };
> +    pixman_fixed_t alpha2[] = { pixman_double_to_fixed(1),
> +                                pixman_double_to_fixed(1),
> +                                pixman_double_to_fixed(1.0) };
> +
> +    if ((surface_width(s->g_plane.surface)
> +         != surface_width(s->v_plane.surface)) ||
> +        (surface_height(s->g_plane.surface)
> +         != surface_height(s->v_plane.surface))) {
> +        return;
> +    }
> +
> +    alpha1[2] = 
> pixman_double_to_fixed((double)(xilinx_dp_global_alpha_value(s))
> +                                       / 256.0);
> +    alpha2[2] = pixman_double_to_fixed((255.0
> +                                    - 
> (double)xilinx_dp_global_alpha_value(s))
> +                                       / 256.0);
> +
> +    pixman_image_set_filter(s->g_plane.surface->image,
> +                            PIXMAN_FILTER_CONVOLUTION, alpha1, 3);
> +    pixman_image_composite(PIXMAN_OP_SRC, s->g_plane.surface->image, 0,
> +                           s->bout_plane.surface->image, 0, 0, 0, 0, 0, 0,
> +                           surface_width(s->g_plane.surface),
> +                           surface_height(s->g_plane.surface));
> +    pixman_image_set_filter(s->v_plane.surface->image,
> +                            PIXMAN_FILTER_CONVOLUTION, alpha2, 3);
> +    pixman_image_composite(PIXMAN_OP_ADD, s->v_plane.surface->image, 0,
> +                           s->bout_plane.surface->image, 0, 0, 0, 0, 0, 0,
> +                           surface_width(s->g_plane.surface),
> +                           surface_height(s->g_plane.surface));
> +}
> +
> +static void xilinx_dp_update_display(void *opaque)
> +{
> +    XilinxDPState *s = XILINX_DP(opaque);
> +
> +    if (DEBUG_DP) {
> +        int64_t last_time = 0;
> +        int64_t frame = 0;
> +        int64_t time = get_clock();
> +        int64_t fps;
> +
> +        if (last_time == 0) {
> +            last_time = get_clock();
> +        }
> +        frame++;
> +        if (last_time + 1000000000 < time) {
> +            fps = (1000000000.0 * frame) / (time - last_time);
> +            last_time = time;
> +            frame = 0;
> +            DPRINTF("xilinx_dp: %ldfps\n", fps);
> +        }
> +    }
> +
> +
> +    if ((s->core_registers[DP_TRANSMITTER_ENABLE] & 0x01) == 0) {
> +        return;
> +    }
> +
> +    s->core_registers[DP_INT_STATUS] |= (1 << 13);
> +    xilinx_dp_update_irq(s);
> +
> +    /*
> +     * Trigger the DMA channel.
> +     */
> +    if (!xilinx_dpdma_start_operation(s->dpdma, 3, false)) {
> +        /*
> +         * An error occured don't do anything with the data..
> +         * Trigger an underflow interrupt.
> +         */
> +        s->core_registers[DP_INT_STATUS] |= (1 << 21);
> +        xilinx_dp_update_irq(s);
> +        return;
> +    }
> +
> +    if (xilinx_dp_global_alpha_enabled(s)) {
> +        if (!xilinx_dpdma_start_operation(s->dpdma, 0, false)) {
> +            s->core_registers[DP_INT_STATUS] |= (1 << 21);
> +            xilinx_dp_update_irq(s);
> +            return;
> +        }
> +        xilinx_dp_blend_surface(s);
> +    }
> +
> +    /*
> +     * XXX: We might want to update only what changed.
> +     */
> +    dpy_gfx_update(s->console, 0, 0, surface_width(s->g_plane.surface),
> +                                     surface_height(s->g_plane.surface));
> +}
> +
> +static void xilinx_dp_invalidate_display(void *opaque)
> +{
> +
> +}
> +

Should the gfx core just null guard this?

> +static const GraphicHwOps xilinx_dp_gfx_ops = {
> +    .invalidate  = xilinx_dp_invalidate_display,
> +    .gfx_update  = xilinx_dp_update_display,
> +};
> +
> +static void xilinx_dp_init(Object *obj)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    XilinxDPState *s = XILINX_DP(obj);
> +
> +    memory_region_init(&s->container, obj, TYPE_XILINX_DP, 0xC050);
> +
> +    memory_region_init_io(&s->core_iomem, obj, &dp_ops, s, TYPE_XILINX_DP
> +                          ".core", 0x3AF);
> +    memory_region_add_subregion(&s->container, 0x0000, &s->core_iomem);
> +
> +    memory_region_init_io(&s->vblend_iomem, obj, &vblend_ops, s, 
> TYPE_XILINX_DP
> +                          ".v_blend", 0x1DF);
> +    memory_region_add_subregion(&s->container, 0xA000, &s->vblend_iomem);
> +
> +    memory_region_init_io(&s->avbufm_iomem, obj, &avbufm_ops, s, 
> TYPE_XILINX_DP
> +                          ".av_buffer_manager", 0x238);
> +    memory_region_add_subregion(&s->container, 0xB000, &s->avbufm_iomem);
> +    memory_region_init_io(&s->audio_iomem, obj, &audio_ops, s, TYPE_XILINX_DP
> +                          ".audio", sizeof(s->audio_registers));
> +    memory_region_add_subregion(&s->container, 0xC000, &s->audio_iomem);
> +    sysbus_init_mmio(sbd, &s->container);
> +
> +    sysbus_init_irq(sbd, &s->irq);
> +
> +    object_property_add_link(obj, "dpdma", TYPE_XILINX_DPDMA,
> +                             (Object **) &s->dpdma,
> +                             xilinx_dp_set_dpdma,
> +                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
> +                             &error_abort);
> +
> +    s->byte_left = 0;

Is this device state and should it be in reset?

> +
> +    /*
> +     * Initialize AUX Bus.
> +     */
> +    s->aux_bus = aux_init_bus(DEVICE(obj), "aux");
> +
> +    /*
> +     * Initialize DPCD and EDID..
> +     */
> +    s->dpcd = DPCD(aux_create_slave(s->aux_bus, "dpcd", 0x00000));
> +    s->edid = I2CDDC(qdev_create(BUS(aux_get_i2c_bus(s->aux_bus)), 
> "i2c-ddc"));

Are these off-chip and should be created by the machine model?

> +    i2c_set_slave_address(I2C_SLAVE(s->edid), 0x50);
> +}
> +
> +static void xilinx_dp_realize(DeviceState *dev, Error **errp)
> +{
> +    XilinxDPState *s = XILINX_DP(dev);
> +    DisplaySurface *surface;
> +
> +    s->console = graphic_console_init(dev, 0, &xilinx_dp_gfx_ops, s);
> +    surface = qemu_console_surface(s->console);
> +    xilinx_dpdma_set_host_data_location(s->dpdma, 3, surface_data(surface));
> +    fifo8_create(&s->rx_fifo, 16);
> +    fifo8_create(&s->tx_fifo, 16);

fifo_creates should move to _init

> +
> +    /* Audio */
> +    struct audsettings as;

def should move up top of fn.

> +    as.freq = 44100;
> +    as.nchannels = 2;
> +    as.fmt = AUD_FMT_S16;
> +    as.endianness = 0;
> +
> +    AUD_register_card("xilinx_dp.audio", &s->aud_card);
> +
> +    s->amixer_output_stream = AUD_open_out(&s->aud_card,
> +                                           s->amixer_output_stream,
> +                                           "xilinx_dp.audio.out",
> +                                           s,
> +                                           xilinx_dp_audio_callback,
> +                                           &as);
> +    AUD_set_volume_out(s->amixer_output_stream, 0, 255, 255);
> +    xilinx_dp_audio_activate(s);
> +}
> +
> +static void xilinx_dp_reset(DeviceState *dev)
> +{
> +    XilinxDPState *s = XILINX_DP(dev);
> +
> +    /*
> +     * Reset the Display Port registers.
> +     */

self documenting.

> +    memset(s->core_registers, 0, sizeof(s->core_registers));
> +    s->core_registers[DP_VERSION_REGISTER] = 0x04010000;
> +    s->core_registers[DP_CORE_ID] = 0x01020000;
> +    s->core_registers[DP_REPLY_STATUS] = 0x00000010;
> +    s->core_registers[DP_MSA_TRANSFER_UNIT_SIZE] = 0x00000040;
> +    s->core_registers[DP_INIT_WAIT] = 0x00000020;
> +    s->core_registers[DP_PHY_RESET] = 0x00010003;
> +    s->core_registers[DP_INT_MASK] = 0xFFFFF03F;
> +
> +    s->core_registers[DP_PHY_STATUS] = 0x00000043;
> +    s->core_registers[DP_INTERRUPT_SIGNAL_STATE] = 0x00000001;
> +
> +    /*
> +     * Video Blender register reset.
> +     */
> +    s->vblend_registers[V_BLEND_RGB2YCBCR_COEFF0] = 0x00001000;
> +    s->vblend_registers[V_BLEND_RGB2YCBCR_COEFF4] = 0x00001000;
> +    s->vblend_registers[V_BLEND_RGB2YCBCR_COEFF8] = 0x00001000;
> +    s->vblend_registers[V_BLEND_IN1CSC_COEFF0] = 0x00001000;
> +    s->vblend_registers[V_BLEND_IN1CSC_COEFF4] = 0x00001000;
> +    s->vblend_registers[V_BLEND_IN1CSC_COEFF8] = 0x00001000;
> +    s->vblend_registers[V_BLEND_IN2CSC_COEFF0] = 0x00001000;
> +    s->vblend_registers[V_BLEND_IN2CSC_COEFF4] = 0x00001000;
> +    s->vblend_registers[V_BLEND_IN2CSC_COEFF8] = 0x00001000;

Probably loopable once the indexing macros are defined.

> +
> +    /*
> +     * Audio Video Buffer Manager register reset.
> +     */
> +    s->avbufm_registers[AV_BUF_NON_LIVE_LATENCY] = 0x00000180;
> +    s->avbufm_registers[AV_BUF_OUTPUT_AUDIO_VIDEO_SELECT] = 0x00000008;
> +    s->avbufm_registers[AV_BUF_DITHER_CONFIG_MAX] = 0x00000FFF;
> +    s->avbufm_registers[AV_BUF_GRAPHICS_COMP0_SCALE_FACTOR] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_GRAPHICS_COMP1_SCALE_FACTOR] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_GRAPHICS_COMP2_SCALE_FACTOR] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_VIDEO_COMP0_SCALE_FACTOR] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_VIDEO_COMP1_SCALE_FACTOR] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_VIDEO_COMP2_SCALE_FACTOR] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_LIVE_VIDEO_COMP0_SF] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_LIVE_VIDEO_COMP1_SF] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_LIVE_VIDEO_COMP2_SF] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_LIVE_GFX_COMP0_SF] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_LIVE_GFX_COMP1_SF] = 0x00010101;
> +    s->avbufm_registers[AV_BUF_LIVE_GFX_COMP2_SF] = 0x00010101;
> +

Same.

> +    /*
> +     * Audio register reset.
> +     */
> +    memset(s->audio_registers, 0, sizeof(s->audio_registers));
> +
> +    xilinx_dp_aux_clear_rx_fifo(s);
> +    xilinx_dp_change_graphic_fmt(s);
> +}
> +
> +static void xilinx_dp_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = xilinx_dp_realize;
> +    dc->vmsd = &vmstate_dp;
> +    dc->reset = xilinx_dp_reset;
> +}
> +
> +static const TypeInfo xilinx_dp_info = {
> +    .name          = TYPE_XILINX_DP,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(XilinxDPState),
> +    .instance_init = xilinx_dp_init,
> +    .class_init    = xilinx_dp_class_init,
> +};
> +
> +static void xilinx_dp_register_types(void)
> +{
> +    type_register_static(&xilinx_dp_info);
> +}
> +
> +type_init(xilinx_dp_register_types)
> diff --git a/hw/display/xilinx_dp.h b/hw/display/xilinx_dp.h
> new file mode 100644
> index 0000000..44fdefd
> --- /dev/null
> +++ b/hw/display/xilinx_dp.h
> @@ -0,0 +1,129 @@
> +/*
> + * xilinx_dp.h
> + *
> + *  Copyright (C) 2015 : GreenSocs Ltd
> + *      http://www.greensocs.com/ , email: address@hidden
> + *
> + *  Developed by :
> + *  Frederic Konrad   <address@hidden>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "hw/sysbus.h"
> +#include "ui/console.h"
> +#include "hw/aux.h"
> +#include "hw/i2c/i2c.h"
> +#include "hw/display/dpcd.h"
> +#include "hw/i2c/i2c-ddc.h"
> +#include "qemu/fifo8.h"
> +#include "hw/dma/xilinx_dpdma.h"
> +#include "audio/audio.h"
> +
> +#ifndef XILINX_DP_H
> +#define XILINX_DP_H
> +
> +#define AUD_CHBUF_MAX_DEPTH                 32768

Is this related to the 32767 saturation logic and should this def be used there?

> +#define MAX_QEMU_BUFFER_SIZE                4096
> +
> +struct PixmanPlane {
> +    pixman_format_code_t format;
> +    DisplaySurface *surface;
> +};
> +

Does this concept belong in core code?

> +struct XilinxDPState {

/*< private >*/

> +    SysBusDevice parent_obj;

/*< public >*/

> +    MemoryRegion container;
> +
> +    /*
> +     * Registers for the Core.
> +     */

self documenting.

> +    uint32_t core_registers[0x3AF >> 2];
> +    MemoryRegion core_iomem;
> +
> +    /*
> +     * Registers for Audio Video Buffer Manager.
> +     */
> +    uint32_t avbufm_registers[0x238 >> 2];
> +    MemoryRegion avbufm_iomem;
> +
> +    /*
> +     * Register for Video Blender.
> +     */
> +    uint32_t vblend_registers[0x1DF >> 2];
> +    MemoryRegion vblend_iomem;
> +
> +    /*
> +     * Registers for Audio.
> +     */
> +    uint32_t audio_registers[0x50 >> 2];
> +    MemoryRegion audio_iomem;
> +
> +    QemuConsole *console;
> +
> +    /*
> +     * This is the planes used to display in console. When the blending is
> +     * enabled bout_plane is displayed in console else it's g_plane.
> +     */
> +    struct PixmanPlane g_plane;
> +    struct PixmanPlane v_plane;
> +    struct PixmanPlane bout_plane;
> +
> +    /*
> +     * Audio related.
> +     */
> +    QEMUSoundCard aud_card;
> +    SWVoiceOut *amixer_output_stream;
> +    int16_t audio_buffer_0[AUD_CHBUF_MAX_DEPTH];
> +    int16_t audio_buffer_1[AUD_CHBUF_MAX_DEPTH];
> +    size_t audio_data_available[2];
> +    int64_t temp_buffer[AUD_CHBUF_MAX_DEPTH];
> +    int16_t out_buffer[AUD_CHBUF_MAX_DEPTH];
> +    size_t byte_left; /* byte available in out_buffer. */
> +    size_t data_ptr;  /* next byte to be sent to QEMU. */
> +
> +    /*
> +     * Associated DPDMA controller.
> +     */
> +    XilinxDPDMAState *dpdma;
> +
> +    /*
> +     * IRQ.
> +     */

Self doc.

> +    qemu_irq irq;
> +
> +    /*
> +     * AUX bus.
> +     */

Self doc.

> +    AUXBus *aux_bus;
> +
> +    Fifo8 rx_fifo;
> +    Fifo8 tx_fifo;
> +
> +    uint32_t last_request;
> +
> +    /*
> +     * XXX: This should be in an other module.
> +     */

Probably the machine model.

Regards,
Peter

> +    DPCDState *dpcd;
> +    I2CDDCState *edid;
> +};
> +
> +typedef struct XilinxDPState XilinxDPState;
> +
> +#define TYPE_XILINX_DP "xlnx.v-dp"
> +#define XILINX_DP(obj) OBJECT_CHECK(XilinxDPState, (obj), TYPE_XILINX_DP)
> +
> +#endif /* !XILINX_DP_H */
> --
> 1.9.0
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]