* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: David Collins @ 2018-05-19 0:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=U-QOQekUHj=ck57YJbTFdFjCAWr4t2A92x+fCNf+yV9A@mail.gmail.com>
On 05/17/2018 06:01 PM, Doug Anderson wrote:
> On Thu, May 17, 2018 at 5:16 PM, David Collins <collinsd@codeaurora.org> wrote:
>> On 05/17/2018 02:22 PM, Doug Anderson wrote:
>>> On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> wrote:
>>>> +- qcom,regulator-initial-microvolt
>>>> + Usage: optional; VRM regulators only
>>>> + Value type: <u32>
>>>> + Definition: Specifies the initial voltage in microvolts to request for a
>>>> + VRM regulator.
>>>
>>> Now that Mark has landed the patch adding support for the
>>> -ENOTRECOVERABLE error code from get_voltage() / get_voltage_sel(), do
>>> we still need the qcom,regulator-initial-microvolt property?
>>
>> Yes, this is still needed. The -ENOTRECOVERABLE patch ensures that
>> qcom-rpmh-regulator devices can be registered even if
>> qcom,regulator-initial-microvolt is not specified. However, that will
>> result in the regulators being configured for the minimum voltage
>> supported in the DT specified min/max range. The
>> qcom,regulator-initial-microvolt property allows us to set a specific
>> voltage that is larger than the min constraint.
>
> Ah, OK. In the device tree fragment I saw the initial was always
> equal to the min, so I wasn't sure if this was really needed in
> practice. I presume it would only be important if a voltage was left
> high by the bootloader for some peripheral that needs to continue to
> function (and use the existing higher voltage) until a real device
> claims it. For all other voltages, it should be fine if it's set to
> the min until a real device claims it. Do you have real examples of
> devices like this in boards using sdm845?
Something to keep in mind about the downstream rpmh-regulator driver is
that it caches the initial voltages specified in device tree and only
sends them after a consumer driver makes a regulator framework call. This
saves time during boot and ensures that requests are not made for
regulators that no Linux consumer cares about.
It is generally not safe to request all regulators to be set to the
minimum allowed voltage. Special care will be needed with the upstream
qcom-rpmh-regulator driver to avoid disrupting the boot up state of
regulators that are needed by other subsystems. Therefore, I would like
to keep the initial voltage feature supported.
>>>> +- regulator-initial-mode
>>>> + Usage: optional; VRM regulators only
>>>> + Value type: <u32>
>>>> + Definition: Specifies the initial mode to request for a VRM regulator.
>>>> + Supported values are RPMH_REGULATOR_MODE_* which are defined
>>>> + in [1] (i.e. 0 to 3). This property may be specified even
>>>> + if the regulator-allow-set-load property is not specified.
>>>
>>> Every time I read the above I wonder why you're documenting a standard
>>> regulator regulator property in your bindings. ...then I realize it's
>>> because you're doing it because you want to explicitly document what
>>> the valid modes are. I wonder if it makes sense to just put a
>>> reference somewhere else in this document to go look at the header
>>> file where these are all nicely documented.
>>
>> Isn't that what the [1] in the above snippet is currently doing. Further
>> down in qcom,rpmh-regulator.txt is this line:
>>
>> +[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
>
> Right, but I want to move it so it doesn't look like you're defining a
> property that's already defined in the common bindings. AKA get rid
> of the "regulator-initial-mode" property description. Then add above
> Examples:
>
> ========================
> Regulator Modes
> ========================
>
> RPMh regulators are designed to work with the standard regulator mode
> bindings, using properties like "regulator-initial-mode". See
> include/dt-bindings/regulator/qcom,rpmh-regulator.h for information on
> the modes relevant to RPMh regulators.
>
> Some RPMh regulators (BOB regulators only) also support bypass using
> the standard "regulator-allow-bypass" binding.
>
>
> ...feel fee to reword, but basically the idea is to document it but
> not make it look like you're defining a novel property.
Ok, I'll try rewording the mode explanation and move it into another
section of the binding doc.
>>> Speaking of documenting things like that, it might be worth finding
>>> somewhere in this doc to mention that the "bob" regulator on PMI8998
>>> can support "regulator-allow-bypass". That tidbit got lost when we
>>> moved to the standard regulator bindings for bypass.
>>
>> I suppose that I could add something like this:
>>
>> +- regulator-allow-bypass
>> + Usage: optional; BOB type VRM regulators only
>> + Value type: <empty>
>> + Definition: See [2] for details.
>> ...
>> +[2]: Documentation/devicetree/bindings/regulator.txt
>>
>> However, I don't want the patch to get NACKed because it is defining a
>> property that is already defined in the common regulator.txt file.
>
> See above for my suggestion.
Ok.
>>>> +- qcom,allowed-drms-modes
>>>> + Usage: required if regulator-allow-set-load is specified;
>>>> + VRM regulators only
>>>> + Value type: <prop-encoded-array>
>>>> + Definition: A list of integers specifying the PMIC regulator modes which
>>>> + can be configured at runtime based upon consumer load needs.
>>>> + Supported values are RPMH_REGULATOR_MODE_* which are defined
>>>> + in [1] (i.e. 0 to 3).
>>>
>>> Why is this still here? You moved it to the core regulator framework,
>>> right? It's still in your examples too. Shouldn't this be removed?
>>> It looks like the driver still needs this and it needs to be an exact
>>> duplicate of the common binding. That doesn't seem right...
>>
>> The qcom,allowed-drms-modes property supports a different feature than the
>> regulator-allowed-modes property accepted in [2]. The latter specifies
>> the modes that may be used at all (e.g. in regulator_set_mode() calls) and
>> it lists the mode values in an unordered fashion.
>>
>> qcom,allowed-drms-modes defines a specific subset of the possible allowed
>> modes that should be set based on DRMS (e.g. in regulator_set_load()
>> calls). Its values are listed in a specific order and must match 1-to-1
>> with qcom,drms-mode-max-microamps entries.
>>
>> It would probably be good to change the name of the property from
>> qcom,allowed-drms-modes to qcom,regulator-drms-modes.
>
> Ah, I see. It's unfortunate that now we need to effectively list all
> modes twice. Have you seen real-life examples where these sets of
> modes need to be different, or is this just theoretical? If not can
> we start with one property (that controls both things) and if we
> really see that we need to specify different sets of modes for the two
> cases we can add a separate property? ...actually, even if you do
> have real-life examples of where these need to be different, if 90% of
> the time they are the same it would still be nice to just have one
> property apply to both cases.
I plan to keep qcom,regulator-drms-modes (and
qcom,drms-mode-max-microamps) around as a property specifically handled
for qcom-rpmh-regulator. It serves a purpose that is distinct from that
of the generic regulator-allowed-modes. Without it, there will not be a
way to utilize regulator_set_load() to configure the regulator modes.
>>>> +- qcom,drms-mode-max-microamps
>>>> + Usage: required if regulator-allow-set-load is specified;
>>>> + VRM regulators only
>>>> + Value type: <prop-encoded-array>
>>>> + Definition: A list of integers specifying the maximum allowed load
>>>> + current in microamps for each of the modes listed in
>>>> + qcom,allowed-drms-modes (matched 1-to-1 in order). Elements
>>>> + must be specified in order from lowest to highest value.
>>>
>>> Any reason this can't go into the regulator core? You'd basically
>>> just take the existing concept of rpmh_regulator_vrm_set_load() and
>>> put it in the core.
>>
>> This could be implemented in the core via new constraint elements parsed
>> in of_regulator and a helper function to specify in regulator_ops.
>> However, I'm not sure about the wide-spread applicability of this feature.
>> I'd prefer to leave it in the driver unless Mark would like me to add it
>> into the core.
>
> You're already using pre-existing APIs around specifying the current
> and having the regulator core call you to map the total current into a
> mode. That implies that this is applicable to others. Adding this
> tiny amount of code to the core makes the pre-existing APIs generally
> useful.
I don't see the benefit of making struct regulation_constraints more
complicated with DRMS mode and current arrays that would only every be
used by the qcom-rpmh-regulator driver. Other regulator drivers are able
to hard code this information in the driver code using get_optimum_mode()
callbacks.
As a side note, changing qcom-rpmh-regulator to use a get_optimum_mode()
callback instead of a set_load() callback would probably be a good idea too.
>>>> +- qcom,headroom-microvolt
>>>> + Usage: optional; VRM regulators only
>>>> + Value type: <u32>
>>>> + Definition: Specifies the headroom voltage in microvolts to request for
>>>> + a VRM regulator. RPMh hardware automatically ensures that
>>>> + the parent of this regulator outputs a voltage high enough
>>>> + to satisfy the requested headroom. Supported values are
>>>> + 0 to 511000.
>>>
>>> I'm curious: is this a voted-for value, or a global value?
>>>
>>> Said another way: the whole point of RPMh is that there may be more
>>> than one processor that needs the same rails, right? So the AP might
>>> request 1.1 V for a rail and the modem might request 1.3 V. RPMh
>>> would decide to pick the higher of those two (1.3 V), but if the modem
>>> said it no longer needs the rail it will drop down to 1.1 V.
>>>
>>> ...and as an example of why the headroom needs to be in hardware, if
>>> the source voltage was normally 1.4 V and the headroom was 200 mV then
>>> the hardware would need to know to bump up the source voltage to 1.5V
>>> during the period of of time that the modem wants the rail at 1.3V.
>>>
>>> So my question is: do the AP and modem in the above situation
>>> separately vote for headroom? How is it aggregated? ...or is it a
>>> global value and this sets the headroom for all clients of RPMh? It
>>> would be interesting to document this as it might help with figuring
>>> out how this value should be set.
>>
>> The headroom voltage voting is supported in hardware per-regulator and
>> per-master (AP, modem, etc). The headroom voltage and output voltage are
>> each aggregated (using max) per-regulator across masters. If the
>> aggregated enable state for a regulator is on, then the aggregated output
>> voltage and headroom voltage are added together and applied as a min
>> constraint on the parent's output voltage (if there is a parent).
>
> Ah, interesting. I'm not 100% convinced that the RPMh API is at the
> right abstraction level here. I guess you increase the headroom
> voltage if you expect a lot of current and need the regulator to still
> give a clean signal? If you truly wanted to aggregate then if both
> the modem and AP wanted to draw a lot of current they would both need
> to increase the headroom and then the headroom should maybe not be the
> max but something slightly more (you wouldn't want to add, but ...)
>
> Since it's just a max, in theory it seems like you get 99% of the way
> there by just using the Linux APIs to deal with dropout voltage. If
> Linux was managing it in software then if it needed to account for
> extra headroom it would just increase the supply voltage. That should
> play just fine with the modem (which might be using the hardware
> headroom feature) since it will be making its own completely separate
> requests and they should be aggregated OK.
>
> In another thread you said you'd be OK dropping the headroom voltage
> since it wasn't needed on SDM845. Maybe we should do that? ...and if
> someone later needs to account for a larger dropout they can figure
> out how to hookup the standard linux min_dropout_uV?
I will remove qcom,headroom-microvolt.
Take care,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [linux-sunxi] Re: [PATCH v4 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet
From: Brüns, Stefan @ 2018-05-19 0:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518071436.3vvyllghrdxptsxg@flea>
On Freitag, 18. Mai 2018 09:14:36 CEST Maxime Ripard wrote:
> On Mon, May 14, 2018 at 10:36:08PM +0200, Paul Kocialkowski wrote:
> > > > + backlight: backlight {
> > > > + compatible = "pwm-backlight";
> > > > + pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> > > > + brightness-levels = < 0 1 1 1 1 2 2 2
> > > > + 2 3 3 3 3 4 4 4
> > > > + 5 5 5 6 6 6 7 7
> > > > + 8 8 8 9 9 9 10 10
> > > > + 10 11 11 12 12 12 13 13
> > > > + 14 14 14 15 15 16 16 17
> > > > + 17 17 18 18 19 19 20 20
> > > > + 21 21 21 22 22 23 23 24
> > > > + 24 25 25 26 26 27 27 28
> > > > + 28 29 30 30 31 31 32 32
> > > > + 33 33 34 35 35 36 36 37
> > > > + 38 38 39 39 40 41 41 42
> > > > + 43 43 44 44 45 46 47 47
> > > > + 48 49 49 50 51 51 52 53
> > > > + 54 54 55 56 57 57 58 59
> > > > + 60 61 61 62 63 64 65 65
> > > > + 66 67 68 69 70 71 71 72
> > > > + 73 74 75 76 77 78 79 80
> > > > + 81 82 83 84 85 86 87 88
> > > > + 89 90 91 92 93 94 95 96
> > > > + 97 98 99 101 102 103 104 105
> > > > + 106 108 109 110 111 112 114 115
> > > > + 116 117 119 120 121 123 124 125
> > > > + 127 128 129 131 132 133 135 136
> > > > + 138 139 141 142 144 145 147 148
> > > > + 150 151 153 154 156 157 159 161
> > > > + 162 164 166 167 169 171 173 174
> > > > + 176 178 180 181 183 185 187 189
> > > > + 191 192 194 196 198 200 202 204
> > > > + 206 208 210 212 214 216 219 221
> > > > + 223 225 227 229 232 234 236 238
> > > > + 241 242 244 246 248 250 253 255>;
> > >
> > > You kind of overdid it here :)
> > >
> > > What I meant to say before was that if you have 10 elements (and you
> > > really should have something in that magnitude) each step should
> > > increase the perceived brightness by 10%.
> >
> > Mhh I think 10 elements would fall too short to really depict the curve
> > with appropriate precision. Given the usual size for brightness cursors
> > in e.g. gnome-shell, it feels like a bigger number would be more
> > appropriate. Let's make it to 100 with values from 0 to 255!
> >
> > > In this particular case, I really think having something close to <0 4
> > > 8 16 32 64 128 255> would be enough.
> > >
> > > And in general, that kind of odd looking table without any more
> > > context is just screaming for a comment :)
> >
> > Noted, I will explain the idea, but probably without the exact formula
> > that's really a nasty hack written down on a piece of paper sitting in
> > my garbage at this point.
>
> So no one will ever be able to understand where this sequence comes
> from (yourself-in-two-years included). That sounds like a pretty bad
> idea.
>
> Maxime
The following formula yields practically the same table:
out = ceil(255 * (0.245 * in/255 + 0.755 * pow(in/255, 2.6) ))
Maximum error: 4, maximum relative error: 0.33
Kind regards,
Stefan
^ permalink raw reply
* [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-18 23:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHp75VdNUW6KoM6oupyQ80A1WVRk7vewwDt6WEZOyjrAUifqRg@mail.gmail.com>
On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
>
> <jmkrzyszt@gmail.com> wrote:
> > + gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> > + if (!IS_ERR_OR_NULL(gpiod_rdy)) {
>
> So, is it optional or not at the end?
> If it is, why do we check for NULL?
As far as I can understand, nand_chip->dev_ready() callback is optional.
That's why I decided to use the _optional variant of devm_gpiod_get(). In case
of ams-delta, the dev_ready() callback depends on availability of the 'rdy'
GPIO pin. As a consequence, I'm checking for both NULL and ERR in order to
decide if dev_ready() will be supported.
I can pretty well replace it with the standard form and check for ERR only if
the purpose of the _optional form is different.
> > this->dev_ready = ams_delta_nand_ready;
> >
> > } else {
> >
> > this->dev_ready = NULL;
> > pr_notice("Couldn't request gpio for Delta NAND
> > ready.\n");
>
> dev_notice() ?
Sure, but maybe in a separate patch? That's not a new code just being added
but an existing one, not the merit of the change.
> > }
> >
> > +err_gpiod:
> > + if (err == -ENODEV || err == -ENOENT)
> > + err = -EPROBE_DEFER;
>
> Hmm...
Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
availble before device init phase, unlike other crucial GPIO drivers which are
initialized earlier, e.g. during the postcore or at latetst the subsys phase.
Hence, devices which depend on GPIO pins provided by gpio-mmio must either be
declared late or fail softly so they get another chance of being probed
succesfully.
I thought of replacing the gpio-mmio platform driver with bgpio functions it
exports but for now I haven't implemented it, not even shared the idea.
Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be obtained?
Thanks,
Janusz
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Rob Herring @ 2018-05-18 22:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <869aad59-1cc5-28ef-1fb5-4ef846696c40@codeaurora.org>
On Thu, May 17, 2018 at 05:16:13PM -0700, David Collins wrote:
> On 05/17/2018 02:22 PM, Doug Anderson wrote:
> > On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> wrote:
> >> +- qcom,regulator-initial-microvolt
> >> + Usage: optional; VRM regulators only
> >> + Value type: <u32>
> >> + Definition: Specifies the initial voltage in microvolts to request for a
> >> + VRM regulator.
> >
> > Now that Mark has landed the patch adding support for the
> > -ENOTRECOVERABLE error code from get_voltage() / get_voltage_sel(), do
> > we still need the qcom,regulator-initial-microvolt property?
>
> Yes, this is still needed. The -ENOTRECOVERABLE patch ensures that
> qcom-rpmh-regulator devices can be registered even if
> qcom,regulator-initial-microvolt is not specified. However, that will
> result in the regulators being configured for the minimum voltage
> supported in the DT specified min/max range. The
> qcom,regulator-initial-microvolt property allows us to set a specific
> voltage that is larger than the min constraint.
>
> > If this is really still needed, can it be moved to the regulator core?
>
> I'm not opposed to the idea, but I think that Mark is [1]:
>
> >> Do you have a preference for qcom,regulator-initial-microvolt vs a generic
> >> framework supported regulator-initial-microvolt property for configuring a
> >> specific voltage at registration time? We'll need to have support for one
> >> or the other in order for the qcom_rpmh-regulator driver to be functional.
> >
> > This is basically specific to Qualcomm, I can't off hand think of any
> > other devices with similar issues.
>
>
> >> +- regulator-initial-mode
> >> + Usage: optional; VRM regulators only
> >> + Value type: <u32>
> >> + Definition: Specifies the initial mode to request for a VRM regulator.
> >> + Supported values are RPMH_REGULATOR_MODE_* which are defined
> >> + in [1] (i.e. 0 to 3). This property may be specified even
> >> + if the regulator-allow-set-load property is not specified.
> >
> > Every time I read the above I wonder why you're documenting a standard
> > regulator regulator property in your bindings. ...then I realize it's
> > because you're doing it because you want to explicitly document what
> > the valid modes are. I wonder if it makes sense to just put a
> > reference somewhere else in this document to go look at the header
> > file where these are all nicely documented.
>
> Isn't that what the [1] in the above snippet is currently doing. Further
> down in qcom,rpmh-regulator.txt is this line:
>
> +[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
>
>
> > Speaking of documenting things like that, it might be worth finding
> > somewhere in this doc to mention that the "bob" regulator on PMI8998
> > can support "regulator-allow-bypass". That tidbit got lost when we
> > moved to the standard regulator bindings for bypass.
>
> I suppose that I could add something like this:
>
> +- regulator-allow-bypass
> + Usage: optional; BOB type VRM regulators only
> + Value type: <empty>
> + Definition: See [2] for details.
> ...
> +[2]: Documentation/devicetree/bindings/regulator.txt
>
> However, I don't want the patch to get NACKed because it is defining a
> property that is already defined in the common regulator.txt file.
If all constraints are defined in the common doc, just "see
regulator.txt" is fine. You just need to say what properties this
binding uses.
Rob
^ permalink raw reply
* [PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart
From: Rob Herring @ 2018-05-18 22:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511103822.31698-3-radu.pirea@microchip.com>
On Fri, May 11, 2018 at 01:38:18PM +0300, Radu Pirea wrote:
> This mfd driver is just a wrapper over atmel_serial driver and
> spi-at91-usart driver. Selection of one of the drivers is based on a
> property from device tree. If the property is not specified, the default
> driver is atmel_serial.
>
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
> drivers/mfd/Kconfig | 10 ++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/at91-usart.c | 75 ++++++++++++++++++++++++++++
> include/dt-bindings/mfd/at91-usart.h | 17 +++++++
> 4 files changed, 103 insertions(+)
> create mode 100644 drivers/mfd/at91-usart.c
> create mode 100644 include/dt-bindings/mfd/at91-usart.h
>
> +#ifndef __DT_BINDINGS_AT91_USART_H__
> +#define __DT_BINDINGS_AT91_USART_H__
> +
> +#define AT91_USART_MODE_SERIAL 1
> +#define AT91_USART_MODE_SPI 2
Won't this require a DT update for serial mode to add the mode property?
That breaks compatibility.
Rob
^ permalink raw reply
* [PATCH 2/2] pwm: stm32: initialize raw local variables
From: Thierry Reding @ 2018-05-18 22:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526657044-14879-3-git-send-email-fabrice.gasnier@st.com>
On Fri, May 18, 2018 at 05:24:04PM +0200, Fabrice Gasnier wrote:
> This removes build warning when COMPILE_TEST=y and MFD_STM32_TIMERS=n
> in drivers/pwm/pwm-stm32.c. In function 'stm32_pwm_capture' 'raw_prd' and
> 'raw_dty' may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
> drivers/pwm/pwm-stm32.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Hi Lee,
I assume you'll pick this up into your branch where you applied the
initial patches along with 1/2 in this series?
Acked-by: Thierry Reding <thierry.reding@gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180519/a4364ea1/attachment-0001.sig>
^ permalink raw reply
* [PATCH 3/3] mmc: sdhci-iproc: add SDHCI_QUIRK2_HOST_OFF_CARD_ON for cygnus
From: Scott Branden @ 2018-05-18 22:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526681037-2074-1-git-send-email-scott.branden@broadcom.com>
From: Corneliu Doban <corneliu.doban@broadcom.com>
The SDHCI_QUIRK2_HOST_OFF_CARD_ON is needed for the driver to
properly reset the host controller (reset all) on initialization
after exiting deep sleep.
Signed-off-by: Corneliu Doban <corneliu.doban@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
---
drivers/mmc/host/sdhci-iproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 1f0ab08..d0e83db 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -186,7 +186,7 @@ static const struct sdhci_ops sdhci_iproc_32only_ops = {
static const struct sdhci_pltfm_data sdhci_iproc_cygnus_pltfm_data = {
.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
- .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN,
+ .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN | SDHCI_QUIRK2_HOST_OFF_CARD_ON,
.ops = &sdhci_iproc_32only_ops,
};
--
2.5.0
^ permalink raw reply related
* [PATCH 2/3] mmc: sdhci-iproc: fix 32bit writes for TRANSFER_MODE register
From: Scott Branden @ 2018-05-18 22:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526681037-2074-1-git-send-email-scott.branden@broadcom.com>
From: Corneliu Doban <corneliu.doban@broadcom.com>
When the host controller accepts only 32bit writes, the value of the
16bit TRANSFER_MODE register, that has the same 32bit address as the
16bit COMMAND register, needs to be saved and it will be written
in a 32bit write together with the command as this will trigger the
host to send the command on the SD interface.
When sending the tuning command, TRANSFER_MODE is written and then
sdhci_set_transfer_mode reads it back to clear AUTO_CMD12 bit and
write it again resulting in wrong value to be written because the
initial write value was saved in a shadow and the read-back returned
a wrong value, from the register.
Fix sdhci_iproc_readw to return the saved value of TRANSFER_MODE
when a saved value exist.
Same fix for read of BLOCK_SIZE and BLOCK_COUNT registers, that are
saved for a different reason, although a scenario that will cause the
mentioned problem on this registers is not probable.
Fixes: b580c52d58d9 ("mmc: sdhci-iproc: add IPROC SDHCI driver")
Signed-off-by: Corneliu Doban <corneliu.doban@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
drivers/mmc/host/sdhci-iproc.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 6f430da..1f0ab08 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -33,6 +33,8 @@ struct sdhci_iproc_host {
const struct sdhci_iproc_data *data;
u32 shadow_cmd;
u32 shadow_blk;
+ bool is_cmd_shadowed;
+ bool is_blk_shadowed;
};
#define REG_OFFSET_IN_BITS(reg) ((reg) << 3 & 0x18)
@@ -48,8 +50,22 @@ static inline u32 sdhci_iproc_readl(struct sdhci_host *host, int reg)
static u16 sdhci_iproc_readw(struct sdhci_host *host, int reg)
{
- u32 val = sdhci_iproc_readl(host, (reg & ~3));
- u16 word = val >> REG_OFFSET_IN_BITS(reg) & 0xffff;
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_iproc_host *iproc_host = sdhci_pltfm_priv(pltfm_host);
+ u32 val;
+ u16 word;
+
+ if ((reg == SDHCI_TRANSFER_MODE) && iproc_host->is_cmd_shadowed) {
+ /* Get the saved transfer mode */
+ val = iproc_host->shadow_cmd;
+ } else if ((reg == SDHCI_BLOCK_SIZE || reg == SDHCI_BLOCK_COUNT) &&
+ iproc_host->is_blk_shadowed) {
+ /* Get the saved block info */
+ val = iproc_host->shadow_blk;
+ } else {
+ val = sdhci_iproc_readl(host, (reg & ~3));
+ }
+ word = val >> REG_OFFSET_IN_BITS(reg) & 0xffff;
return word;
}
@@ -105,13 +121,15 @@ static void sdhci_iproc_writew(struct sdhci_host *host, u16 val, int reg)
if (reg == SDHCI_COMMAND) {
/* Write the block now as we are issuing a command */
- if (iproc_host->shadow_blk != 0) {
+ if (iproc_host->is_blk_shadowed) {
sdhci_iproc_writel(host, iproc_host->shadow_blk,
SDHCI_BLOCK_SIZE);
- iproc_host->shadow_blk = 0;
+ iproc_host->is_blk_shadowed = false;
}
oldval = iproc_host->shadow_cmd;
- } else if (reg == SDHCI_BLOCK_SIZE || reg == SDHCI_BLOCK_COUNT) {
+ iproc_host->is_cmd_shadowed = false;
+ } else if ((reg == SDHCI_BLOCK_SIZE || reg == SDHCI_BLOCK_COUNT) &&
+ iproc_host->is_blk_shadowed) {
/* Block size and count are stored in shadow reg */
oldval = iproc_host->shadow_blk;
} else {
@@ -123,9 +141,11 @@ static void sdhci_iproc_writew(struct sdhci_host *host, u16 val, int reg)
if (reg == SDHCI_TRANSFER_MODE) {
/* Save the transfer mode until the command is issued */
iproc_host->shadow_cmd = newval;
+ iproc_host->is_cmd_shadowed = true;
} else if (reg == SDHCI_BLOCK_SIZE || reg == SDHCI_BLOCK_COUNT) {
/* Save the block info until the command is issued */
iproc_host->shadow_blk = newval;
+ iproc_host->is_blk_shadowed = true;
} else {
/* Command or other regular 32-bit write */
sdhci_iproc_writel(host, newval, reg & ~3);
--
2.5.0
^ permalink raw reply related
* [PATCH 1/3] mmc: sdhci-iproc: remove hard coded mmc cap 1.8v
From: Scott Branden @ 2018-05-18 22:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526681037-2074-1-git-send-email-scott.branden@broadcom.com>
From: Srinath Mannam <srinath.mannam@broadcom.com>
Remove hard coded mmc cap 1.8v from platform data as it is board specific.
The 1.8v DDR mmc caps can be enabled using DTS property for those
boards that support it.
Fixes: b17b4ab8ce38 ("mmc: sdhci-iproc: define MMC caps in platform data")
Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
---
drivers/mmc/host/sdhci-iproc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 0ef741b..6f430da 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -206,7 +206,6 @@ static const struct sdhci_iproc_data iproc_data = {
.caps1 = SDHCI_DRIVER_TYPE_C |
SDHCI_DRIVER_TYPE_D |
SDHCI_SUPPORT_DDR50,
- .mmc_caps = MMC_CAP_1_8V_DDR,
};
static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = {
--
2.5.0
^ permalink raw reply related
* [PATCH 0/3] mmc: sdhci-iproc: UHS and 32bit access fixes
From: Scott Branden @ 2018-05-18 22:03 UTC (permalink / raw)
To: linux-arm-kernel
Collection of bug fixes for sdhci-iproc driver.
- fix for 32bit writes for TRANSFER_MODE register by correcting shadow
register logic
- fix for deep sleep mode by adding SDHCI_QUIRK2_HOST_OFF_CARD_ON
- remove hard coded mmc capability of 1.8V to allow boards to be supported
that do support 1.8V.
Corneliu Doban (2):
mmc: sdhci-iproc: fix 32bit writes for TRANSFER_MODE register
mmc: sdhci-iproc: add SDHCI_QUIRK2_HOST_OFF_CARD_ON for cygnus
Srinath Mannam (1):
mmc: sdhci-iproc: remove hard coded mmc cap 1.8v
drivers/mmc/host/sdhci-iproc.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
--
2.5.0
^ permalink raw reply
* [GIT PULL v2 2/5] memory: tegra: Changes for v4.18-rc1
From: Thierry Reding @ 2018-05-18 21:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518142245.20242-2-thierry.reding@gmail.com>
Hi ARM SoC maintainers,
The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:
Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/tegra-for-4.18-memory-v2
for you to fetch changes up to a1be3cfdfb81cc55c1b2feb73aca6945f61acddb:
dt-bindings: memory: tegra: Remove Tegra114 SATA and AFI reset definitions (2018-05-18 22:45:01 +0200)
This contains the same patches as the previous pull request with the exception
that the final two are reordered to keep the set bisectible.
Thanks,
Thierry
----------------------------------------------------------------
memory: tegra: Changes for v4.18-rc1
This contains some cleanup of the memory controller driver as well as
unification work to share more code between Tegra20 and later SoC
generations. Also included are an implementation for the hot resets
functionality by the memory controller which is required to properly
reset busy hardware.
----------------------------------------------------------------
Dmitry Osipenko (14):
dt-bindings: memory: tegra: Add hot resets definitions
memory: tegra: Do not handle spurious interrupts
memory: tegra: Setup interrupts mask before requesting IRQ
memory: tegra: Apply interrupts mask per SoC
memory: tegra: Remove unused headers inclusions
memory: tegra: Squash tegra20-mc into common tegra-mc driver
memory: tegra: Introduce memory client hot reset
memory: tegra: Add Tegra20 memory controller hot resets
memory: tegra: Add Tegra30 memory controller hot resets
memory: tegra: Add Tegra114 memory controller hot resets
memory: tegra: Add Tegra124 memory controller hot resets
memory: tegra: Register SMMU after MC driver became ready
memory: tegra: Remove Tegra114 SATA and AFI reset definitions
dt-bindings: memory: tegra: Remove Tegra114 SATA and AFI reset definitions
Thierry Reding (1):
memory: tegra: Add Tegra210 memory controller hot resets
drivers/memory/Kconfig | 10 -
drivers/memory/Makefile | 1 -
drivers/memory/tegra/Makefile | 1 +
drivers/memory/tegra/mc.c | 362 +++++++++++++++++++++++++++----
drivers/memory/tegra/mc.h | 22 ++
drivers/memory/tegra/tegra114.c | 33 +++
drivers/memory/tegra/tegra124.c | 48 ++++
drivers/memory/tegra/tegra20.c | 296 +++++++++++++++++++++++++
drivers/memory/tegra/tegra210.c | 53 ++++-
drivers/memory/tegra/tegra30.c | 35 +++
drivers/memory/tegra20-mc.c | 254 ----------------------
include/dt-bindings/memory/tegra114-mc.h | 17 ++
include/dt-bindings/memory/tegra124-mc.h | 25 +++
include/dt-bindings/memory/tegra20-mc.h | 21 ++
include/dt-bindings/memory/tegra210-mc.h | 31 +++
include/dt-bindings/memory/tegra30-mc.h | 19 ++
include/soc/tegra/mc.h | 37 +++-
17 files changed, 956 insertions(+), 309 deletions(-)
create mode 100644 drivers/memory/tegra/tegra20.c
delete mode 100644 drivers/memory/tegra20-mc.c
create mode 100644 include/dt-bindings/memory/tegra20-mc.h
^ permalink raw reply
* dma_sync_*_for_cpu and direction=TO_DEVICE (was Re: [PATCH 02/20] dma-mapping: provide a generic dma-noncoherent implementation)
From: Russell King - ARM Linux @ 2018-05-18 21:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cecfe6bd-ef1f-1e25-bfcf-992d1f828efb@synopsys.com>
On Fri, May 18, 2018 at 01:35:08PM -0700, Vineet Gupta wrote:
> On 05/18/2018 10:50 AM, Russell King - ARM Linux wrote:
> >On Fri, May 18, 2018 at 10:20:02AM -0700, Vineet Gupta wrote:
> >>I never understood the need for this direction. And if memory serves me
> >>right, at that time I was seeing twice the amount of cache flushing !
> >It's necessary. Take a moment to think carefully about this:
> >
> > dma_map_single(, dir)
> >
> > dma_sync_single_for_cpu(, dir)
> >
> > dma_sync_single_for_device(, dir)
> >
> > dma_unmap_single(, dir)
>
> As an aside, do these imply a state machine of sorts - does a driver needs
> to always call map_single first ?
Kind-of, but some drivers do omit some of the dma_sync_*() calls.
For example, if a buffer is written to, then mapped with TO_DEVICE,
and then the CPU wishes to write to it, it's fairly common that a
driver omits the dma_sync_single_for_cpu() call. If you think about
the cases I gave and what cache operations happen, such a scenario
practically turns out to be safe.
> My original point of contention/confusion is the specific combinations of
> API and direction, specifically for_cpu(TO_DEV) and for_device(TO_CPU)
Remember that it is expected that all calls for a mapping use the
same direction argument while that mapping exists. In other words,
if you call dma_map_single(TO_DEVICE) and then use any of the other
functions, the other functions will also use TO_DEVICE. The DMA
direction argument describes the direction of the DMA operation
being performed on the buffer, not on the individual dma_* operation.
What isn't expected at arch level is for drivers to do:
dma_map_single(TO_DEVICE)
dma_sync_single_for_cpu(FROM_DEVICE)
or vice versa.
> Semantically what does dma_sync_single_for_cpu(TO_DEV) even imply for a non
> dma coherent arch.
>
> Your tables below have "none" for both, implying it is unlikely to be a real
> combination (for ARM and ARC atleast).
Very little for the cases that I've stated (and as I mentioned
above, some drivers do omit the call in that case.)
> The other case, actually @dir TO_CPU, independent of for_{cpu, device}?
> implies driver intends to touch it after the call, so it would invalidate
> any stray lines, unconditionally (and not just for speculative prefetch
> case).
If you don't have a CPU that speculatively prefetches, and you've
already had to invalidate the cache lines (to avoid write-backs
corrupting DMA'd data) then there's no need for the architecture
to do any work at the for_cpu(TO_CPU) case - the CPU shouldn't
be touching cache lines that are part of the buffer while it is
mapped, which means a non-speculating CPU won't pull in any
cache lines without an explicit access.
Speculating CPUs are different. The action of the speculation is
to try and guess what data the program wants to access ahead of
the program flow. That causes the CPU to prefetch data into the
cache. The point in the program flow that this happens is not
really determinant to the programmer. This means that if you try
to read from the DMA buffer after the DMA operation has complete
without invalidating the cache between the DMA completing and the
CPU reading, you have no guarantee that you're reading the data
that the DMA operation has been written. The cache may have
loaded itself with data before the DMA operation completed, and
the CPU may see that stale data.
The difference between non-speculating CPUs and speculating CPUs
is that for non-speculating CPUs, caches work according to explicit
accesses by the program, and the program is stalled while the data
is fetched from external memory. Speculating CPUs try to predict
ahead of time what data the program will require in the future,
and attempt to load that data into the caches _before_ the program
requires it - which means that the program suffers fewer stalls.
> >In the case of a DMA-incoherent architecture, the operations done at each
> >stage depend on the direction argument:
> >
> > map for_cpu for_device unmap
> >TO_DEV writeback none writeback none
> >TO_CPU invalidate invalidate* invalidate invalidate*
> >BIDIR writeback invalidate writeback invalidate
> >
> >* - only necessary if the CPU speculatively prefetches.
> >
> >The multiple invalidations for the TO_CPU case handles different
> >conditions that can result in data corruption, and for some CPUs, all
> >four are necessary.
>
> Can you please explain in some more detail, TO_CPU row, why invalidate is
> conditional sometimes.
See above - I hope my explanation above is sufficient.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH v9 02/12] drivers: base: cacheinfo: setup DT cache properties early
From: Andy Shevchenko @ 2018-05-18 21:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517154701.GA20281@e107155-lin>
On Thu, May 17, 2018 at 6:47 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Is below patch does what you were looking for ?
Somewhat.
See below for some minors.
> of_property_read_u64 searches for a property in a device node and read
> a 64-bit value from it. Instead of using of_get_property to get the
> property and then read 64-bit value using of_read_number, we can make
> use of of_property_read_u64.
Suggested-by?
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> - cache_size = of_get_property(np, propname, NULL);
> - if (cache_size)
> - this_leaf->size = of_read_number(cache_size, 1);
> + if (!of_property_read_u64(np, propname, &cache_size))
> + this_leaf->size = cache_size;
I suppose it's something like this
ret = of_property_...(..., &this_leaf->VAR);
if (ret)
warning / set default / etc
> propname = cache_type_info[ct_idx].line_size_props[i];
> - line_size = of_get_property(np, propname, NULL);
> - if (line_size)
> + line_size = of_property_read_u64(np, propname, &line_size);
> + if (line_size) {
ret = ...
if (ret) {
> + this_leaf->coherency_line_size = line_size;
> break;
> + }
> + if (!of_property_read_u64(np, propname, &nr_sets))
> + this_leaf->number_of_sets = nr_sets;
As in first case.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH 4/5] pinctrl: actions: Add gpio support for Actions S900 SoC
From: Andy Shevchenko @ 2018-05-18 21:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518023056.7869-5-manivannan.sadhasivam@linaro.org>
On Fri, May 18, 2018 at 5:30 AM, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
> Add gpio support to pinctrl driver for Actions Semi S900 SoC.
>
LGTM,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> drivers/pinctrl/actions/Kconfig | 1 +
> drivers/pinctrl/actions/pinctrl-owl.c | 206 +++++++++++++++++++++++++++++++++
> drivers/pinctrl/actions/pinctrl-owl.h | 20 ++++
> drivers/pinctrl/actions/pinctrl-s900.c | 29 ++++-
> 4 files changed, 255 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/actions/Kconfig b/drivers/pinctrl/actions/Kconfig
> index ede97cdbbc12..490927b4ea76 100644
> --- a/drivers/pinctrl/actions/Kconfig
> +++ b/drivers/pinctrl/actions/Kconfig
> @@ -4,6 +4,7 @@ config PINCTRL_OWL
> select PINMUX
> select PINCONF
> select GENERIC_PINCONF
> + select GPIOLIB
> help
> Say Y here to enable Actions Semi OWL pinctrl driver
>
> diff --git a/drivers/pinctrl/actions/pinctrl-owl.c b/drivers/pinctrl/actions/pinctrl-owl.c
> index ee090697b1e9..4942e34c8b76 100644
> --- a/drivers/pinctrl/actions/pinctrl-owl.c
> +++ b/drivers/pinctrl/actions/pinctrl-owl.c
> @@ -11,6 +11,7 @@
>
> #include <linux/clk.h>
> #include <linux/err.h>
> +#include <linux/gpio/driver.h>
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> @@ -31,6 +32,7 @@
> * struct owl_pinctrl - pinctrl state of the device
> * @dev: device handle
> * @pctrldev: pinctrl handle
> + * @chip: gpio chip
> * @lock: spinlock to protect registers
> * @soc: reference to soc_data
> * @base: pinctrl register base address
> @@ -38,6 +40,7 @@
> struct owl_pinctrl {
> struct device *dev;
> struct pinctrl_dev *pctrldev;
> + struct gpio_chip chip;
> raw_spinlock_t lock;
> struct clk *clk;
> const struct owl_pinctrl_soc_data *soc;
> @@ -536,6 +539,198 @@ static struct pinctrl_desc owl_pinctrl_desc = {
> .owner = THIS_MODULE,
> };
>
> +static const struct owl_gpio_port *
> +owl_gpio_get_port(struct owl_pinctrl *pctrl, unsigned int *pin)
> +{
> + unsigned int start = 0, i;
> +
> + for (i = 0; i < pctrl->soc->nports; i++) {
> + const struct owl_gpio_port *port = &pctrl->soc->ports[i];
> +
> + if (*pin >= start && *pin < start + port->pins) {
> + *pin -= start;
> + return port;
> + }
> +
> + start += port->pins;
> + }
> +
> + return NULL;
> +}
> +
> +static void owl_gpio_update_reg(void __iomem *base, unsigned int pin, int flag)
> +{
> + u32 val;
> +
> + val = readl_relaxed(base);
> +
> + if (flag)
> + val |= BIT(pin);
> + else
> + val &= ~BIT(pin);
> +
> + writel_relaxed(val, base);
> +}
> +
> +static int owl_gpio_request(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return -ENODEV;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + /*
> + * GPIOs have higher priority over other modules, so either setting
> + * them as OUT or IN is sufficient
> + */
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + owl_gpio_update_reg(gpio_base + port->outen, offset, true);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return 0;
> +}
> +
> +static void owl_gpio_free(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + /* disable gpio output */
> + owl_gpio_update_reg(gpio_base + port->outen, offset, false);
> +
> + /* disable gpio input */
> + owl_gpio_update_reg(gpio_base + port->inen, offset, false);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +}
> +
> +static int owl_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> + u32 val;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return -ENODEV;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + val = readl_relaxed(gpio_base + port->dat);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return !!(val & BIT(offset));
> +}
> +
> +static void owl_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + owl_gpio_update_reg(gpio_base + port->dat, offset, value);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +}
> +
> +static int owl_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return -ENODEV;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + owl_gpio_update_reg(gpio_base + port->outen, offset, false);
> + owl_gpio_update_reg(gpio_base + port->inen, offset, true);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return 0;
> +}
> +
> +static int owl_gpio_direction_output(struct gpio_chip *chip,
> + unsigned int offset, int value)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return -ENODEV;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + owl_gpio_update_reg(gpio_base + port->inen, offset, false);
> + owl_gpio_update_reg(gpio_base + port->outen, offset, true);
> + owl_gpio_update_reg(gpio_base + port->dat, offset, value);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return 0;
> +}
> +
> +static int owl_gpio_init(struct owl_pinctrl *pctrl)
> +{
> + struct gpio_chip *chip;
> + int ret;
> +
> + chip = &pctrl->chip;
> + chip->base = -1;
> + chip->ngpio = pctrl->soc->ngpios;
> + chip->label = dev_name(pctrl->dev);
> + chip->parent = pctrl->dev;
> + chip->owner = THIS_MODULE;
> + chip->of_node = pctrl->dev->of_node;
> +
> + ret = gpiochip_add_data(&pctrl->chip, pctrl);
> + if (ret) {
> + dev_err(pctrl->dev, "failed to register gpiochip\n");
> + return ret;
> + }
> +
> + ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
> + 0, 0, chip->ngpio);
> + if (ret) {
> + dev_err(pctrl->dev, "failed to add pin range\n");
> + gpiochip_remove(&pctrl->chip);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int owl_pinctrl_probe(struct platform_device *pdev,
> struct owl_pinctrl_soc_data *soc_data)
> {
> @@ -571,6 +766,13 @@ int owl_pinctrl_probe(struct platform_device *pdev,
> owl_pinctrl_desc.pins = soc_data->pins;
> owl_pinctrl_desc.npins = soc_data->npins;
>
> + pctrl->chip.direction_input = owl_gpio_direction_input;
> + pctrl->chip.direction_output = owl_gpio_direction_output;
> + pctrl->chip.get = owl_gpio_get;
> + pctrl->chip.set = owl_gpio_set;
> + pctrl->chip.request = owl_gpio_request;
> + pctrl->chip.free = owl_gpio_free;
> +
> pctrl->soc = soc_data;
> pctrl->dev = &pdev->dev;
>
> @@ -581,6 +783,10 @@ int owl_pinctrl_probe(struct platform_device *pdev,
> return PTR_ERR(pctrl->pctrldev);
> }
>
> + ret = owl_gpio_init(pctrl);
> + if (ret)
> + return ret;
> +
> platform_set_drvdata(pdev, pctrl);
>
> return 0;
> diff --git a/drivers/pinctrl/actions/pinctrl-owl.h b/drivers/pinctrl/actions/pinctrl-owl.h
> index 448f81a6db3b..74342378937c 100644
> --- a/drivers/pinctrl/actions/pinctrl-owl.h
> +++ b/drivers/pinctrl/actions/pinctrl-owl.h
> @@ -114,6 +114,22 @@ struct owl_pinmux_func {
> unsigned int ngroups;
> };
>
> +/**
> + * struct owl_gpio_port - Actions GPIO port info
> + * @offset: offset of the GPIO port.
> + * @pins: number of pins belongs to the GPIO port.
> + * @outen: offset of the output enable register.
> + * @inen: offset of the input enable register.
> + * @dat: offset of the data register.
> + */
> +struct owl_gpio_port {
> + unsigned int offset;
> + unsigned int pins;
> + unsigned int outen;
> + unsigned int inen;
> + unsigned int dat;
> +};
> +
> /**
> * struct owl_pinctrl_soc_data - Actions pin controller driver configuration
> * @pins: array describing all pins of the pin controller.
> @@ -124,6 +140,8 @@ struct owl_pinmux_func {
> * @ngroups: number of entries in @groups.
> * @padinfo: array describing the pad info of this SoC.
> * @ngpios: number of pingroups the driver should expose as GPIOs.
> + * @port: array describing all GPIO ports of this SoC.
> + * @nports: number of GPIO ports in this SoC.
> */
> struct owl_pinctrl_soc_data {
> const struct pinctrl_pin_desc *pins;
> @@ -134,6 +152,8 @@ struct owl_pinctrl_soc_data {
> unsigned int ngroups;
> const struct owl_padinfo *padinfo;
> unsigned int ngpios;
> + const struct owl_gpio_port *ports;
> + unsigned int nports;
> };
>
> int owl_pinctrl_probe(struct platform_device *pdev,
> diff --git a/drivers/pinctrl/actions/pinctrl-s900.c b/drivers/pinctrl/actions/pinctrl-s900.c
> index 08d93f8fc086..5503c7945764 100644
> --- a/drivers/pinctrl/actions/pinctrl-s900.c
> +++ b/drivers/pinctrl/actions/pinctrl-s900.c
> @@ -33,6 +33,13 @@
> #define PAD_SR1 (0x0274)
> #define PAD_SR2 (0x0278)
>
> +#define OWL_GPIO_PORT_A 0
> +#define OWL_GPIO_PORT_B 1
> +#define OWL_GPIO_PORT_C 2
> +#define OWL_GPIO_PORT_D 3
> +#define OWL_GPIO_PORT_E 4
> +#define OWL_GPIO_PORT_F 5
> +
> #define _GPIOA(offset) (offset)
> #define _GPIOB(offset) (32 + (offset))
> #define _GPIOC(offset) (64 + (offset))
> @@ -1814,6 +1821,24 @@ static struct owl_padinfo s900_padinfo[NUM_PADS] = {
> [SGPIO3] = PAD_INFO_PULLCTL_ST(SGPIO3)
> };
>
> +#define OWL_GPIO_PORT(port, base, count, _outen, _inen, _dat) \
> + [OWL_GPIO_PORT_##port] = { \
> + .offset = base, \
> + .pins = count, \
> + .outen = _outen, \
> + .inen = _inen, \
> + .dat = _dat, \
> + }
> +
> +static const struct owl_gpio_port s900_gpio_ports[] = {
> + OWL_GPIO_PORT(A, 0x0000, 32, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(B, 0x000C, 32, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(C, 0x0018, 12, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(D, 0x0024, 30, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(E, 0x0030, 32, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8)
> +};
> +
> static struct owl_pinctrl_soc_data s900_pinctrl_data = {
> .padinfo = s900_padinfo,
> .pins = (const struct pinctrl_pin_desc *)s900_pads,
> @@ -1822,7 +1847,9 @@ static struct owl_pinctrl_soc_data s900_pinctrl_data = {
> .nfunctions = ARRAY_SIZE(s900_functions),
> .groups = s900_groups,
> .ngroups = ARRAY_SIZE(s900_groups),
> - .ngpios = NUM_GPIOS
> + .ngpios = NUM_GPIOS,
> + .ports = s900_gpio_ports,
> + .nports = ARRAY_SIZE(s900_gpio_ports)
> };
>
> static int s900_pinctrl_probe(struct platform_device *pdev)
> --
> 2.14.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v3 4/6] dt-bindings: add binding for at91-usart in spi mode
From: Rob Herring @ 2018-05-18 21:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511103822.31698-5-radu.pirea@microchip.com>
On Fri, May 11, 2018 at 01:38:20PM +0300, Radu Pirea wrote:
> These are bindings for at91-usart IP in spi spi mode. There is no support for
s/spi spi/SPI/
> internal chip select. Only kind of chip selects available are gpio chip
GPIO
> selects.
>
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
> .../bindings/spi/microchip,at91-usart-spi.txt | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
>
> diff --git a/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> new file mode 100644
> index 000000000000..b68a3bec4121
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
So now we have 2 copies of the same thing that varies by 2 properties?
Please make this one doc.
> @@ -0,0 +1,28 @@
> +* Universal Synchronous Asynchronous Receiver/Transmitter (USART) in SPI mode
> +
> +Required properties:
> +- #size-cells : Must be <0>
> +- #address-cells : Must be <1>
> +- compatible: Should be "atmel,at91rm9200-usart" or "atmel,at91sam9260-usart"
> +- reg: Should contain registers location and length
> +- interrupts: Should contain interrupt
> +- clocks: phandles to input clocks.
> +- clock-names: tuple listing input clock names.
> + Required elements: "usart"
> +- cs-gpios: chipselects (internal cs not supported)
> +- at91,usart-mode: AT91_USART_MODE_SPI (found in dt-bindings/mfd/at91-usart.h)
at91 is not a vendor.
> +
> +Example:
> + #include <dt-bindings/mfd/at91-usart.h>
> +
> + spi0: spi at f001c000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
> + at91,usart-mode = <AT91_USART_MODE_SPI>;
> + reg = <0xf001c000 0x100>;
> + interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&usart0_clk>;
> + clock-names = "usart";
> + cs-gpios = <&pioB 3 0>;
> + };
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH 16/16] iommu/arm-smmu: Add list of devices to opt out of DMA domains
From: Jordan Crouse @ 2018-05-18 21:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
Add a list of compatible strings for devices that wish to opt out
of attaching to a DMA domain. This is for devices that prefer to
manage their own IOMMU space for any number of reasons. Returning
-ENOTSUPP for attach device will filter down and force
arch_setup_dma_ops() to not set up the iommu DMA ops. Later
the client device in question can set up and attach their own
domain.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 100797a07be0..df6e4eacf727 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1080,6 +1080,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
goto out_unlock;
cfg->cbndx = ret;
+
if (smmu->version < ARM_SMMU_V2) {
cfg->irptndx = atomic_inc_return(&smmu->irptndx);
cfg->irptndx %= smmu->num_context_irqs;
@@ -1450,6 +1451,15 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
return 0;
}
+/*
+ * This is a list of compatible strings for devices that wish to manage their
+ * own IOMMU space instead of the DMA IOMMU ops. Devices on this list will not
+ * allow themselves to be attached to a IOMMU_DOMAIN_DMA domain
+ */
+static const char *arm_smmu_dma_blacklist[] = {
+ "qcom,adreno",
+};
+
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int ret;
@@ -1472,6 +1482,20 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
if (!fwspec->iommu_priv)
return -ENODEV;
+ /*
+ * If this is the dfeault DMA domain, check to see if the device is on
+ * the blacklist and reject if so
+ */
+ if (domain->type == IOMMU_DOMAIN_DMA && dev->of_node) {
+ int i;
+
+ for(i = 0; i < ARRAY_SIZE(arm_smmu_dma_blacklist); i++) {
+ if (of_device_is_compatible(dev->of_node,
+ arm_smmu_dma_blacklist[i]))
+ return -ENOTSUPP;
+ }
+ }
+
smmu = fwspec_smmu(fwspec);
/* Ensure that the domain is finalised */
ret = arm_smmu_init_domain_context(domain, smmu);
--
2.17.0
^ permalink raw reply related
* [PATCH 15/16] iommu: Gracefully allow drivers to not attach to a default domain
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
Provide individual device drivers the chance to gracefully refuse
to attach a device to the default domain. If the attach_device
op returns -ENOTSUPP don't print a error message and don't set
group->domain but still return success from iommu_group_add_dev().
This allows all the usual APIs to work and the next domain to try
to attach will take group->domain for itself and everything will
proceed as normal.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/iommu/iommu.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0ba3d27f2300..a255b5d6c495 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -599,7 +599,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
if (group->domain)
ret = __iommu_attach_device(group->domain, dev);
mutex_unlock(&group->mutex);
- if (ret)
+ if (ret && ret != -ENOTSUPP)
goto err_put_group;
/* Notify any listeners about change to group. */
@@ -625,7 +625,8 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
sysfs_remove_link(&dev->kobj, "iommu_group");
err_free_device:
kfree(device);
- pr_err("Failed to add device %s to group %d: %d\n", dev_name(dev), group->id, ret);
+ if (ret != -ENOTSUPP)
+ pr_err("Failed to add device %s to group %d: %d\n", dev_name(dev), group->id, ret);
return ret;
}
EXPORT_SYMBOL_GPL(iommu_group_add_device);
@@ -1238,8 +1239,16 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
ret = iommu_group_add_device(group, dev);
if (ret) {
- iommu_group_put(group);
- return ERR_PTR(ret);
+ /*
+ * If the driver chooses not to bind the device, reset
+ * group->domain so a new domain can be added later
+ */
+ if (ret == -ENOTSUPP)
+ group->domain = NULL;
+ else {
+ iommu_group_put(group);
+ return ERR_PTR(ret);
+ }
}
return group;
--
2.17.0
^ permalink raw reply related
* [PATCH 14/16] drm/msm: Support per-instance address spaces
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
Create a per-instance address spaces when a new DRM file instance is
opened assuming the target supports it and the underlying
infrastructure exists. If the operation is unsupported fall back
quietly to use the global pagetable.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/msm_drv.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 2b663435a3f7..31d1e7589892 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -22,6 +22,7 @@
#include "msm_fence.h"
#include "msm_gpu.h"
#include "msm_kms.h"
+#include "msm_gem.h"
/*
@@ -511,7 +512,27 @@ static int context_init(struct drm_device *dev, struct drm_file *file)
msm_submitqueue_init(dev, ctx);
- ctx->aspace = priv->gpu->aspace;
+ /* FIXME: Do we want a dynamic name of some sort? */
+ /* FIXME: We need a smarter way to set the range based on target */
+
+ ctx->aspace = msm_gem_address_space_create_instance(
+ priv->gpu->aspace->mmu, "gpu", 0x100000000, 0x1ffffffff);
+
+ if (IS_ERR(ctx->aspace)) {
+ int ret = PTR_ERR(ctx->aspace);
+
+ /*
+ * if per-instance pagetables are not supported, fall back to
+ * using the generic address space
+ */
+ if (ret == -EOPNOTSUPP)
+ ctx->aspace = priv->gpu->aspace;
+ else {
+ kfree(ctx);
+ return ret;
+ }
+ }
+
file->driver_priv = ctx;
return 0;
@@ -527,8 +548,12 @@ static int msm_open(struct drm_device *dev, struct drm_file *file)
return context_init(dev, file);
}
-static void context_close(struct msm_file_private *ctx)
+static void context_close(struct msm_drm_private *priv,
+ struct msm_file_private *ctx)
{
+ if (ctx && ctx->aspace != priv->gpu->aspace)
+ msm_gem_address_space_put(ctx->aspace);
+
msm_submitqueue_close(ctx);
kfree(ctx);
}
@@ -543,7 +568,7 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file)
priv->lastctx = NULL;
mutex_unlock(&dev->struct_mutex);
- context_close(ctx);
+ context_close(priv, ctx);
}
static irqreturn_t msm_irq(int irq, void *arg)
--
2.17.0
^ permalink raw reply related
* [PATCH 13/16] drm/msm/a5xx: Support per-instance pagetables
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
Add support for per-instance pagetables for 5XX targets. Create a support
buffer for preemption to hold the SMMU pagetable information for a preempted
ring, enable TTBR1 to support split pagetables and add the necessary PM4
commands to trigger a pagetable switch at the beginning of a user command.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 55 +++++++++++++++++
drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 17 ++++++
drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 74 +++++++++++++++++++----
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 ++++
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 ++
drivers/gpu/drm/msm/msm_ringbuffer.h | 1 +
7 files changed, 152 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 38cbde971b48..e69cbf88bb3d 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -15,6 +15,7 @@ config DRM_MSM
select SND_SOC_HDMI_CODEC if SND_SOC
select SYNC_FILE
select PM_OPP
+ select IOMMU_SVA
default y
help
DRM/KMS driver for MSM/snapdragon.
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index b2c0370072dd..f4be2536441b 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -199,6 +199,59 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit
msm_gpu_retire(gpu);
}
+static void a5xx_set_pagetable(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
+ struct msm_file_private *ctx)
+{
+ u64 ttbr;
+ u32 asid;
+
+ if (msm_iommu_pasid_info(ctx->aspace->mmu, &ttbr, &asid))
+ return;
+
+ ttbr = ttbr | ((u64) asid) << 48;
+
+ /* Turn off protected mode */
+ OUT_PKT7(ring, CP_SET_PROTECTED_MODE, 1);
+ OUT_RING(ring, 0);
+
+ /* Turn on APIV mode to access critical regions */
+ OUT_PKT4(ring, REG_A5XX_CP_CNTL, 1);
+ OUT_RING(ring, 1);
+
+ /* Make sure the ME is synchronized before staring the update */
+ OUT_PKT7(ring, CP_WAIT_FOR_ME, 0);
+
+ /* Execute the table update */
+ OUT_PKT7(ring, CP_SMMU_TABLE_UPDATE, 3);
+ OUT_RING(ring, lower_32_bits(ttbr));
+ OUT_RING(ring, upper_32_bits(ttbr));
+ OUT_RING(ring, 0);
+
+ /*
+ * Write the new TTBR0 to the preemption records - this will be used to
+ * reload the pagetable if the current ring gets preempted out.
+ */
+ OUT_PKT7(ring, CP_MEM_WRITE, 4);
+ OUT_RING(ring, lower_32_bits(rbmemptr(ring, ttbr0)));
+ OUT_RING(ring, upper_32_bits(rbmemptr(ring, ttbr0)));
+ OUT_RING(ring, lower_32_bits(ttbr));
+ OUT_RING(ring, upper_32_bits(ttbr));
+
+ /* Invalidate the draw state so we start off fresh */
+ OUT_PKT7(ring, CP_SET_DRAW_STATE, 3);
+ OUT_RING(ring, 0x40000);
+ OUT_RING(ring, 1);
+ OUT_RING(ring, 0);
+
+ /* Turn off APRIV */
+ OUT_PKT4(ring, REG_A5XX_CP_CNTL, 1);
+ OUT_RING(ring, 0);
+
+ /* Turn off protected mode */
+ OUT_PKT7(ring, CP_SET_PROTECTED_MODE, 1);
+ OUT_RING(ring, 1);
+}
+
static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
struct msm_file_private *ctx)
{
@@ -214,6 +267,8 @@ static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
return;
}
+ a5xx_set_pagetable(gpu, ring, ctx);
+
OUT_PKT7(ring, CP_PREEMPT_ENABLE_GLOBAL, 1);
OUT_RING(ring, 0x02);
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
index 7d71860c4bee..9387d6085576 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
@@ -45,6 +45,9 @@ struct a5xx_gpu {
atomic_t preempt_state;
struct timer_list preempt_timer;
+ struct a5xx_smmu_info *smmu_info;
+ struct drm_gem_object *smmu_info_bo;
+ uint64_t smmu_info_iova;
};
#define to_a5xx_gpu(x) container_of(x, struct a5xx_gpu, base)
@@ -132,6 +135,20 @@ struct a5xx_preempt_record {
*/
#define A5XX_PREEMPT_COUNTER_SIZE (16 * 4)
+/*
+ * This is a global structure that the preemption code uses to switch in the
+ * pagetable for the preempted process - the code switches in whatever we
+ * after preempting in a new ring.
+ */
+struct a5xx_smmu_info {
+ uint32_t magic;
+ uint32_t _pad4;
+ uint64_t ttbr0;
+ uint32_t asid;
+ uint32_t contextidr;
+};
+
+#define A5XX_SMMU_INFO_MAGIC 0x3618CDA3UL
int a5xx_power_init(struct msm_gpu *gpu);
void a5xx_gpmu_ucode_init(struct msm_gpu *gpu);
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
index 970c7963ae29..d5dbcbd494f3 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
@@ -12,6 +12,7 @@
*/
#include "msm_gem.h"
+#include "msm_mmu.h"
#include "a5xx_gpu.h"
/*
@@ -145,6 +146,15 @@ void a5xx_preempt_trigger(struct msm_gpu *gpu)
a5xx_gpu->preempt[ring->id]->wptr = get_wptr(ring);
spin_unlock_irqrestore(&ring->lock, flags);
+ /* Do read barrier to make sure we have updated pagetable info */
+ rmb();
+
+ /* Set the SMMU info for the preemption */
+ if (a5xx_gpu->smmu_info) {
+ a5xx_gpu->smmu_info->ttbr0 = ring->memptrs->ttbr0;
+ a5xx_gpu->smmu_info->contextidr = 0;
+ }
+
/* Set the address of the incoming preemption record */
gpu_write64(gpu, REG_A5XX_CP_CONTEXT_SWITCH_RESTORE_ADDR_LO,
REG_A5XX_CP_CONTEXT_SWITCH_RESTORE_ADDR_HI,
@@ -214,9 +224,10 @@ void a5xx_preempt_hw_init(struct msm_gpu *gpu)
a5xx_gpu->preempt[i]->rbase = gpu->rb[i]->iova;
}
- /* Write a 0 to signal that we aren't switching pagetables */
+ /* Tell the CP where to find the smmu_info buffer*/
gpu_write64(gpu, REG_A5XX_CP_CONTEXT_SWITCH_SMMU_INFO_LO,
- REG_A5XX_CP_CONTEXT_SWITCH_SMMU_INFO_HI, 0);
+ REG_A5XX_CP_CONTEXT_SWITCH_SMMU_INFO_HI,
+ a5xx_gpu->smmu_info_iova);
/* Reset the preemption state */
set_preempt_state(a5xx_gpu, PREEMPT_NONE);
@@ -275,8 +286,43 @@ void a5xx_preempt_fini(struct msm_gpu *gpu)
drm_gem_object_unreference(a5xx_gpu->preempt_bo[i]);
a5xx_gpu->preempt_bo[i] = NULL;
}
+
+ if (a5xx_gpu->smmu_info_bo) {
+ if (a5xx_gpu->smmu_info_iova)
+ msm_gem_put_iova(a5xx_gpu->smmu_info_bo, gpu->aspace);
+ drm_gem_object_unreference_unlocked(a5xx_gpu->smmu_info_bo);
+ a5xx_gpu->smmu_info_bo = NULL;
+ }
}
+static int a5xx_smmu_info_init(struct msm_gpu *gpu)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu);
+ struct a5xx_smmu_info *ptr;
+ struct drm_gem_object *bo;
+ u64 iova;
+
+ if (!msm_mmu_has_feature(gpu->aspace->mmu,
+ MMU_FEATURE_PER_INSTANCE_TABLES))
+ return 0;
+
+ ptr = msm_gem_kernel_new(gpu->dev, sizeof(struct a5xx_smmu_info),
+ MSM_BO_UNCACHED, gpu->aspace, &bo, &iova);
+
+ if (IS_ERR(ptr))
+ return PTR_ERR(ptr);
+
+ ptr->magic = A5XX_SMMU_INFO_MAGIC;
+
+ a5xx_gpu->smmu_info_bo = bo;
+ a5xx_gpu->smmu_info_iova = iova;
+ a5xx_gpu->smmu_info = ptr;
+
+ return 0;
+}
+
+
void a5xx_preempt_init(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -288,17 +334,21 @@ void a5xx_preempt_init(struct msm_gpu *gpu)
return;
for (i = 0; i < gpu->nr_rings; i++) {
- if (preempt_init_ring(a5xx_gpu, gpu->rb[i])) {
- /*
- * On any failure our adventure is over. Clean up and
- * set nr_rings to 1 to force preemption off
- */
- a5xx_preempt_fini(gpu);
- gpu->nr_rings = 1;
-
- return;
- }
+ if (preempt_init_ring(a5xx_gpu, gpu->rb[i]))
+ goto fail;
}
+ if (a5xx_smmu_info_init(gpu))
+ goto fail;
+
timer_setup(&a5xx_gpu->preempt_timer, a5xx_preempt_timer, 0);
+
+ return;
+fail:
+ /*
+ * On any failure our adventure is over. Clean up and
+ * set nr_rings to 1 to force preemption off
+ */
+ a5xx_preempt_fini(gpu);
+ gpu->nr_rings = 1;
}
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 17d0506d058c..b681edec4560 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -558,6 +558,17 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
adreno_gpu_config.ioname = "kgsl_3d0_reg_memory";
adreno_gpu_config.irqname = "kgsl_3d0_irq";
+ if (adreno_is_a5xx(adreno_gpu)) {
+ /*
+ * If possible use the TTBR1 virtual address space for all the
+ * "global" buffer objects which are shared between processes.
+ * This leaves the lower virtual address space open for
+ * per-instance pagables if they are available
+ */
+ adreno_gpu_config.va_start_global = 0xfffffff800000000ULL;
+ adreno_gpu_config.va_end_global = 0xfffffff8ffffffffULL;
+ }
+
adreno_gpu_config.va_start = SZ_16M;
adreno_gpu_config.va_end = 0xffffffff;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index d6b0e7b813f4..dc4b21ea3e65 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -203,6 +203,11 @@ static inline int adreno_is_a530(struct adreno_gpu *gpu)
return gpu->revn == 530;
}
+static inline bool adreno_is_a5xx(struct adreno_gpu *gpu)
+{
+ return ((gpu->revn >= 500) & (gpu->revn < 600));
+}
+
int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value);
const struct firmware *adreno_request_fw(struct adreno_gpu *adreno_gpu,
const char *fwname);
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h
index cffce094aecb..fd71484d5894 100644
--- a/drivers/gpu/drm/msm/msm_ringbuffer.h
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.h
@@ -26,6 +26,7 @@
struct msm_rbmemptrs {
volatile uint32_t rptr;
volatile uint32_t fence;
+ volatile uint64_t ttbr0;
};
struct msm_ringbuffer {
--
2.17.0
^ permalink raw reply related
* [PATCH 12/16] drm/msm: Add support for per-instance address spaces
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
Add a function to allocate a new pasid from a existing
MMU domain and create a per-instance address space.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/msm_drv.h | 3 +++
drivers/gpu/drm/msm/msm_gem_vma.c | 37 +++++++++++++++++++++++++------
2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 897b08135927..d92b009dfef4 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -177,6 +177,9 @@ void msm_gem_address_space_put(struct msm_gem_address_space *aspace);
struct msm_gem_address_space *
msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
const char *name);
+struct msm_gem_address_space *
+msm_gem_address_space_create_instance(struct msm_mmu *parent, const char *name,
+ u64 start, u64 end);
void msm_gem_submit_free(struct msm_gem_submit *submit);
int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index ffbec224551b..d75b56119752 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -92,12 +92,11 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
}
struct msm_gem_address_space *
-msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
- const char *name)
+msm_gem_address_space_new(struct msm_mmu *mmu, const char *name,
+ u64 start, u64 end)
{
struct msm_gem_address_space *aspace;
- u64 size = domain->geometry.aperture_end -
- domain->geometry.aperture_start;
+ u64 size = end - start;
aspace = kzalloc(sizeof(*aspace), GFP_KERNEL);
if (!aspace)
@@ -105,12 +104,36 @@ msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
spin_lock_init(&aspace->lock);
aspace->name = name;
- aspace->mmu = msm_iommu_new(dev, domain);
+ aspace->mmu = mmu;
- drm_mm_init(&aspace->mm, (domain->geometry.aperture_start >> PAGE_SHIFT),
- size >> PAGE_SHIFT);
+ drm_mm_init(&aspace->mm, (start >> PAGE_SHIFT), size >> PAGE_SHIFT);
kref_init(&aspace->kref);
return aspace;
}
+
+struct msm_gem_address_space *
+msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
+ const char *name)
+{
+ struct msm_mmu *mmu = msm_iommu_new(dev, domain);
+
+ if (IS_ERR(mmu))
+ return ERR_CAST(mmu);
+
+ return msm_gem_address_space_new(mmu, name,
+ domain->geometry.aperture_start,
+ domain->geometry.aperture_end);
+}
+
+struct msm_gem_address_space *
+msm_gem_address_space_create_instance(struct msm_mmu *parent, const char *name,
+ u64 start, u64 end)
+{
+ struct msm_mmu *instance = msm_iommu_pasid_new(parent);
+ if (IS_ERR(instance))
+ return ERR_CAST(instance);
+
+ return msm_gem_address_space_new(instance, name, start, end);
+}
--
2.17.0
^ permalink raw reply related
* [PATCH 11/16] drm/msm: Add support for iommu-sva PASIDs
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
The IOMMU core can support creating multiple pagetables
for a specific domai and making them available to a client
driver that has the means to manage the pagetable itself.
PASIDs are unique indexes to a software created pagetable with
the same format and characteristics as the parent IOMMU device.
The IOMMU driver allocates the pagetable and tracks it with a
unique token (PASID) - it does not touch the actual hardware.
The client driver is expected to be able to manage the pagetables
and do something interesting with them.
Some flavors of the MSM GPU are able to allow each DRM instance
to have its own pagetable (and virtual memory space) and switch them
asynchronously at the beginning of a command. This protects against
accidental or malicious corruption or copying of buffers from other
instances.
The first step is to add a MMU implementation that can allocate a
PASID and set up a msm_mmu struct to abstract (most) of the details
from the rest of the system.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/msm_iommu.c | 190 ++++++++++++++++++++++++++++++++
drivers/gpu/drm/msm/msm_mmu.h | 6 +
2 files changed, 196 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index fdbe1a8372f0..99e6611969d4 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -15,6 +15,9 @@
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <linux/hashtable.h>
+#include <linux/arm-smmu.h>
+
#include "msm_drv.h"
#include "msm_mmu.h"
@@ -34,12 +37,32 @@ static int msm_fault_handler(struct iommu_domain *domain, struct device *dev,
return 0;
}
+static bool msm_iommu_check_per_instance(struct msm_iommu *iommu)
+{
+ int val;
+
+ if (!IS_ENABLED(CONFIG_IOMMU_SVA))
+ return false;
+
+ if (iommu_domain_get_attr(iommu->domain, DOMAIN_ATTR_SPLIT_TABLES,
+ &val))
+ return false;
+
+ return val ? true : false;
+}
+
static int msm_iommu_attach(struct msm_mmu *mmu, const char * const *names,
int cnt)
{
struct msm_iommu *iommu = to_msm_iommu(mmu);
int ret;
+ if (msm_iommu_check_per_instance(iommu)) {
+ if (!iommu_sva_device_init(mmu->dev, 0, (1 << 31), NULL))
+ msm_mmu_set_feature(mmu,
+ MMU_FEATURE_PER_INSTANCE_TABLES);
+ }
+
pm_runtime_get_sync(mmu->dev);
ret = iommu_attach_device(iommu->domain, mmu->dev);
pm_runtime_put_sync(mmu->dev);
@@ -112,3 +135,170 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain)
return &iommu->base;
}
+
+struct pasid_entry {
+ int pasid;
+ u64 ttbr;
+ u32 asid;
+ struct hlist_node node;
+};
+
+DECLARE_HASHTABLE(pasid_table, 4);
+
+static int install_pasid_cb(int pasid, u64 ttbr, u32 asid, void *data)
+{
+ struct pasid_entry *entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+
+ if (!entry)
+ return -ENOMEM;
+
+ entry->pasid = pasid;
+ entry->ttbr = ttbr;
+ entry->asid = asid;
+
+ /* FIXME: Assume that we'll never have a pasid conflict? */
+ /* FIXME: locks? RCU? */
+ hash_add(pasid_table, &entry->node, pasid);
+ return 0;
+}
+
+static void remove_pasid_cb(int pasid, void *data)
+{
+ struct pasid_entry *entry;
+
+ hash_for_each_possible(pasid_table, entry, node, pasid) {
+ if (pasid == entry->pasid) {
+ hash_del(&entry->node);
+ kfree(entry);
+ return;
+ }
+ }
+}
+
+struct msm_iommu_pasid {
+ struct msm_mmu base;
+ struct device *dev;
+ int pasid;
+ u64 ttbr;
+ u32 asid;
+};
+#define to_msm_iommu_pasid(x) container_of(x, struct msm_iommu_pasid, base)
+
+static int msm_iommu_pasid_attach(struct msm_mmu *mmu,
+ const char * const *names, int cnt)
+{
+ return 0;
+}
+
+static int msm_iommu_pasid_map(struct msm_mmu *mmu, uint64_t iova,
+ struct sg_table *sgt, unsigned len, int prot)
+{
+ struct msm_iommu_pasid *pasid = to_msm_iommu_pasid(mmu);
+ int ret;
+
+ ret = iommu_sva_map_sg(pasid->pasid, iova, sgt->sgl, sgt->nents, prot);
+ WARN_ON(ret < 0);
+
+ return (ret == len) ? 0 : -EINVAL;
+}
+
+static int msm_iommu_pasid_unmap(struct msm_mmu *mmu, uint64_t iova,
+ struct sg_table *sgt, unsigned len)
+{
+ struct msm_iommu_pasid *pasid = to_msm_iommu_pasid(mmu);
+
+ iommu_sva_unmap(pasid->pasid, iova, len);
+
+ return 0;
+}
+
+static void msm_iommu_pasid_detach(struct msm_mmu *mmu,
+ const char * const *names, int cnt)
+{
+}
+
+static void msm_iommu_pasid_destroy(struct msm_mmu *mmu)
+{
+ struct msm_iommu_pasid *pasid = to_msm_iommu_pasid(mmu);
+
+ iommu_sva_free_pasid(pasid->pasid, pasid->dev);
+ kfree(pasid);
+}
+
+static const struct msm_mmu_funcs pasid_funcs = {
+ .attach = msm_iommu_pasid_attach,
+ .detach = msm_iommu_pasid_detach,
+ .map = msm_iommu_pasid_map,
+ .unmap = msm_iommu_pasid_unmap,
+ .destroy = msm_iommu_pasid_destroy,
+};
+
+static const struct arm_smmu_pasid_ops msm_iommu_pasid_ops = {
+ .install_pasid = install_pasid_cb,
+ .remove_pasid = remove_pasid_cb,
+};
+
+struct msm_mmu *msm_iommu_pasid_new(struct msm_mmu *parent)
+{
+ struct msm_iommu *parent_iommu = to_msm_iommu(parent);
+ struct msm_iommu_pasid *pasid;
+ int id;
+
+ if (!msm_mmu_has_feature(parent, MMU_FEATURE_PER_INSTANCE_TABLES))
+ return ERR_PTR(-EOPNOTSUPP);
+
+ pasid = kzalloc(sizeof(*pasid), GFP_KERNEL);
+ if (!pasid)
+ return ERR_PTR(-ENOMEM);
+
+ arm_smmu_add_pasid_ops(parent_iommu->domain, &msm_iommu_pasid_ops,
+ NULL);
+
+ id = iommu_sva_alloc_pasid(parent_iommu->domain, parent->dev);
+ if (id < 0) {
+ kfree(pasid);
+ return ERR_PTR(id);
+ }
+
+ pasid->pasid = id;
+ pasid->dev = parent->dev;
+
+ msm_mmu_init(&pasid->base, parent->dev, &pasid_funcs);
+
+ return &pasid->base;
+}
+
+/* Given a pasid return the TTBR and ASID associated with it */
+int msm_iommu_pasid_info(struct msm_mmu *mmu, u64 *ttbr, u32 *asid)
+{
+ struct msm_iommu_pasid *pasid;
+ struct pasid_entry *entry;
+
+ if (mmu->funcs->map != msm_iommu_pasid_map)
+ return -ENODEV;
+
+ pasid = to_msm_iommu_pasid(mmu);
+
+ if (!pasid->ttbr) {
+ /* Find the pasid entry in the hash */
+ hash_for_each_possible(pasid_table, entry, node, pasid->pasid) {
+ if (pasid->pasid == entry->pasid) {
+ pasid->ttbr = entry->ttbr;
+ pasid->asid = entry->asid;
+ goto out;
+ }
+ }
+
+ WARN(1, "Couldn't find the entry for pasid %d\n", pasid->pasid);
+ return -EINVAL;
+ }
+
+out:
+ if (*ttbr)
+ *ttbr = pasid->ttbr;
+
+ if (*asid)
+ *asid = pasid->asid;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h
index 85df78d71398..29436b9daa73 100644
--- a/drivers/gpu/drm/msm/msm_mmu.h
+++ b/drivers/gpu/drm/msm/msm_mmu.h
@@ -30,6 +30,9 @@ struct msm_mmu_funcs {
void (*destroy)(struct msm_mmu *mmu);
};
+/* MMU features */
+#define MMU_FEATURE_PER_INSTANCE_TABLES (1 << 0)
+
struct msm_mmu {
const struct msm_mmu_funcs *funcs;
struct device *dev;
@@ -48,6 +51,9 @@ static inline void msm_mmu_init(struct msm_mmu *mmu, struct device *dev,
struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain);
struct msm_mmu *msm_gpummu_new(struct device *dev, struct msm_gpu *gpu);
+struct msm_mmu *msm_iommu_pasid_new(struct msm_mmu *parent);
+int msm_iommu_pasid_info(struct msm_mmu *mmu, u64 *ttbr, u32 *asid);
+
static inline void msm_mmu_set_fault_handler(struct msm_mmu *mmu, void *arg,
int (*handler)(void *arg, unsigned long iova, int flags))
{
--
2.17.0
^ permalink raw reply related
* [PATCH 10/16] drm/msm: Add msm_mmu features
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
Add a few simple support functions to support a bitmask of
features that a specific MMU implementation supports. The
first feature will be per-instance pagetables coming in the
following patch.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/msm_mmu.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h
index aa2c5d4580c8..85df78d71398 100644
--- a/drivers/gpu/drm/msm/msm_mmu.h
+++ b/drivers/gpu/drm/msm/msm_mmu.h
@@ -35,6 +35,7 @@ struct msm_mmu {
struct device *dev;
int (*handler)(void *arg, unsigned long iova, int flags);
void *arg;
+ unsigned long features;
};
static inline void msm_mmu_init(struct msm_mmu *mmu, struct device *dev,
@@ -54,4 +55,16 @@ static inline void msm_mmu_set_fault_handler(struct msm_mmu *mmu, void *arg,
mmu->handler = handler;
}
+static inline void msm_mmu_set_feature(struct msm_mmu *mmu,
+ unsigned long feature)
+{
+ mmu->features |= feature;
+}
+
+static inline bool msm_mmu_has_feature(struct msm_mmu *mmu,
+ unsigned long feature)
+{
+ return (mmu->features & feature) ? true : false;
+}
+
#endif /* __MSM_MMU_H__ */
--
2.17.0
^ permalink raw reply related
* [PATCH 09/16] drm/msm/gpu: Support using split page tables for kernel buffer objects
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
arm-smmu based targets can support split pagetables (TTBR0/TTBR1).
This is most useful for implementing per-instance pagetables so that
the "user" pagetable can be swapped out while the "kernel" or
"global" pagetable remains entact.
if the target specifies a global virtual memory range then try to
enable TTBR1 (the "global" pagetable) on the domain and if
successful use the global virtual memory range for allocations
on the default GPU address space - this ensures that the global
allocations make it into the right space. Per-instance pagetables
still need additional support to be enabled but even if they
aren't set up it isn't harmful to just use TTBR1 for all
virtual memory regions and leave the other pagetable unused.
If TTBR1 support isn't enabled then fall back to the "legacy"
virtual address space both kernel and user.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/msm_gpu.c | 19 +++++++++++++++++--
drivers/gpu/drm/msm/msm_gpu.h | 4 ++--
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 2f45bea04221..78e8e56d2499 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -703,7 +703,8 @@ static int get_clocks(struct platform_device *pdev, struct msm_gpu *gpu)
static struct msm_gem_address_space *
msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
- uint64_t va_start, uint64_t va_end)
+ u64 va_start, u64 va_end,
+ u64 va_global_start, u64 va_global_end)
{
struct iommu_domain *iommu;
struct msm_gem_address_space *aspace;
@@ -721,6 +722,19 @@ msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
iommu->geometry.aperture_start = va_start;
iommu->geometry.aperture_end = va_end;
+ /* If a va_global range was specified then try to set up split tables */
+ if (va_global_start && va_global_end) {
+ int val = 1;
+
+ ret = iommu_domain_set_attr(iommu, DOMAIN_ATTR_SPLIT_TABLES,
+ &val);
+
+ if (!WARN(ret, "Unable to enable split pagetables for the IOMMU\n")) {
+ iommu->geometry.aperture_start = va_global_start;
+ iommu->geometry.aperture_end = va_global_end;
+ }
+ }
+
dev_info(gpu->dev->dev, "%s: using IOMMU\n", gpu->name);
aspace = msm_gem_address_space_create(&pdev->dev, iommu, "gpu");
@@ -813,7 +827,8 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
msm_devfreq_init(gpu);
gpu->aspace = msm_gpu_create_address_space(gpu, pdev,
- config->va_start, config->va_end);
+ config->va_start, config->va_end, config->va_start_global,
+ config->va_end_global);
if (gpu->aspace == NULL)
dev_info(drm->dev, "%s: no IOMMU, fallback to VRAM carveout!\n", name);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index b8241179175a..da58aa6c12c8 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -31,8 +31,8 @@ struct msm_gpu_perfcntr;
struct msm_gpu_config {
const char *ioname;
const char *irqname;
- uint64_t va_start;
- uint64_t va_end;
+ uint64_t va_start, va_end;
+ uint64_t va_start_global, va_end_global;
unsigned int nr_rings;
};
--
2.17.0
^ permalink raw reply related
* [PATCH 08/16] drm/msm: Pass the MMU domain index in struct msm_file_private
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
Pass the index of the MMU domain in struct msm_file_private instead
of assuming gpu->id throughout the submit path. This clears the way
to change ctx->aspace to a per-instance pagetable.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/msm_drv.c | 16 ++++------------
drivers/gpu/drm/msm/msm_drv.h | 1 +
drivers/gpu/drm/msm/msm_gem.h | 1 +
drivers/gpu/drm/msm/msm_gem_submit.c | 11 ++++++-----
drivers/gpu/drm/msm/msm_gpu.c | 5 ++---
5 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 30cd514d8f7c..2b663435a3f7 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -502,6 +502,7 @@ static void load_gpu(struct drm_device *dev)
static int context_init(struct drm_device *dev, struct drm_file *file)
{
+ struct msm_drm_private *priv = dev->dev_private;
struct msm_file_private *ctx;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -510,6 +511,7 @@ static int context_init(struct drm_device *dev, struct drm_file *file)
msm_submitqueue_init(dev, ctx);
+ ctx->aspace = priv->gpu->aspace;
file->driver_priv = ctx;
return 0;
@@ -683,17 +685,6 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device *dev, void *data,
return ret;
}
-static int msm_ioctl_gem_info_iova(struct drm_device *dev,
- struct drm_gem_object *obj, uint64_t *iova)
-{
- struct msm_drm_private *priv = dev->dev_private;
-
- if (!priv->gpu)
- return -EINVAL;
-
- return msm_gem_get_iova(obj, priv->gpu->aspace, iova);
-}
-
static int msm_ioctl_gem_info(struct drm_device *dev, void *data,
struct drm_file *file)
{
@@ -709,9 +700,10 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data,
return -ENOENT;
if (args->flags & MSM_INFO_IOVA) {
+ struct msm_file_private *ctx = file->driver_priv;
uint64_t iova;
- ret = msm_ioctl_gem_info_iova(dev, obj, &iova);
+ ret = msm_gem_get_iova(obj, ctx->aspace, &iova);
if (!ret)
args->offset = iova;
} else {
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 48ed5b9a8580..897b08135927 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -58,6 +58,7 @@ struct msm_file_private {
rwlock_t queuelock;
struct list_head submitqueues;
int queueid;
+ struct msm_gem_address_space *aspace;
};
enum msm_mdp_plane_property {
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index c5d9bd3e47a8..fe8b3aa7d76f 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -138,6 +138,7 @@ void msm_gem_vunmap(struct drm_gem_object *obj, enum msm_gem_lock subclass);
struct msm_gem_submit {
struct drm_device *dev;
struct msm_gpu *gpu;
+ struct msm_gem_address_space *aspace;
struct list_head node; /* node in ring submit list */
struct list_head bo_list;
struct ww_acquire_ctx ticket;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 7bd83e0afa97..d5dffcba9919 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -31,8 +31,8 @@
#define BO_PINNED 0x2000
static struct msm_gem_submit *submit_create(struct drm_device *dev,
- struct msm_gpu *gpu, struct msm_gpu_submitqueue *queue,
- uint32_t nr_bos, uint32_t nr_cmds)
+ struct msm_gpu *gpu, struct msm_gem_address_space *aspace,
+ struct msm_gpu_submitqueue *queue, uint32_t nr_bos, uint32_t nr_cmds)
{
struct msm_gem_submit *submit;
uint64_t sz = sizeof(*submit) + ((u64)nr_bos * sizeof(submit->bos[0])) +
@@ -46,6 +46,7 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev,
return NULL;
submit->dev = dev;
+ submit->aspace = aspace;
submit->gpu = gpu;
submit->fence = NULL;
submit->pid = get_pid(task_pid(current));
@@ -167,7 +168,7 @@ static void submit_unlock_unpin_bo(struct msm_gem_submit *submit,
struct msm_gem_object *msm_obj = submit->bos[i].obj;
if (submit->bos[i].flags & BO_PINNED)
- msm_gem_put_iova(&msm_obj->base, submit->gpu->aspace);
+ msm_gem_put_iova(&msm_obj->base, submit->aspace);
if (submit->bos[i].flags & BO_LOCKED)
ww_mutex_unlock(&msm_obj->resv->lock);
@@ -270,7 +271,7 @@ static int submit_pin_objects(struct msm_gem_submit *submit)
/* if locking succeeded, pin bo: */
ret = msm_gem_get_iova(&msm_obj->base,
- submit->gpu->aspace, &iova);
+ submit->aspace, &iova);
if (ret)
break;
@@ -471,7 +472,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
}
}
- submit = submit_create(dev, gpu, queue, args->nr_bos, args->nr_cmds);
+ submit = submit_create(dev, gpu, ctx->aspace, queue, args->nr_bos, args->nr_cmds);
if (!submit) {
ret = -ENOMEM;
goto out_unlock;
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 1c09acfb4028..2f45bea04221 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -551,7 +551,7 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
struct msm_gem_object *msm_obj = submit->bos[i].obj;
/* move to inactive: */
msm_gem_move_to_inactive(&msm_obj->base);
- msm_gem_put_iova(&msm_obj->base, gpu->aspace);
+ msm_gem_put_iova(&msm_obj->base, submit->aspace);
drm_gem_object_put(&msm_obj->base);
}
@@ -635,8 +635,7 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
/* submit takes a reference to the bo and iova until retired: */
drm_gem_object_get(&msm_obj->base);
- msm_gem_get_iova(&msm_obj->base,
- submit->gpu->aspace, &iova);
+ msm_gem_get_iova(&msm_obj->base, submit->aspace, &iova);
if (submit->bos[i].flags & MSM_SUBMIT_BO_WRITE)
msm_gem_move_to_active(&msm_obj->base, gpu, true, submit->fence);
--
2.17.0
^ permalink raw reply related
* [PATCH 07/16] drm/msm/gpu: Enable 64 bit mode by default
From: Jordan Crouse @ 2018-05-18 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518213500.31595-1-jcrouse@codeaurora.org>
A5XX GPUs can be run in either 32 or 64 bit mode. The GPU registers
and the microcode use 64 bit virtual addressing in either case but the
upper 32 bits are ignored if the GPU is in 32 bit mode. There is no
performance disadvantage to remaining in 64 bit mode even if we are
only generating 32 bit addresses so switch over now to prepare for
using addresses above 4G for targets that support them.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 14 ++++++++++++++
drivers/gpu/drm/msm/msm_iommu.c | 2 +-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index d39400e5bc42..b2c0370072dd 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -741,6 +741,20 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_HI, 0x00000000);
gpu_write(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_SIZE, 0x00000000);
+ /* Put the GPU into 64 bit by default */
+ gpu_write(gpu, REG_A5XX_CP_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_VSC_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_GRAS_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_RB_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_PC_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_HLSQ_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_VFD_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_VPC_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_UCHE_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_SP_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_TPL1_ADDR_MODE_CNTL, 0x1);
+ gpu_write(gpu, REG_A5XX_RBBM_SECVID_TSB_ADDR_MODE_CNTL, 0x1);
+
ret = adreno_hw_init(gpu);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index b23d33622f37..fdbe1a8372f0 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -30,7 +30,7 @@ static int msm_fault_handler(struct iommu_domain *domain, struct device *dev,
struct msm_iommu *iommu = arg;
if (iommu->base.handler)
return iommu->base.handler(iommu->base.arg, iova, flags);
- pr_warn_ratelimited("*** fault: iova=%08lx, flags=%d\n", iova, flags);
+ pr_warn_ratelimited("*** fault: iova=%16lx, flags=%d\n", iova, flags);
return 0;
}
--
2.17.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox