On Fri, Sep 4, 2015 at 2:01 AM, Frederic Konrad
<fred.kon...@greensocs.com> wrote:
> On 04/09/2015 01:34, Alistair Francis wrote:
>>
>> 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.
>
> Oh yes missed that when I replaced xilinx by xlnx in functions..
> I fixed that.
Great! Thanks
Alistair
>
> Thanks,
> Fred
>
>>
>> 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
>>>>>
>>>>>
>
