On Mon, Jul 6, 2015 at 9:30 AM, Frederic Konrad
<fred.kon...@greensocs.com> wrote:
> On 24/06/2015 08:44, Peter Crosthwaite wrote:
>>
>> On Mon, Jun 15, 2015 at 8:15 AM, <fred.kon...@greensocs.com> wrote:
>>>
>>> From: KONRAD Frederic <fred.kon...@greensocs.com>
>>>
>>> This introduces a DPCD modules. It wires on a aux-bus and can be accessed
>>> by
>>
>> "module"
>>
>>> driver to get lane-speed, etc.
>>>
>>> Signed-off-by: KONRAD Frederic <fred.kon...@greensocs.com>
>>> ---
>>> hw/display/Makefile.objs | 1 +
>>> hw/display/dpcd.c | 151
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>> hw/display/dpcd.h | 72 ++++++++++++++++++++++
>>> 3 files changed, 224 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 61c80f3..f75094f 100644
>>> --- a/hw/display/Makefile.objs
>>> +++ b/hw/display/Makefile.objs
>>> @@ -36,3 +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
>>
>> Make a DPCD config and add to aarch64 defconfig.
>>
>>> diff --git a/hw/display/dpcd.c b/hw/display/dpcd.c
>>> new file mode 100644
>>> index 0000000..b4eeea7
>>> --- /dev/null
>>> +++ b/hw/display/dpcd.c
>>> @@ -0,0 +1,151 @@
>>> +/*
>>> + * dpcd.c
>>> + *
>>> + * Copyright (C)2015 : GreenSocs Ltd
>>
>> (C) 2015
>> (missing space)
>>
>>> + * http://www.greensocs.com/ , email: i...@greensocs.com
>>> + *
>>> + * Developed by :
>>> + * Frederic Konrad <fred.kon...@greensocs.com>
>>> + *
>>> + * 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/>.
>>> + *
>>> + */
>>> +
>>> +/*
>>> + * This is a simple AUX slave which emulates a connected screen.
>>> + */
>>> +
>>> +#include "hw/aux.h"
>>> +#include "dpcd.h"
>>> +
>>> +#ifndef DEBUG_DPCD
>>> +#define DEBUG_DPCD 0
>>> +#endif
>>> +
>>> +#define DPRINTF(fmt, ...) do {
>>> \
>>> + if (DEBUG_DPCD) {
>>> \
>>> + qemu_log("dpcd: " fmt, ## __VA_ARGS__);
>>> \
>>> + }
>>> \
>>> +} while (0);
>>> +
>>> +#define DPCD_READABLE_AREA 0x600
>>> +
>>> +struct DPCDState {
>>
>> /*< public >*/
>>
>>> + AUXSlave parent_obj;
>>> +
>>
>> /*< private >*/
>>
>>> + /*
>>> + * The DCPD is 0x7FFFF length but read as 0 after offset 0x5FF.
>>> + */
>>> + uint8_t dpcd_info[DPCD_READABLE_AREA];
>>> +
>>> + MemoryRegion iomem;
>>> +};
>>> +
>>> +static uint64_t dpcd_read(void *opaque, hwaddr offset, unsigned size)
>>> +{
>>> + uint64_t ret;
>>
>> make a uint8_t
>>
>>> + DPCDState *e = DPCD(opaque);
>>> +
>>> + if (offset < DPCD_READABLE_AREA) {
>>> + ret = e->dpcd_info[offset];
>>> + } else {
>>> + ret = 0;
>>> + }
>>> +
>>> + DPRINTF("read %u @0x%8.8lX\n", (uint8_t)ret, offset);
>>
>> to avoid this cast, and just let the implicit cast on the return
>> handle it for you.
>>
>> PRIx8
>> HWADDR_PRIx
>>
>>> + return ret;
>>> +}
>>> +
>>> +static void dpcd_write(void *opaque, hwaddr offset, uint64_t value,
>>> + unsigned size)
>>> +{
>>> + DPCDState *e = DPCD(opaque);
>>> +
>>> + DPRINTF("write %u @0x%8.8lX\n", (uint8_t)value, offset);
>>> +
>>
>> PRIx8
>> HWADDR_PRIx
>>
>>> + if (offset < DPCD_READABLE_AREA) {
>>> + e->dpcd_info[offset] = value;
>>> + }
>>
>> Should there be a else for a guest error?
>
>
> I think it's fine. Seems it's accessible but just read as 0.
>
So guest error is this semantic, it is not limited to only undefs, it
also applied to OOB registers that have a defined background. This
goal is to let the user know "the guest is doing something that
doesn't make sense" rather than trap undef behaviour.
Regards,
Peter
>>
>>> +}
>>> +
>>> +static const MemoryRegionOps aux_ops = {
>>> + .read = dpcd_read,
>>> + .write = dpcd_write,
>>> + .valid = {
>>> + .min_access_size = 1,
>>> + .max_access_size = 1,
>>> + },
>>> + .impl = {
>>> + .min_access_size = 1,
