Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> > On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> > > On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> > >> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> > >>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> > >>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> > >>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> > >>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
> > >>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> > >>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> > >>>>>>>
> > >>>>>>> Will this call work on platforms without patched ATF ?
> > >>>>>>> (I think not, don't you need to handle return value?)
> > >>>>>>
> > >>>>>> I have not actually tested that, but if I understand the ATF code
> > >>>>>> correctly, unimplemented calls return
> > >>>>>> SMC_UNK (0xffffffff), which should be handled by the default case
> > >>>>>> (NOP)
> > >>>>>> below.
> > >>>>>
> > >>>>> Which means the board has a memory size of 0 and fails to boot ?
> > >>>>>
> > >>>>>>>> +             switch (res.a0) {
> > >>>>>>>> +             case 1:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x038000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x040000000ULL;
> > >>>>>>>> +                     base[2] = 0x600000000ULL;
> > >>>>>>>> +                     size[2] = 0x040000000ULL;
> > >>>>>>>> +                     base[3] = 0x700000000ULL;
> > >>>>>>>> +                     size[3] = 0x040000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> > >>>>>>>> +                     break;
> > >>>>>>>> +             case 2:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x078000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x080000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> > >>>>>>>> +                     break;
> > >>>>>>>> +             case 3:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x078000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x080000000ULL;
> > >>>>>>>> +                     base[2] = 0x600000000ULL;
> > >>>>>>>> +                     size[2] = 0x080000000ULL;
> > >>>>>>>> +                     base[3] = 0x700000000ULL;
> > >>>>>>>> +                     size[3] = 0x080000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> > >>>>>>>> +                     break;
> > >>>>>>>
> > >>>>>>> Obvious design question is -- since you're adding new SMC call
> > >>>>>>> anyway,
> > >>>>>>> can't the call just return the memory layout table itself, so that
> > >>>>>>> it
> > >>>>>>> won't be duplicated both in U-Boot and ATF ?
> > >>>>>>
> > >>>>>> My gut feeling was to go with the smallest interface possible.
> > >>>>>
> > >>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> > >>>>> identify memory layout tables which have to be encoded both in ATF and
> > >>>>> U-Boot, both of which must be kept in sync.
> > >>>>>
> > >>>>> The ATF already has those memory layout tables, it's only a matter of
> > >>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
> > >>>>> encoding of tables into U-Boot goes away and in fact simplifies the
> > >>>>> design.
> > >>>>>
> > >>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> > >>>>> standard SMC call to get memory topology. It surprises me that it
> > >>>>> wouldn't.
> > >>>>
> > >>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> > >>>> decomp
> > >>>> and I think this involved some passing of memory layout information
> > >>>> from
> > >>>> ATF to U-Boot too, or am I mistaken ?
> > >>>
> > >>> That's correct, ATF stores information about the memory layout at a
> > >>> fixed
> > >>> address in system memory, and U-Boot can read it.
> > >>
> > >> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> > >> then? :)
> > >
> > > I'd prefer that, yes.
> > >
> > > Let's not duplicate the mechanism used to pass FCNL information from ATF
> > > to U- Boot, but instead create a data table format that can store all the
> > > information we need, in an easily extensible way.
> > >
> > > To see how the mechanism is implemented for FCNL, search for 47FD7000 in
> > > the Renesas ATF sources
> > > (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> > For everyone involved, can you explain what FCNL is ? ;-)
>
> FCNL is Frame Compression Near Lossless. It's a way to reduce memory bandwidth
> by transparent compression and decompression of video frames. Compression is
> handled by an IP core called FCP, and decompression is handled by the DRAM
> controller. ATF programs the DRAM controller with ranges of memory addresses
> that will be dynamically decompressed. The registers containing those ranges
> are accessible in secure mode only, so neither U-Boot nor Linux can read them.
> That's why ATF has to pass the information to U-Boot, in order to add the
> ranges as reserved memory in DT.
>
> > Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> > about passing the memory configuration around and the result is
> > basically the same -- pass a table from ATF to U-Boot. If there's
> > already something, great.

Pass a "table"? Or an FDT containing the /memory nodes?
The latter allows for easier future extension.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux