[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <93e4d83d-9559-c987-d93b-c49572413275@collabora.com>
Date: Tue, 14 Mar 2023 13:25:28 +0000
From: Lucas Tanure <lucas.tanure@...labora.com>
To: Robin Murphy <robin.murphy@....com>,
Peter Geis <pgwipeout@...il.com>
Cc: Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Heiko Stuebner <heiko@...ech.de>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczynski <kw@...ux.com>,
Bjorn Helgaas <bhelgaas@...gle.com>, Qu Wenruo <wqu@...e.com>,
Piotr Oniszczuk <piotr.oniszczuk@...il.com>,
Kever Yang <kever.yang@...k-chips.com>,
linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH 1/7] irqchip/gic-v3: Add a DMA Non-Coherent flag
On 10-03-2023 12:04, Robin Murphy wrote:
> On 2023-03-10 11:41, Peter Geis wrote:
>> On Fri, Mar 10, 2023 at 3:05 AM Lucas Tanure
>> <lucas.tanure@...labora.com> wrote:
>>>
>>> The GIC600 integration in RK356x, used in rk3588, doesn't support
>>> any of the shareability or cacheability attributes, and requires
>>> both values to be set to 0b00 for all the ITS and Redistributor
>>> tables.
>>>
>>> This is loosely based on prior work from XiaoDong Huang and
>>> Peter Geis fixing this issue specifically for Rockchip 356x.
>>
>> Good Morning,
>>
>> Since the gic is using dma, would it be reasonable to have all memory
>> allocations be requested with the GFP_DMA flag? Otherwise this doesn't
>> fully solve the problem for rk356x, where only the lower 4GB range is
>> DMA capable, but this tends to get allocated in the upper 4GB on 8GB
>> boards.
>
> Not really, because there's no fixed definition of what GFP_DMA actually
> means, and it may mean nothing (same for GFP_DMA32, which may or may not
> be meaningful depending on kernel config and platform topology). Drivers
> should really use the DMA API allocation functions if they care about
> what they get, which comes back round to the notion from years ago of
> converting the ITS driver to a regular platform driver, so it can
> benefit from regular DT concepts like "dma-ranges" automatically.
>
> Thanks,
> Robin.
>
I am looking how to do that conversion to platform driver.
But about the communication between irq-gic-v3-its and irq-gic-v3.
Should irq-gic-v3-its be a MFD child of irq-gic-v3?
Or use the component bind/unbind framework?
>>
>> Very Respectfully,
>> Peter Geis
>>
>>>
>>> Suggested-by: Robin Murphy <robin.murphy@....com>
>>> Signed-off-by: Lucas Tanure <lucas.tanure@...labora.com>
>>> ---
>>> drivers/irqchip/irq-gic-v3-its.c | 22 ++++++++++++++++++++++
>>> 1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>>> b/drivers/irqchip/irq-gic-v3-its.c
>>> index 973ede0197e3..1c334dfeb647 100644
>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>> @@ -42,6 +42,7 @@
>>> #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0)
>>> #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1)
>>> #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
>>> +#define ITS_FLAGS_DMA_NON_COHERENT (1ULL << 3)
>>>
>>> #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
>>> #define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
>>> @@ -2359,6 +2360,13 @@ static int its_setup_baser(struct its_node
>>> *its, struct its_baser *baser,
>>> its_write_baser(its, baser, val);
>>> tmp = baser->val;
>>>
>>> + if (its->flags & ITS_FLAGS_DMA_NON_COHERENT) {
>>> + if (tmp & GITS_BASER_SHAREABILITY_MASK)
>>> + tmp &= ~GITS_BASER_SHAREABILITY_MASK;
>>> + else
>>> + gic_flush_dcache_to_poc(base,
>>> PAGE_ORDER_TO_SIZE(order));
>>> + }
>>> +
>>> if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
>>> /*
>>> * Shareability didn't stick. Just use
>>> @@ -3055,6 +3063,7 @@ static u64 its_clear_vpend_valid(void __iomem
>>> *vlpi_base, u64 clr, u64 set)
>>>
>>> static void its_cpu_init_lpis(void)
>>> {
>>> + struct its_node *its = list_first_entry(&its_nodes, struct
>>> its_node, entry);
>>> void __iomem *rbase = gic_data_rdist_rd_base();
>>> struct page *pend_page;
>>> phys_addr_t paddr;
>>> @@ -3096,6 +3105,9 @@ static void its_cpu_init_lpis(void)
>>> gicr_write_propbaser(val, rbase + GICR_PROPBASER);
>>> tmp = gicr_read_propbaser(rbase + GICR_PROPBASER);
>>>
>>> + if (its->flags & ITS_FLAGS_DMA_NON_COHERENT)
>>> + tmp &= ~GICR_PROPBASER_SHAREABILITY_MASK;
>>> +
>>> if ((tmp ^ val) & GICR_PROPBASER_SHAREABILITY_MASK) {
>>> if (!(tmp & GICR_PROPBASER_SHAREABILITY_MASK)) {
>>> /*
>>> @@ -3120,6 +3132,9 @@ static void its_cpu_init_lpis(void)
>>> gicr_write_pendbaser(val, rbase + GICR_PENDBASER);
>>> tmp = gicr_read_pendbaser(rbase + GICR_PENDBASER);
>>>
>>> + if (its->flags & ITS_FLAGS_DMA_NON_COHERENT)
>>> + tmp &= ~GICR_PENDBASER_SHAREABILITY_MASK;
>>> +
>>> if (!(tmp & GICR_PENDBASER_SHAREABILITY_MASK)) {
>>> /*
>>> * The HW reports non-shareable, we must remove the
>>> @@ -5005,6 +5020,7 @@ static int __init
>>> its_compute_its_list_map(struct resource *res,
>>> static int __init its_probe_one(struct resource *res,
>>> struct fwnode_handle *handle, int
>>> numa_node)
>>> {
>>> + struct device_node *np = to_of_node(handle);
>>> struct its_node *its;
>>> void __iomem *its_base;
>>> u64 baser, tmp, typer;
>>> @@ -5076,6 +5092,9 @@ static int __init its_probe_one(struct resource
>>> *res,
>>> its->get_msi_base = its_irq_get_msi_base;
>>> its->msi_domain_flags = IRQ_DOMAIN_FLAG_MSI_REMAP;
>>>
>>> + if (np && !of_dma_is_coherent(np))
>>> + its->flags |= ITS_FLAGS_DMA_NON_COHERENT;
>>> +
>>> its_enable_quirks(its);
>>>
>>> err = its_alloc_tables(its);
>>> @@ -5095,6 +5114,9 @@ static int __init its_probe_one(struct resource
>>> *res,
>>> gits_write_cbaser(baser, its->base + GITS_CBASER);
>>> tmp = gits_read_cbaser(its->base + GITS_CBASER);
>>>
>>> + if (its->flags & ITS_FLAGS_DMA_NON_COHERENT)
>>> + tmp &= ~GITS_CBASER_SHAREABILITY_MASK;
>>> +
>>> if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) {
>>> if (!(tmp & GITS_CBASER_SHAREABILITY_MASK)) {
>>> /*
>>> --
>>> 2.39.2
>>>
Powered by blists - more mailing lists