From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36311) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuLXb-0003SC-9b for qemu-devel@nongnu.org; Mon, 18 May 2015 09:57:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YuLXX-0001wM-5H for qemu-devel@nongnu.org; Mon, 18 May 2015 09:57:11 -0400 Received: from greensocs.com ([193.104.36.180]:59688) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuLXW-0001w4-Pq for qemu-devel@nongnu.org; Mon, 18 May 2015 09:57:07 -0400 Message-ID: <5559EFAC.9050700@greensocs.com> Date: Mon, 18 May 2015 15:57:00 +0200 From: Frederic Konrad MIME-Version: 1.0 References: <1431544326-13372-1-git-send-email-fred.konrad@greensocs.com> <1431544326-13372-5-git-send-email-fred.konrad@greensocs.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/8] introduce dpcd module. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Mark Burton , "qemu-devel@nongnu.org Developers" , hyunk@xilinx.com On 14/05/2015 06:10, Peter Crosthwaite wrote: > On Wed, May 13, 2015 at 12:12 PM, wrote: >> From: KONRAD Frederic >> >> This introduces a DPCD modules. It wires on a aux-bus and can be accessed by >> driver to get lane-speed, etc. >> >> Signed-off-by: KONRAD Frederic >> --- >> hw/display/Makefile.objs | 1 + >> hw/display/dpcd.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++ >> hw/display/dpcd.h | 72 ++++++++++++++++++++++++ >> 3 files changed, 212 insertions(+) >> create mode 100644 hw/display/dpcd.c >> create mode 100644 hw/display/dpcd.h >> >> diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs >> index 3ea106d..f746cec 100644 >> --- a/hw/display/Makefile.objs >> +++ b/hw/display/Makefile.objs >> @@ -34,3 +34,4 @@ obj-$(CONFIG_CG3) += cg3.o >> obj-$(CONFIG_VGA) += vga.o >> >> common-obj-$(CONFIG_QXL) += qxl.o qxl-logger.o qxl-render.o >> +common-obj-y += dpcd.o >> diff --git a/hw/display/dpcd.c b/hw/display/dpcd.c >> new file mode 100644 >> index 0000000..757b65e >> --- /dev/null >> +++ b/hw/display/dpcd.c >> @@ -0,0 +1,139 @@ >> +/* >> + * dpcd.c >> + * >> + * Copyright (C)2015 : GreenSocs Ltd >> + * http://www.greensocs.com/ , email: info@greensocs.com >> + * >> + * Developed by : >> + * Frederic Konrad >> + * >> + * 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 . >> + * >> + */ >> + >> +/* >> + * This is a simple AUX slave which emulate a screen connected. >> + */ > emulates a connected screen. > >> + >> +#include "hw/aux.h" >> +#include "dpcd.h" >> + >> +/* #define DEBUG_DPCD */ >> +#ifdef DEBUG_DPCD >> +#define DPRINTF(fmt, ...) do { printf("dpcd: "fmt , ## __VA_ARGS__); } while (0) >> +#else >> +#define DPRINTF(fmt, ...) do {} while (0) >> +#endif >> + > Same comment as before about debug stuffs. > >> +struct DPCDState { >> + AUXSlave parent_obj; >> + >> + size_t current_reg; > What the actual size of the bus? This should be a fixed width type to match. Actually this is a mistake and is unused, so I just drop it. > >> + /* >> + * The DCPD is 0x7FFFF length but read as 0 after offset 0x600. >> + */ >> + uint8_t dpcd_info[0x600]; >> + >> + MemoryRegion iomem; >> +}; >> + >> +static void dpcd_realize(DeviceState *dev, Error **errp) >> +{ >> + >> +} > Blank realize not needed. Just leave the hook unset. Ok makes sense. > >> + >> +static uint64_t aux_read(void *opaque, hwaddr offset, unsigned size) > The fn name should match the name of the bus and not the attached dev. I guess you mean the opposite :). >> +{ >> + uint64_t ret; >> + DPCDState *e = DPCD(opaque); >> + assert(size == 1); > Is this a limitation of aux or the device? In the former, I would just > delete assertion. For the later it should be a GUEST_ERROR or UNIMP. > >> + >> + if (offset <= 0x600) { >> + ret = e->dpcd_info[offset]; >> + } else { >> + ret = 0; > GUEST_ERROR? Actually I think it's fine like that. The region size is 0x80000 the first 0x600 are used, the rest is read as zero. > >> + } >> + >> + DPRINTF("read %u @0x%8.8lX\n", (uint8_t)ret, offset); >> + return ret; >> +} >> + >> +static void aux_write(void *opaque, hwaddr offset, uint64_t value, >> + unsigned size) >> +{ >> + DPCDState *e = DPCD(opaque); >> + assert(size == 1); >> + >> + DPRINTF("write %u @0x%8.8lX\n", (uint8_t)value, offset); >> + >> + if (offset <= 0x600) { >> + e->dpcd_info[offset] = value; >> + } >> +} >> + >> +static const MemoryRegionOps aux_ops = { >> + .read = aux_read, >> + .write = aux_write > Chould you set your access width restrictions here instead of the assert? Yes, I do that. > >> +}; >> + >> +static void aux_edid_init(Object *obj) >> +{ >> + /* >> + * Create a default DPCD.. >> + */ >> + DPCDState *s = DPCD(obj); >> + >> + memset(&(s->dpcd_info), 0, sizeof(s->dpcd_info)); >> + >> + s->current_reg = 0; >> + >> + s->dpcd_info[0x00] = DPCD_REV_1_0; >> + s->dpcd_info[0x01] = DPCD_5_4GBPS; >> + s->dpcd_info[0x02] = 0x1; >> + s->dpcd_info[0x08] = DPCD_EDID_PRESENT; >> + s->dpcd_info[0x09] = 0xFF; >> + >> + /* CR DONE, CE DONE, SYMBOL LOCKED.. */ >> + s->dpcd_info[0x202] = 0x07; >> + /* INTERLANE_ALIGN_DONE.. */ >> + s->dpcd_info[0x204] = 0x01; >> + s->dpcd_info[0x205] = 0x01; >> + > State setup should be handled by a reset callback. > >> + /* >> + * Create the address-map. >> + */ >> + memory_region_init_io(&s->iomem, obj, &aux_ops, s, TYPE_DPCD, 0x7FFFF); >> + aux_init_mmio(AUX_SLAVE(obj), &s->iomem); >> +} >> + >> +static void aux_edid_class_init(ObjectClass *oc, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(oc); >> + dc->realize = dpcd_realize; > reset and vmsd hooks needed. > >> +} >> + >> +static const TypeInfo aux_edid_info = { >> + .name = TYPE_DPCD, >> + .parent = TYPE_AUX_SLAVE, >> + .instance_size = sizeof(DPCDState), >> + .instance_init = aux_edid_init, >> + .class_init = aux_edid_class_init, >> +}; >> + >> +static void aux_edid_register_types(void) >> +{ >> + type_register_static(&aux_edid_info); >> +} >> + >> +type_init(aux_edid_register_types) >> diff --git a/hw/display/dpcd.h b/hw/display/dpcd.h >> new file mode 100644 >> index 0000000..cd22258 >> --- /dev/null >> +++ b/hw/display/dpcd.h >> @@ -0,0 +1,72 @@ >> +/* >> + * dpcd.h >> + * >> + * Copyright (C)2015 : GreenSocs Ltd >> + * http://www.greensocs.com/ , email: info@greensocs.com >> + * >> + * Developed by : >> + * Frederic Konrad >> + * >> + * 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 . >> + * >> + */ >> + >> +#ifndef DPCD_H >> +#define DPCD_H >> + >> +typedef struct DPCDState DPCDState; >> + >> +#define TYPE_DPCD "dpcd" >> +#define DPCD(obj) OBJECT_CHECK(DPCDState, (obj), TYPE_DPCD) >> + >> +/* DCPD Revision. */ >> +#define DPCD_REV_1_0 0x10 >> +#define DPCD_REV_1_1 0x11 >> + >> +/* DCPD Max Link Rate. */ >> +#define DPCD_1_62GBPS 0x06 >> +#define DPCD_2_7GBPS 0x0A >> +#define DPCD_5_4GBPS 0x14 >> + >> +/* DCPD Max down spread. */ >> +#define DPCD_UP_TO_0_5 0x01 >> +#define DPCD_NO_AUX_HANDSHAKE_LINK_TRAINING 0x40 >> + >> +/* DCPD Downstream port type. */ >> +#define DPCD_DISPLAY_PORT 0x00 >> +#define DPCD_ANALOG 0x02 >> +#define DPCD_DVI_HDMI 0x04 >> +#define DPCD_OTHER 0x06 >> + > Tab constants out to consistent tab stop for readability. > > Regards, > Peter > >> +/* DPCD Format conversion. */ >> +#define DPCD_FORMAT_CONVERSION 0x08 >> + >> +/* Main link channel coding. */ >> +#define DPCD_ANSI_8B_10B 0x01 >> + >> +/* Down stream port count. */ >> +#define DPCD_OUI_SUPPORTED 0x80 >> + >> +/* Receiver port capability. */ >> +#define DPCD_EDID_PRESENT 0x02 >> +#define DPCD_ASSOCIATED_TO_PRECEDING_PORT 0x04 >> + >> +/* Down stream port capability. */ >> +#define DPCD_CAP_DISPLAY_PORT 0x000 >> +#define DPCD_CAP_ANALOG_VGA 0x001 >> +#define DPCD_CAP_DVI 0x002 >> +#define DPCD_CAP_HDMI 0x003 >> +#define DPCD_CAP_OTHER 0x100 >> + >> +#endif /* !DPCD_H */ >> -- >> 1.9.0 >> >>