On Thu, Sep 3, 2015 at 12:28 AM, Frederic Konrad
<fred.kon...@greensocs.com> wrote:
> On 02/09/2015 23:39, Alistair Francis wrote:
>>
>> On Tue, Jul 21, 2015 at 10:17 AM, <fred.kon...@greensocs.com> wrote:
>>>
>>> From: KONRAD Frederic <fred.kon...@greensocs.com>
>>>
>>> This is the implementation of the DPDMA.
>>>
>>> Signed-off-by: KONRAD Frederic <fred.kon...@greensocs.com>
>>> ---
>>> hw/dma/Makefile.objs | 1 +
>>> hw/dma/xlnx_dpdma.c | 790
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> hw/dma/xlnx_dpdma.h | 85 ++++++
>>> 3 files changed, 876 insertions(+)
>>> create mode 100644 hw/dma/xlnx_dpdma.c
>>> create mode 100644 hw/dma/xlnx_dpdma.h
>>
>> What are the rules with the placement of the header files? Should the
>> header be in the include directory instead of being here?
>
> Probably moving headers to include/hw makes sense here..
> I will change it.
Hey Fred,
Great, thanks
>
> [...]
>>>
>>> +
>>> + if (xlnx_dpdma_desc_is_already_done(&desc)
>>> + && !xlnx_dpdma_desc_ignore_done_bit(&desc)) {
>>
>> The second line of the if should be indented across.
>>
>> Also, I generally think the operator should go on the first line, but
>> that doesn't seem worth changing throughout.
>
> Ok I can do that.
>
>>
>>> + /* We are trying to process an already processed descriptor.
>>> */
>>> + s->registers[DPDMA_EISR] |= ((1 << 25) << channel);
>>> + xlnx_dpdma_update_irq(s);
>>> + s->operation_finished[channel] = true;
>>> + DPRINTF("Already processed descriptor..\n");
>>> + break;
>>> + }
>>> +
>>> + done = xlnx_dpdma_desc_is_last(&desc)
>>> + || xlnx_dpdma_desc_is_last_of_frame(&desc);
>>> +
>>> + s->operation_finished[channel] = done;
>>> + if (s->data[channel]) {
>>> + int64_t transfer_len =
>>> +
>>> xlnx_dpdma_desc_get_transfer_size(&desc);
>>
>> Why is this over two lines?
>
>
> Probably because of the 79~80 columns limitation.
It looks like it is less then 80 columns though.
Thanks,
Alistair
>
> Thanks,
> Fred
>
>>
>> Functioanlly it looks fine.
>>
>> Besides the header file location, which someone else will need to comment
>> on:
>>
>> Reviewed-by: Alistair Francis <alistair.fran...@xilinx.com>
>> Tested-By: Hyun Kwon <hyun.k...@xilinx.com>
>>
>> Thanks,
>>
>> Alistair
>>
>>> + uint32_t line_size = xlnx_dpdma_desc_get_line_size(&desc);
>>> + uint32_t line_stride =
>>> xlnx_dpdma_desc_get_line_stride(&desc);
>>> + if (xlnx_dpdma_desc_is_contiguous(&desc)) {
>>> + source_addr[0] =
>>> + xlnx_dpdma_desc_get_source_address(&desc,
>>> 0);
>>> + while (transfer_len != 0) {
>>> + if (dma_memory_read(&address_space_memory,
>>> + source_addr[0],
>>> + &s->data[channel][ptr],
>>> + line_size)) {
>>> + s->registers[DPDMA_ISR] |= ((1 << 12) <<
>>> channel);
>>> + xlnx_dpdma_update_irq(s);
>>> + DPRINTF("Can't get data.\n");
>>> + break;
>>> + }
>>> + ptr += line_size;
>>> + transfer_len -= line_size;
>>> + source_addr[0] += line_stride;
>>> + }
>>> + } else {
>>> + DPRINTF("Source address:\n");
>>> + int frag;
>>> + for (frag = 0; frag < 5; frag++) {
>>> + source_addr[frag] =
>>> + xlnx_dpdma_desc_get_source_address(&desc,
>>> frag);
>>> + DPRINTF("Fragment %u: %" PRIx64 "\n", frag + 1,
>>> + source_addr[frag]);
>>> + }
>>> +
>>> + frag = 0;
>>> + while ((transfer_len < 0) && (frag < 5)) {
>>> + size_t fragment_len = DPDMA_FRAG_MAX_SZ
>>> + - (source_addr[frag] %
>>> DPDMA_FRAG_MAX_SZ);
>>> +
>>> + if (dma_memory_read(&address_space_memory,
>>> + source_addr[frag],
>>> + &(s->data[channel][ptr]),
>>> + fragment_len)) {
>>> + s->registers[DPDMA_ISR] |= ((1 << 12) <<
>>> channel);
>>> + xlnx_dpdma_update_irq(s);
>>> + DPRINTF("Can't get data.\n");
>>> + break;
>>> + }
>>> + ptr += fragment_len;
>>> + transfer_len -= fragment_len;
>>> + frag += 1;
>>> + }
>>> + }
>>> + }
>>> +
>>> + if (xlnx_dpdma_desc_update_enabled(&desc)) {
>>> + /* The descriptor need to be updated when it's completed. */
>>> + DPRINTF("update the descriptor with the done flag set.\n");
>>> + xlnx_dpdma_desc_set_done(&desc);
>>> + dma_memory_write(&address_space_memory, desc_addr, &desc,
>>> + sizeof(DPDMADescriptor));
>>> + }
>>> +
>>> + if (xlnx_dpdma_desc_completion_interrupt(&desc)) {
>>> + DPRINTF("completion interrupt enabled!\n");
>>> + s->registers[DPDMA_ISR] |= (1 << channel);
>>> + xlnx_dpdma_update_irq(s);
>>> + }
>>> +
>>> + } while (!done && !one_desc);
>>> +
>>> + return ptr;
>>> +}
>>> +
>>> +void xlnx_dpdma_set_host_data_location(XlnxDPDMAState *s, uint8_t
>>> channel,
>>> + void *p)
>>> +{
>>> + if (!s) {
>>> + qemu_log_mask(LOG_UNIMP, "DPDMA client not attached to valid
>>> DPDMA"
>>> + " instance\n");
>>> + return;
>>> + }
>>> +
>>> + assert(channel <= 5);
>>> + s->data[channel] = p;
>>> +}
>>> +
>>> +void xlnx_dpdma_trigger_vsync_irq(XlnxDPDMAState *s)
>>> +{
>>> + s->registers[DPDMA_ISR] |= (1 << 27);
>>> + xlnx_dpdma_update_irq(s);
>>> +}
>>> +
>>> +type_init(xlnx_dpdma_register_types)
>>> diff --git a/hw/dma/xlnx_dpdma.h b/hw/dma/xlnx_dpdma.h
>>> new file mode 100644
>>> index 0000000..ae571a0
>>> --- /dev/null
>>> +++ b/hw/dma/xlnx_dpdma.h
>>> @@ -0,0 +1,85 @@
>>> +/*
>>> + * xlnx_dpdma.h
>>> + *
>>> + * Copyright (C) 2015 : GreenSocs Ltd
>>> + * 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/>.
>>> + *
>>> + */
>>> +
>>> +#ifndef XLNX_DPDMA_H
>>> +#define XLNX_DPDMA_H
>>> +
>>> +#include "hw/sysbus.h"
>>> +#include "ui/console.h"
>>> +#include "sysemu/dma.h"
>>> +
>>> +#define XLNX_DPDMA_REG_ARRAY_SIZE (0x1000 >> 2)
>>> +
>>> +struct XlnxDPDMAState {
>>> + /*< private >*/
>>> + SysBusDevice parent_obj;
>>> + /*< public >*/
>>> + MemoryRegion iomem;
>>> + uint32_t registers[XLNX_DPDMA_REG_ARRAY_SIZE];
>>> + uint8_t *data[6];
>>> + bool operation_finished[6];
>>> + qemu_irq irq;
>>> +};
>>> +
>>> +typedef struct XlnxDPDMAState XlnxDPDMAState;
>>> +
>>> +#define TYPE_XLNX_DPDMA "xlnx.dpdma"
>>> +#define XLNX_DPDMA(obj) OBJECT_CHECK(XlnxDPDMAState, (obj),
>>> TYPE_XLNX_DPDMA)
>>> +
>>> +/*
>>> + * xlnx_dpdma_start_operation: Start the operation on the specified
>>> channel. The
>>> + * DPDMA gets the current descriptor and
>>> retrieves
>>> + * data to the buffer specified by
>>> + * dpdma_set_host_data_location().
>>> + *
>>> + * Returns The number of bytes transfered by the DPDMA or 0 if an error
>>> occured.
>>> + *
>>> + * @s The DPDMA state.
>>> + * @channel The channel to start.
>>> + */
>>> +size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel,
>>> + bool one_desc);
>>> +
>>> +/*
>>> + * xlnx_dpdma_set_host_data_location: Set the location in the host
>>> memory where
>>> + * to store the data out from the dma
>>> + * channel.
>>> + *
>>> + * @s The DPDMA state.
>>> + * @channel The channel associated to the pointer.
>>> + * @p The buffer where to store the data.
>>> + */
>>> +/* XXX: add a maximum size arg and send an interrupt in case of
>>> overflow. */
>>> +void xlnx_dpdma_set_host_data_location(XlnxDPDMAState *s, uint8_t
>>> channel,
>>> + void *p);
>>> +
>>> +/*
>>> + * xlnx_dpdma_trigger_vsync_irq: Trigger a VSYNC IRQ when the display is
>>> + * updated.
>>> + *
>>> + * @s The DPDMA state.
>>> + */
>>> +void xlnx_dpdma_trigger_vsync_irq(XlnxDPDMAState *s);
>>> +
>>> +#endif /* !XLNX_DPDMA_H */
>>> --
>>> 1.9.0
>>>
>>>
>
