[PATCH v12 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
Reviewed-by: Steven Price
---
(no changes since v7)
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.
Changes in v5:
- Change power domain name from 2d to core2.
Changes in v4:
- Add power domain names.
Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 83a461bdeea8..ca07098a6141 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = {
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2"
};
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
+ .pm_domain_names = mediatek_mt8183_pm_domains,
+};
+
static const struct of_device_id dt_match[] = {
/* Set first to probe before the generic compatibles */
{ .compatible = "amlogic,meson-gxm-mali",
@@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t860", .data = _data, },
{ .compatible = "arm,mali-t880", .data = _data, },
{ .compatible = "arm,mali-bifrost", .data = _data, },
+ { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.31.1.368.gbe11c130af-goog
[PATCH v12 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will
require platform-specific handling for devfreq, for 2 reasons:
1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator)
does not support multiple regulators, so we'll need custom
handlers.
2. Generally, platforms with 2 regulators have platform-specific
constraints on how the voltages should be set (e.g.
minimum/maximum voltage difference between them), so we
should not just create generic handlers that simply
change the voltages without taking care of those constraints.
Disable devfreq for now on those GPUs.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
Reviewed-by: Steven Price
---
(no changes since v9)
Changes in v9:
- Explain why devfreq needs to be disabled for GPUs with >1
regulators.
Changes in v8:
- Use DRM_DEV_INFO instead of ERROR
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- devfreq: New change
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 47d27e54a34f..aca3bb9a12e4 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -92,9 +92,19 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
- ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
-pfdev->comp->num_supplies);
- if (ret) {
+ if (pfdev->comp->num_supplies > 1) {
+ /*
+* GPUs with more than 1 supply require platform-specific
handling:
+* continue without devfreq
+*/
+ DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n");
+ return 0;
+ }
+
+ opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
+ pfdev->comp->num_supplies);
+ if (IS_ERR(opp_table)) {
+ ret = PTR_ERR(opp_table);
/* Continue if the optional regulator is missing */
if (ret != -ENODEV) {
DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
--
2.31.1.368.gbe11c130af-goog
[PATCH v12 2/4] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat
---
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
< CLK_TOP_MFGPLL_CK>,
< CLK_TOP_MUX_MFG>,
<>,
< CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
(based on discussions, this probably belongs in the clock core)
This only matters for devfreq, that is disabled anyway as we don't
have platform-specific code to handle >1 supplies.
Changes in v12:
- Add gpu node to mt8183-pumpkin.dts as well (Neil Armstrong).
Changes in v11:
- mt8183*.dts: remove incorrect supply-names
Changes in v6:
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.
Changes in v5:
- Rename "2d" power domain to "core2" (keep R-B again).
Changes in v4:
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)
Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 +
.../boot/dts/mediatek/mt8183-pumpkin.dts | 5 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++
4 files changed, 120 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index edff1e03e6fe..7bc0a6a7fadf 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -42,6 +42,11 @@ {
status = "okay";
};
+ {
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index ff56bcfa3370..e4e54be1c2b2 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -279,6 +279,11 @@ dsi_out: endpoint {
};
};
+ {
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
index 0aff5eb52e88..ee912825cfc6 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
@@ -68,6 +68,11 @@ {
status = "okay";
};
+ {
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index c5e822b6b77a..c75fdeea8aa4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1118,6 +1118,111 @@ mfgcfg: syscon@1300 {
#clock-cells = <1>;
};
+ gpu: gpu@1304 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x1304 0 0x4000>;
+ interrupts =
+ ,
+ ,
+ ;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = < CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ < MT8183_POWER_DOMAIN_MFG_CORE0>,
+ < MT8183_POWER_DOMAIN_MFG_CORE1>,
+ < MT8183_POWER_DOMAIN_MFG_2D>;
+ power-domain-names = "core0", "core1", "core2";
+
+ operating-points-v2 = <_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ opp-microvolt = <625000>, <85>;
+ };
+
+ opp-3
[PATCH v12 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat --- Changes in v12: - binding: Fix min/maxItems logic (Rob Herring) Changes in v11: - binding: power-domain-names not power-domainS-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v6: - Rebased, actually tested with recent mesa driver. Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) .../bindings/gpu/arm,mali-bifrost.yaml| 30 ++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..b22cd8f1b015 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -41,10 +42,13 @@ properties: mali-supply: true + sram-supply: true + operating-points-v2: true power-domains: -maxItems: 1 +minItems: 1 +maxItems: 3 resets: maxItems: 2 @@ -87,6 +91,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +power-domains: + minItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domain-names +else: + properties: +power-domains: + maxItems: 1 +sram-supply: false examples: - | -- 2.31.1.368.gbe11c130af-goog
[PATCH v12 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! This is just a rebase of the v11, untested (but it seems like Neil Armstrong recently tested it), with small changes in binding and dts. v11 cover follows: Follow-up on the v5 [1], things have gotten significantly better in the last year, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v12: - binding: Fix min/maxItems logic (Rob Herring) - Add gpu node to mt8183-pumpkin.dts as well (Neil Armstrong). Changes in v11: - binding: power-domain-names not power-domainS-names - mt8183*.dts: remove incorrect supply-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. - Add gpu regulators to kukui dtsi as well. - Power domains are now attached to spm, not scpsys - Drop R-B. - devfreq: New change - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Rename "2d" power domain to "core2" - Rename "2d" power domain to "core2" (keep R-B again). - Change power domain name from 2d to core2. Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) - Add power-domain-names to describe the 3 domains. (kept Alyssa's reviewed-by as the change is minor) - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. Changes in v2: - Use sram instead of mali_sram as SRAM supply name. - Rename mali@ to gpu@. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 30 - arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 + .../boot/dts/mediatek/mt8183-pumpkin.dts | 5 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 16 ++- drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 7 files changed, 172 insertions(+), 4 deletions(-) -- 2.31.1.368.gbe11c130af-goog
Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
On Tue, Apr 20, 2021 at 9:01 PM Rob Herring wrote: > > On Mon, Jan 25, 2021 at 7:18 PM Nicolas Boichat wrote: > > > > Define a compatible string for the Mali Bifrost GPU found in > > Mediatek's MT8183 SoCs. > > > > Signed-off-by: Nicolas Boichat > > --- > > > > Changes in v11: > > - binding: power-domain-names not power-domainS-names > > > > Changes in v10: > > - Fix the binding to make sure sram-supply property can be provided. > > > > Changes in v9: None > > Changes in v8: None > > Changes in v7: None > > Changes in v6: > > - Rebased, actually tested with recent mesa driver. > > > > Changes in v5: > > - Rename "2d" power domain to "core2" > > > > Changes in v4: > > - Add power-domain-names description > >(kept Alyssa's reviewed-by as the change is minor) > > > > Changes in v3: None > > Changes in v2: None > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 28 +++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > index 184492162e7e..3e758f88e2cd 100644 > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > @@ -17,6 +17,7 @@ properties: > > items: > >- enum: > >- amlogic,meson-g12a-mali > > + - mediatek,mt8183-mali > >- realtek,rtd1619-mali > >- rockchip,px30-mali > >- const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully > > discoverable > > @@ -41,6 +42,8 @@ properties: > > > >mali-supply: true > > > > + sram-supply: true > > + > >operating-points-v2: true > > > >power-domains: > > @@ -87,6 +90,31 @@ allOf: > > then: > >required: > > - resets > > + - if: > > + properties: > > +compatible: > > + contains: > > +const: mediatek,mt8183-mali > > +then: > > + properties: > > +power-domains: > > + description: > > +List of phandle and PM domain specifier as documented in > > +Documentation/devicetree/bindings/power/power_domain.txt > > + minItems: 3 > > + maxItems: 3 > > This won't work because the top level schema restricts this to 1. The > top level needs to say: > > power-domains: > minItems: 1 > maxItems: 3 > > And you need just 'minItems: 3' here and 'maxItems: 1' in the else clause. > > And drop the description. That's every 'power-domains' property. > > > +power-domain-names: > > + items: > > +- const: core0 > > +- const: core1 > > +- const: core2 > > Blank line Thanks, hopefully all fixed in v12. > > + required: > > +- sram-supply > > +- power-domains > > +- power-domain-names > > +else: > > + properties: > > +sram-supply: false > > > > examples: > >- | > > -- > > 2.30.0.280.ga3ce27912f-goog > >
Re: [PATCH v11 2/4] arm64: dts: mt8183: Add node for the Mali GPU
On Tue, Apr 20, 2021 at 11:33 PM Neil Armstrong wrote:
>
> On 26/01/2021 02:17, Nicolas Boichat wrote:
> > Add a basic GPU node for mt8183.
> >
> > Signed-off-by: Nicolas Boichat
> > ---
> > The binding we use with out-of-tree Mali drivers includes more
> > clocks, this is used for devfreq: the out-of-tree driver switches
> > clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
> > switches clk_mux back to clk_main_parent:
> > (see
> > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
> > clocks =
> > < CLK_TOP_MFGPLL_CK>,
> > < CLK_TOP_MUX_MFG>,
> > <>,
> > < CLK_MFG_BG3D>;
> > clock-names =
> > "clk_main_parent",
> > "clk_mux",
> > "clk_sub_parent",
> > "subsys_mfg_cg";
> > (based on discussions, this probably belongs in the clock core)
> >
> > This only matters for devfreq, that is disabled anyway as we don't
> > have platform-specific code to handle >1 supplies.
> >
> > Changes in v11:
> > - mt8183*.dts: remove incorrect supply-names
> >
> > Changes in v10: None
> > Changes in v9: None
> > Changes in v8: None
> > Changes in v7: None
> > Changes in v6:
> > - Add gpu regulators to kukui dtsi as well.
> > - Power domains are now attached to spm, not scpsys
> > - Drop R-B.
> >
> > Changes in v5:
> > - Rename "2d" power domain to "core2" (keep R-B again).
> >
> > Changes in v4:
> > - Add power-domain-names to describe the 3 domains.
> >(kept Alyssa's reviewed-by as the change is minor)
> >
> > Changes in v3: None
> > Changes in v2:
> > - Use sram instead of mali_sram as SRAM supply name.
> > - Rename mali@ to gpu@.
> >
> > arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 +
> > .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 +
> > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++
> > 3 files changed, 115 insertions(+)
> >
>
> If you re-spin, you can also add the same changes to mt8183-pumpkin.dts :
Will do in v12.
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
> b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
> index eb6e595c2975..cc23e5df391e 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
> @@ -68,6 +68,11 @@ {
> status = "okay";
> };
>
> + {
> + mali-supply = <_vgpu_reg>;
> + sram-supply = <_vsram_gpu_reg>;
> +};
> +
> {
> pinctrl-names = "default";
> pinctrl-0 = <_pins_0>;
>
> I did a boot-test of the platform with panfrost and drm-misc-next and it
> worked fine.
Great news thanks!
>
> Thanks,
> Neil
>
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > index cba2d8933e79..1cfbea5a0101 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > @@ -42,6 +42,11 @@ {
> > status = "okay";
> > };
> >
> > + {
> > + mali-supply = <_vgpu_reg>;
> > + sram-supply = <_vsram_gpu_reg>;
> > +};
> > +
> > {
> > pinctrl-names = "default";
> > pinctrl-0 = <_pins_0>;
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > index bf2ad1294dd3..a38315b604df 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > @@ -249,6 +249,11 @@ {
> > proc-supply = <_vproc11_reg>;
> > };
> >
> > + {
> > + mali-supply = <_vgpu_reg>;
> > + sram-supply = <_vsram_gpu_reg>;
> > +};
> > +
> > {
> > pinctrl-names = "default";
> > pinctrl-0 = <_pins>;
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 5b782a4769e7..5430e05e18a0 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -964,6 +964,111 @@ mfgcfg: syscon@1300 {
> > #clock-cells = <1>;
> >
Re: [PATCH 3/3] arm64: dts: mt8183-kukui: fix dtbs_check warnings
On Wed, Apr 14, 2021 at 10:46 PM wrote:
>
> From: Matthias Brugger
>
> The dsi children don't have any reg property,
Confused, see below.
> so we don't need address and
> size cells. This makes dtbs_check happy.
>
> CC: Hsin-Yi Wang
> CC: Enric Balletbo i Serra
> CC: Nicolas Boichat
> Signed-off-by: Matthias Brugger
>
> ---
>
> arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index ff56bcfa3370..f4dca6a33168 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> @@ -251,8 +251,7 @@ {
>
> {
> status = "okay";
> - #address-cells = <1>;
> - #size-cells = <0>;
> +
> panel: panel@0 {
> /* compatible will be set in board dts */
> reg = <0>;
^^ isn't that... a reg property?
> --
> 2.30.2
>
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Wed, Feb 24, 2021 at 6:44 PM Nicolas Boichat wrote:
>
> On Wed, Feb 24, 2021 at 6:22 PM Luis Henriques wrote:
> >
> > On Tue, Feb 23, 2021 at 08:00:54PM -0500, Olga Kornievskaia wrote:
> > > On Mon, Feb 22, 2021 at 5:25 AM Luis Henriques wrote:
> > > >
> > > > A regression has been reported by Nicolas Boichat, found while using the
> > > > copy_file_range syscall to copy a tracefs file. Before commit
> > > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the
> > > > kernel would return -EXDEV to userspace when trying to copy a file
> > > > across
> > > > different filesystems. After this commit, the syscall doesn't fail
> > > > anymore
> > > > and instead returns zero (zero bytes copied), as this file's content is
> > > > generated on-the-fly and thus reports a size of zero.
> > > >
> > > > This patch restores some cross-filesystem copy restrictions that existed
> > > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
> > > > devices"). Filesystems are still allowed to fall-back to the VFS
> > > > generic_copy_file_range() implementation, but that has now to be done
> > > > explicitly.
> > > >
> > > > nfsd is also modified to fall-back into generic_copy_file_range() in
> > > > case
> > > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV.
> > > >
> > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
> > > > devices")
> > > > Link:
> > > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/
> > > > Link:
> > > > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/
> > > > Link:
> > > > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/
> > > > Reported-by: Nicolas Boichat
> > > > Signed-off-by: Luis Henriques
> > >
> > > I tested v8 and I believe it works for NFS.
> >
> > Thanks a lot for the testing. And to everyone else for reviews,
> > feedback,... and patience.
>
> Thanks so much to you!!!
>
> Works here, you can add my
> Tested-by: Nicolas Boichat
What happened to this patch? It does not seem to have been picked up
yet? Any reason why?
> >
> > I'll now go look into the manpage and see what needs to be changed.
> >
> > Cheers,
> > --
> > Luís
[for-stable-4.19 PATCH v2 2/2] lkdtm: don't move ctors to .rodata
From: Mark Rutland
commit 3f618ab3323407ee4c6a6734a37eb6e9663ebfb9 upstream.
When building with KASAN and LKDTM, clang may implictly generate an
asan.module_ctor function in the LKDTM rodata object. The Makefile moves
the lkdtm_rodata_do_nothing() function into .rodata by renaming the
file's .text section to .rodata, and consequently also moves the ctor
function into .rodata, leading to a boot time crash (splat below) when
the ctor is invoked by do_ctors().
Let's prevent this by marking the function as noinstr rather than
notrace, and renaming the file's .noinstr.text to .rodata. Marking the
function as noinstr will prevent tracing and kprobes, and will inhibit
any undesireable compiler instrumentation.
The ctor function (if any) will be placed in .text and will work
correctly.
Example splat before this patch is applied:
[0.916359] Unable to handle kernel execute from non-executable memory at
virtual address a0006b60f5ac
[0.922088] Mem abort info:
[0.922828] ESR = 0x860e
[0.923635] EC = 0x21: IABT (current EL), IL = 32 bits
[0.925036] SET = 0, FnV = 0
[0.925838] EA = 0, S1PTW = 0
[0.926714] swapper pgtable: 4k pages, 48-bit VAs, pgdp=427b3000
[0.928489] [a0006b60f5ac] pgd=00023003, p4d=00023003,
pud=00023fffe003, pmd=006842000f01
[0.931330] Internal error: Oops: 860e [#1] PREEMPT SMP
[0.932806] Modules linked in:
[0.933617] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc7 #2
[0.935620] Hardware name: linux,dummy-virt (DT)
[0.936924] pstate: 4045 (nZcv daif +PAN -UAO -TCO BTYPE=--)
[0.938609] pc : asan.module_ctor+0x0/0x14
[0.939759] lr : do_basic_setup+0x4c/0x70
[0.940889] sp : 27b600177e30
[0.941815] x29: 27b600177e30 x28:
[0.943306] x27: x26:
[0.944803] x25: x24:
[0.946289] x23: 0001 x22:
[0.94] x21: a0006bf4a890 x20: a0006befb6c0
[0.949271] x19: a0006bef9358 x18: 0068
[0.950756] x17: fff8 x16:
[0.952246] x15: x14:
[0.953734] x13: 838a16d5 x12: 0001
[0.955223] x11: 94000da74041 x10: dfffa000
[0.956715] x9 : x8 : a0006b60f5ac
[0.958199] x7 : f9f9f9f9f9f9f9f9 x6 : 003f
[0.959683] x5 : 0040 x4 :
[0.961178] x3 : a0006bdc15a0 x2 : 0005
[0.962662] x1 : 00f9 x0 : a0006bef9350
[0.964155] Call trace:
[0.964844] asan.module_ctor+0x0/0x14
[0.965895] kernel_init_freeable+0x158/0x198
[0.967115] kernel_init+0x14/0x19c
[0.968104] ret_from_fork+0x10/0x30
[0.969110] Code: 0003 ()
[0.970815] ---[ end trace b5339784e20d015c ]---
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Kees Cook
Acked-by: Kees Cook
Signed-off-by: Mark Rutland
Link: https://lore.kernel.org/r/20201207170533.10738-1-mark.rutl...@arm.com
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Nicolas Boichat
---
(no changes since v1)
drivers/misc/lkdtm/Makefile | 2 +-
drivers/misc/lkdtm/rodata.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
index cce47a15a79f..aeb960cb096d 100644
--- a/drivers/misc/lkdtm/Makefile
+++ b/drivers/misc/lkdtm/Makefile
@@ -13,7 +13,7 @@ KCOV_INSTRUMENT_rodata.o := n
OBJCOPYFLAGS :=
OBJCOPYFLAGS_rodata_objcopy.o := \
- --rename-section .text=.rodata,alloc,readonly,load
+ --rename-section
.noinstr.text=.rodata,alloc,readonly,load
targets += rodata.o rodata_objcopy.o
$(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE
$(call if_changed,objcopy)
diff --git a/drivers/misc/lkdtm/rodata.c b/drivers/misc/lkdtm/rodata.c
index 58d180af72cf..baacb876d1d9 100644
--- a/drivers/misc/lkdtm/rodata.c
+++ b/drivers/misc/lkdtm/rodata.c
@@ -5,7 +5,7 @@
*/
#include "lkdtm.h"
-void notrace lkdtm_rodata_do_nothing(void)
+void noinstr lkdtm_rodata_do_nothing(void)
{
/* Does nothing. We just want an architecture agnostic "return". */
}
--
2.31.0.rc2.261.g7f71774620-goog
[for-stable-4.19 PATCH v2 1/2] vmlinux.lds.h: Create section for protection against instrumentation
From: Thomas Gleixner
commit 655389433e7efec589838b400a2a652b3ffa upstream.
Some code pathes, especially the low level entry code, must be protected
against instrumentation for various reasons:
- Low level entry code can be a fragile beast, especially on x86.
- With NO_HZ_FULL RCU state needs to be established before using it.
Having a dedicated section for such code allows to validate with tooling
that no unsafe functions are invoked.
Add the .noinstr.text section and the noinstr attribute to mark
functions. noinstr implies notrace. Kprobes will gain a section check
later.
Provide also a set of markers: instrumentation_begin()/end()
These are used to mark code inside a noinstr function which calls
into regular instrumentable text section as safe.
The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is
enabled as the end marker emits a NOP to prevent the compiler from merging
the annotation points. This means the objtool verification requires a
kernel compiled with this option.
Signed-off-by: Thomas Gleixner
Reviewed-by: Alexandre Chartre
Acked-by: Peter Zijlstra
Link: https://lkml.kernel.org/r/20200505134100.075416...@linutronix.de
[Nicolas:
Guard noinstr macro in include/linux/compiler_types.h in __KERNEL__
&& !__ASSEMBLY__, otherwise noinstr is expanded in the linker
script construct.
Upstream does not have this problem as many macros were moved by
commit 71391bdd2e9a ("include/linux/compiler_types.h: don't pollute
userspace with macro definitions"). We take the minimal approach here
and just guard the new macro.
Minor context conflicts in:
arch/powerpc/kernel/vmlinux.lds.S
include/asm-generic/vmlinux.lds.h
include/linux/compiler.h]
Signed-off-by: Nicolas Boichat
---
Technically guarding with !__ASSEMBLY__ should be enough, but
there seems to be no reason to expose this new macro when
!__KERNEL__, so let's just match what upstream does.
Changes in v2:
- Guard noinstr macro by __KERNEL__ && !__ASSEMBLY__ to prevent
expansion in linker script and match upstream.
arch/powerpc/kernel/vmlinux.lds.S | 1 +
include/asm-generic/sections.h| 3 ++
include/asm-generic/vmlinux.lds.h | 10 ++
include/linux/compiler.h | 54 +++
include/linux/compiler_types.h| 6
scripts/mod/modpost.c | 2 +-
6 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S
b/arch/powerpc/kernel/vmlinux.lds.S
index 695432965f20..9b346f3d2814 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -99,6 +99,7 @@ SECTIONS
#endif
/* careful! __ftr_alt_* sections need to be close to .text */
*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
__ftr_alt_* .ref.text);
+ NOINSTR_TEXT
SCHED_TEXT
CPUIDLE_TEXT
LOCK_TEXT
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 849cd8eb5ca0..ea5987bb0b84 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -53,6 +53,9 @@ extern char __ctors_start[], __ctors_end[];
/* Start and end of .opd section - used for function descriptors. */
extern char __start_opd[], __end_opd[];
+/* Start and end of instrumentation protected text section */
+extern char __noinstr_text_start[], __noinstr_text_end[];
+
extern __visible const void __nosave_begin, __nosave_end;
/* Function descriptor handling (if any). Override in asm/sections.h */
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 2d632a74cc5e..88484ee023ca 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -482,6 +482,15 @@
__security_initcall_end = .;\
}
+/*
+ * Non-instrumentable text section
+ */
+#define NOINSTR_TEXT \
+ ALIGN_FUNCTION(); \
+ __noinstr_text_start = .; \
+ *(.noinstr.text)\
+ __noinstr_text_end = .;
+
/*
* .text section. Map to function alignment to avoid address changes
* during second ld run in second ld pass when generating System.map
@@ -496,6 +505,7 @@
*(TEXT_MAIN .text.fixup)\
*(.text.unlikely .text.unlikely.*) \
*(.text.unknown .text.unknown.*)\
+ NOINSTR_TEXT\
*(.text..refcount) \
*(.ref.text)\
MEM_KEEP(init.text*)\
diff --git a/include/linux/
[for-stable-4.19 PATCH v2 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64
Backport 2 patches that are required to make KASAN+LKDTM work with recent clang (patch 2/2 has a complete description). Tested on our chromeos-4.19 branch. Also compile tested on x86-64 and arm64 with gcc this time around. Patch 1/2 adds a guard around noinstr that matches upstream, to prevent a build issue, and has some minor context conflicts. Patch 2/2 is a clean backport. These patches have been merged to 5.4 stable already. We might need to backport to older stable branches, but this is what I could test for now. Changes in v2: - Guard noinstr macro by __KERNEL__ && !__ASSEMBLY__ to prevent expansion in linker script and match upstream. Mark Rutland (1): lkdtm: don't move ctors to .rodata Thomas Gleixner (1): vmlinux.lds.h: Create section for protection against instrumentation arch/powerpc/kernel/vmlinux.lds.S | 1 + drivers/misc/lkdtm/Makefile | 2 +- drivers/misc/lkdtm/rodata.c | 2 +- include/asm-generic/sections.h| 3 ++ include/asm-generic/vmlinux.lds.h | 10 ++ include/linux/compiler.h | 54 +++ include/linux/compiler_types.h| 6 scripts/mod/modpost.c | 2 +- 8 files changed, 77 insertions(+), 3 deletions(-) -- 2.31.0.rc2.261.g7f71774620-goog
Re: [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation
On Fri, Mar 19, 2021 at 7:55 PM Greg Kroah-Hartman
wrote:
>
> On Fri, Mar 19, 2021 at 12:20:22PM +0100, Alexandre Chartre wrote:
> >
> > On 3/19/21 11:39 AM, Greg Kroah-Hartman wrote:
> > > On Fri, Mar 19, 2021 at 07:54:15AM +0800, Nicolas Boichat wrote:
> > > > From: Thomas Gleixner
> > > >
> > > > commit 655389433e7efec589838b400a2a652b3ffa upstream.
> > > >
> > > > Some code pathes, especially the low level entry code, must be protected
> > > > against instrumentation for various reasons:
> > > >
> > > > - Low level entry code can be a fragile beast, especially on x86.
> > > >
> > > > - With NO_HZ_FULL RCU state needs to be established before using it.
> > > >
> > > > Having a dedicated section for such code allows to validate with tooling
> > > > that no unsafe functions are invoked.
> > > >
> > > > Add the .noinstr.text section and the noinstr attribute to mark
> > > > functions. noinstr implies notrace. Kprobes will gain a section check
> > > > later.
> > > >
> > > > Provide also a set of markers: instrumentation_begin()/end()
> > > >
> > > > These are used to mark code inside a noinstr function which calls
> > > > into regular instrumentable text section as safe.
> > > >
> > > > The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is
> > > > enabled as the end marker emits a NOP to prevent the compiler from
> > > > merging
> > > > the annotation points. This means the objtool verification requires a
> > > > kernel compiled with this option.
> > > >
> > > > Signed-off-by: Thomas Gleixner
> > > > Reviewed-by: Alexandre Chartre
> > > > Acked-by: Peter Zijlstra
> > > > Link: https://lkml.kernel.org/r/20200505134100.075416...@linutronix.de
> > > >
> > > > [Nicolas: context conflicts in:
> > > > arch/powerpc/kernel/vmlinux.lds.S
> > > > include/asm-generic/vmlinux.lds.h
> > > > include/linux/compiler.h
> > > > include/linux/compiler_types.h]
> > > > Signed-off-by: Nicolas Boichat
> > >
> > > Did you build this on x86?
> > >
> > > I get the following build error:
> > >
> > > ld:./arch/x86/kernel/vmlinux.lds:20: syntax error
> > >
> > > And that line looks like:
> > >
> > > . = ALIGN(8); *(.text.hot .text.hot.*) *(.text .text.fixup)
> > > *(.text.unlikely .text.unlikely.*) *(.text.unknown .text.unknown.*) . =
> > > ALIGN(8); __noinstr_text_start = .; *(.__attribute__((noinline))
> > > __attribute__((no_instrument_function))
> > > __attribute((__section__(".noinstr.text"))).text) __noinstr_text_end = .;
> > > *(.text..refcount) *(.ref.text) *(.meminit.text*) *(.memexit.text*)
> > >
> >
> > In the NOINSTR_TEXT macro, noinstr is expanded with the value of the noinstr
> > macro from linux/compiler_types.h while it shouldn't.
> >
> > The problem is possibly that the noinstr macro is defined for assembly. Make
> > sure that the macro is not defined for assembly e.g.:
> >
> > #ifndef __ASSEMBLY__
> >
> > /* Section for code which can't be instrumented at all */
> > #define noinstr
> > \
> > noinline notrace __attribute((__section__(".noinstr.text")))
> >
> > #endif
>
> This implies that the backport is incorrect, so I'll wait for an updated
> version...
Yep, sorry about that. I did test on ARM64 only and these patches
happily went through our Chrome OS CQ (we don't have gcc coverage
though).
Guenter has a fixup here with explanation:
https://crrev.com/c/2776332, I'll look carefully and resubmit.
Thanks,
> thanks,
>
> greg k-h
[for-stable-4.19 PATCH 2/2] lkdtm: don't move ctors to .rodata
From: Mark Rutland
commit 3f618ab3323407ee4c6a6734a37eb6e9663ebfb9 upstream.
When building with KASAN and LKDTM, clang may implictly generate an
asan.module_ctor function in the LKDTM rodata object. The Makefile moves
the lkdtm_rodata_do_nothing() function into .rodata by renaming the
file's .text section to .rodata, and consequently also moves the ctor
function into .rodata, leading to a boot time crash (splat below) when
the ctor is invoked by do_ctors().
Let's prevent this by marking the function as noinstr rather than
notrace, and renaming the file's .noinstr.text to .rodata. Marking the
function as noinstr will prevent tracing and kprobes, and will inhibit
any undesireable compiler instrumentation.
The ctor function (if any) will be placed in .text and will work
correctly.
Example splat before this patch is applied:
[0.916359] Unable to handle kernel execute from non-executable memory at
virtual address a0006b60f5ac
[0.922088] Mem abort info:
[0.922828] ESR = 0x860e
[0.923635] EC = 0x21: IABT (current EL), IL = 32 bits
[0.925036] SET = 0, FnV = 0
[0.925838] EA = 0, S1PTW = 0
[0.926714] swapper pgtable: 4k pages, 48-bit VAs, pgdp=427b3000
[0.928489] [a0006b60f5ac] pgd=00023003, p4d=00023003,
pud=00023fffe003, pmd=006842000f01
[0.931330] Internal error: Oops: 860e [#1] PREEMPT SMP
[0.932806] Modules linked in:
[0.933617] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc7 #2
[0.935620] Hardware name: linux,dummy-virt (DT)
[0.936924] pstate: 4045 (nZcv daif +PAN -UAO -TCO BTYPE=--)
[0.938609] pc : asan.module_ctor+0x0/0x14
[0.939759] lr : do_basic_setup+0x4c/0x70
[0.940889] sp : 27b600177e30
[0.941815] x29: 27b600177e30 x28:
[0.943306] x27: x26:
[0.944803] x25: x24:
[0.946289] x23: 0001 x22:
[0.94] x21: a0006bf4a890 x20: a0006befb6c0
[0.949271] x19: a0006bef9358 x18: 0068
[0.950756] x17: fff8 x16:
[0.952246] x15: x14:
[0.953734] x13: 838a16d5 x12: 0001
[0.955223] x11: 94000da74041 x10: dfffa000
[0.956715] x9 : x8 : a0006b60f5ac
[0.958199] x7 : f9f9f9f9f9f9f9f9 x6 : 003f
[0.959683] x5 : 0040 x4 :
[0.961178] x3 : a0006bdc15a0 x2 : 0005
[0.962662] x1 : 00f9 x0 : a0006bef9350
[0.964155] Call trace:
[0.964844] asan.module_ctor+0x0/0x14
[0.965895] kernel_init_freeable+0x158/0x198
[0.967115] kernel_init+0x14/0x19c
[0.968104] ret_from_fork+0x10/0x30
[0.969110] Code: 0003 ()
[0.970815] ---[ end trace b5339784e20d015c ]---
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Kees Cook
Acked-by: Kees Cook
Signed-off-by: Mark Rutland
Link: https://lore.kernel.org/r/20201207170533.10738-1-mark.rutl...@arm.com
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Nicolas Boichat
---
drivers/misc/lkdtm/Makefile | 2 +-
drivers/misc/lkdtm/rodata.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
index cce47a15a79f..aeb960cb096d 100644
--- a/drivers/misc/lkdtm/Makefile
+++ b/drivers/misc/lkdtm/Makefile
@@ -13,7 +13,7 @@ KCOV_INSTRUMENT_rodata.o := n
OBJCOPYFLAGS :=
OBJCOPYFLAGS_rodata_objcopy.o := \
- --rename-section .text=.rodata,alloc,readonly,load
+ --rename-section
.noinstr.text=.rodata,alloc,readonly,load
targets += rodata.o rodata_objcopy.o
$(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE
$(call if_changed,objcopy)
diff --git a/drivers/misc/lkdtm/rodata.c b/drivers/misc/lkdtm/rodata.c
index 58d180af72cf..baacb876d1d9 100644
--- a/drivers/misc/lkdtm/rodata.c
+++ b/drivers/misc/lkdtm/rodata.c
@@ -5,7 +5,7 @@
*/
#include "lkdtm.h"
-void notrace lkdtm_rodata_do_nothing(void)
+void noinstr lkdtm_rodata_do_nothing(void)
{
/* Does nothing. We just want an architecture agnostic "return". */
}
--
2.31.0.rc2.261.g7f71774620-goog
[for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation
From: Thomas Gleixner
commit 655389433e7efec589838b400a2a652b3ffa upstream.
Some code pathes, especially the low level entry code, must be protected
against instrumentation for various reasons:
- Low level entry code can be a fragile beast, especially on x86.
- With NO_HZ_FULL RCU state needs to be established before using it.
Having a dedicated section for such code allows to validate with tooling
that no unsafe functions are invoked.
Add the .noinstr.text section and the noinstr attribute to mark
functions. noinstr implies notrace. Kprobes will gain a section check
later.
Provide also a set of markers: instrumentation_begin()/end()
These are used to mark code inside a noinstr function which calls
into regular instrumentable text section as safe.
The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is
enabled as the end marker emits a NOP to prevent the compiler from merging
the annotation points. This means the objtool verification requires a
kernel compiled with this option.
Signed-off-by: Thomas Gleixner
Reviewed-by: Alexandre Chartre
Acked-by: Peter Zijlstra
Link: https://lkml.kernel.org/r/20200505134100.075416...@linutronix.de
[Nicolas: context conflicts in:
arch/powerpc/kernel/vmlinux.lds.S
include/asm-generic/vmlinux.lds.h
include/linux/compiler.h
include/linux/compiler_types.h]
Signed-off-by: Nicolas Boichat
---
arch/powerpc/kernel/vmlinux.lds.S | 1 +
include/asm-generic/sections.h| 3 ++
include/asm-generic/vmlinux.lds.h | 10 ++
include/linux/compiler.h | 54 +++
include/linux/compiler_types.h| 4 +++
scripts/mod/modpost.c | 2 +-
6 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S
b/arch/powerpc/kernel/vmlinux.lds.S
index 695432965f20..9b346f3d2814 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -99,6 +99,7 @@ SECTIONS
#endif
/* careful! __ftr_alt_* sections need to be close to .text */
*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
__ftr_alt_* .ref.text);
+ NOINSTR_TEXT
SCHED_TEXT
CPUIDLE_TEXT
LOCK_TEXT
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 849cd8eb5ca0..ea5987bb0b84 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -53,6 +53,9 @@ extern char __ctors_start[], __ctors_end[];
/* Start and end of .opd section - used for function descriptors. */
extern char __start_opd[], __end_opd[];
+/* Start and end of instrumentation protected text section */
+extern char __noinstr_text_start[], __noinstr_text_end[];
+
extern __visible const void __nosave_begin, __nosave_end;
/* Function descriptor handling (if any). Override in asm/sections.h */
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 2d632a74cc5e..88484ee023ca 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -482,6 +482,15 @@
__security_initcall_end = .;\
}
+/*
+ * Non-instrumentable text section
+ */
+#define NOINSTR_TEXT \
+ ALIGN_FUNCTION(); \
+ __noinstr_text_start = .; \
+ *(.noinstr.text)\
+ __noinstr_text_end = .;
+
/*
* .text section. Map to function alignment to avoid address changes
* during second ld run in second ld pass when generating System.map
@@ -496,6 +505,7 @@
*(TEXT_MAIN .text.fixup)\
*(.text.unlikely .text.unlikely.*) \
*(.text.unknown .text.unknown.*)\
+ NOINSTR_TEXT\
*(.text..refcount) \
*(.ref.text)\
MEM_KEEP(init.text*)\
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 6b6505e3b2c7..6a53300cbd1e 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -129,11 +129,65 @@ void ftrace_likely_update(struct ftrace_likely_data *f,
int val,
".pushsection .discard.unreachable\n\t" \
".long 999b - .\n\t"\
".popsection\n\t"
+
+#ifdef CONFIG_DEBUG_ENTRY
+/* Begin/end of an instrumentation safe region */
+#define instrumentation_begin() ({ \
+ asm volatile("%c0:\n\t" \
+
[for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64
Backport 2 patches that are required to make KASAN+LKDTM work with recent clang (patch 2/2 has a complete description). Tested on our chromeos-4.19 branch. Patch 1/2 is context conflict only, and 2/2 is a clean backport. These patches have been merged to 5.4 stable already. We might need to backport to older stable branches, but this is what I could test for now. Mark Rutland (1): lkdtm: don't move ctors to .rodata Thomas Gleixner (1): vmlinux.lds.h: Create section for protection against instrumentation arch/powerpc/kernel/vmlinux.lds.S | 1 + drivers/misc/lkdtm/Makefile | 2 +- drivers/misc/lkdtm/rodata.c | 2 +- include/asm-generic/sections.h| 3 ++ include/asm-generic/vmlinux.lds.h | 10 ++ include/linux/compiler.h | 54 +++ include/linux/compiler_types.h| 4 +++ scripts/mod/modpost.c | 2 +- 8 files changed, 75 insertions(+), 3 deletions(-) -- 2.31.0.rc2.261.g7f71774620-goog
Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Mon, Mar 1, 2021 at 4:59 PM Linus Walleij wrote: > > On Thu, Feb 11, 2021 at 4:34 AM Nicolas Boichat wrote: > > > Many of the DSI flags have names opposite to their actual effects, > > e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually > > be disabled. Fix this by including _NO_ in the flag names, e.g. > > MIPI_DSI_MODE_NO_EOT_PACKET. > > Unless someone like me interpreted it literally... > > Like in these: > > > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- > > drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +- > > drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +- > > drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +- > > > diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c > > b/drivers/gpu/drm/mcde/mcde_dsi.c > > index 2314c8122992..f4cdc3cfd7d0 100644 > > --- a/drivers/gpu/drm/mcde/mcde_dsi.c > > +++ b/drivers/gpu/drm/mcde/mcde_dsi.c > > @@ -760,7 +760,7 @@ static void mcde_dsi_start(struct mcde_dsi *d) > > DSI_MCTL_MAIN_DATA_CTL_BTA_EN | > > DSI_MCTL_MAIN_DATA_CTL_READ_EN | > > DSI_MCTL_MAIN_DATA_CTL_REG_TE_EN; > > - if (d->mdsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) > > + if (d->mdsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) > > val |= DSI_MCTL_MAIN_DATA_CTL_HOST_EOT_GEN; > > If you read the code you can see that this is interpreted as inserting > an EOT packet, so here you need to change the logic such: > > if (!d->mdsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) > val |= DSI_MCTL_MAIN_DATA_CTL_HOST_EOT_GEN; > > This will make sure the host generates the EOT packet in HS mode > *unless* the flag is set. > > (I checked the data sheet.) > > > diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c > > b/drivers/gpu/drm/panel/panel-novatek-nt35510.c > > index b9a0e56f33e2..9d9334656803 100644 > > --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c > > +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c > > @@ -899,7 +899,7 @@ static int nt35510_probe(struct mipi_dsi_device *dsi) > > dsi->hs_rate = 34944; > > dsi->lp_rate = 960; > > dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS | > > - MIPI_DSI_MODE_EOT_PACKET; > > + MIPI_DSI_MODE_NO_EOT_PACKET; > > Here you should just delete the MIPI_DSI_MODE_EOT_PACKET > flag because this was used with the MCDE driver which interpret the > flag literally. > > > diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > > b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > > index 4aac0d1573dd..b04b9975e9b2 100644 > > --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > > +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > > @@ -186,7 +186,7 @@ static int s6d16d0_probe(struct mipi_dsi_device *dsi) > > */ > > dsi->mode_flags = > > MIPI_DSI_CLOCK_NON_CONTINUOUS | > > - MIPI_DSI_MODE_EOT_PACKET; > > + MIPI_DSI_MODE_NO_EOT_PACKET; > > Same, just delete the flag. > > > --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c > > +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c > > @@ -97,7 +97,7 @@ static int s6e63m0_dsi_probe(struct mipi_dsi_device *dsi) > > dsi->hs_rate = 34944; > > dsi->lp_rate = 960; > > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > > - MIPI_DSI_MODE_EOT_PACKET | > > + MIPI_DSI_MODE_NO_EOT_PACKET | > > MIPI_DSI_MODE_VIDEO_BURST; > > Same, just delete the flag. > > > diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c > > b/drivers/gpu/drm/panel/panel-sony-acx424akp.c > > index 065efae213f5..6b706cbf2f9c 100644 > > --- a/drivers/gpu/drm/panel/panel-sony-acx424akp.c > > +++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c > > @@ -450,7 +450,7 @@ static int acx424akp_probe(struct mipi_dsi_device *dsi) > > else > > dsi->mode_flags = > > MIPI_DSI_CLOCK_NON_CONTINUOUS | > > - MIPI_DSI_MODE_EOT_PACKET; > > + MIPI_DSI_MODE_NO_EOT_PACKET; > > Same, just delete the flag. > > These are all just semantic bugs due to the ambiguity of the flags, it is > possible to provide a Fixes: flag for each file using this flag the wrong way > but I dunno if it's worth it. Wow nice catch. I think we should fix all of those _before_ my patch is applied, with proper Fixes tags so that this can be backported to stable branches, even if it's a no-op. I can look into it but that may take a bit of time. Thanks, > > Yours, > Linus Walleij
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Wed, Feb 24, 2021 at 6:22 PM Luis Henriques wrote:
>
> On Tue, Feb 23, 2021 at 08:00:54PM -0500, Olga Kornievskaia wrote:
> > On Mon, Feb 22, 2021 at 5:25 AM Luis Henriques wrote:
> > >
> > > A regression has been reported by Nicolas Boichat, found while using the
> > > copy_file_range syscall to copy a tracefs file. Before commit
> > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the
> > > kernel would return -EXDEV to userspace when trying to copy a file across
> > > different filesystems. After this commit, the syscall doesn't fail
> > > anymore
> > > and instead returns zero (zero bytes copied), as this file's content is
> > > generated on-the-fly and thus reports a size of zero.
> > >
> > > This patch restores some cross-filesystem copy restrictions that existed
> > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
> > > devices"). Filesystems are still allowed to fall-back to the VFS
> > > generic_copy_file_range() implementation, but that has now to be done
> > > explicitly.
> > >
> > > nfsd is also modified to fall-back into generic_copy_file_range() in case
> > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV.
> > >
> > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices")
> > > Link:
> > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/
> > > Link:
> > > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/
> > > Link:
> > > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/
> > > Reported-by: Nicolas Boichat
> > > Signed-off-by: Luis Henriques
> >
> > I tested v8 and I believe it works for NFS.
>
> Thanks a lot for the testing. And to everyone else for reviews,
> feedback,... and patience.
Thanks so much to you!!!
Works here, you can add my
Tested-by: Nicolas Boichat
>
> I'll now go look into the manpage and see what needs to be changed.
>
> Cheers,
> --
> Luís
Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Mon, Feb 22, 2021 at 3:21 PM Andrzej Hajda wrote:
>
> Hi Nicolas,
>
> W dniu 22.02.2021 o 06:31, Nicolas Boichat pisze:
> > On Mon, Feb 22, 2021 at 3:08 AM Laurent Pinchart
> > wrote:
> >> Hi Nicolas,
> >>
> >> Thank you for the patch.
> >>
> >> On Thu, Feb 11, 2021 at 11:33:55AM +0800, Nicolas Boichat wrote:
> >>> Many of the DSI flags have names opposite to their actual effects,
> >>> e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually
> >>> be disabled. Fix this by including _NO_ in the flag names, e.g.
> >>> MIPI_DSI_MODE_NO_EOT_PACKET.
> >>>
> >>> Signed-off-by: Nicolas Boichat
> >> This looks good to me, it increases readability.
> >>
> >> Reviewed-by: Laurent Pinchart
> >>
> >> Please however see the end of the mail for a comment.
>
>
> Reviewed-by: Andrzej Hajda
>
> And comment at the end.
>
> >>
> >>> ---
> >>> I considered adding _DISABLE_ instead, but that'd make the
> >>> flag names a big too long.
> >>>
> >>> Generated with:
> >>> flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \
> >>>xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {}
> >>> flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \
> >>>xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {}
> >>> flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \
> >>>xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {}
> >>> flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \
> >>>xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {}
> >>> (then minor format changes)
> >> Ever tried coccinelle ? :-)
> > Fun project for next time ,-)
> >
> >>> drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +-
> >>> drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +-
> >>> drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++--
> >>> drivers/gpu/drm/bridge/tc358768.c| 2 +-
> >>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8
> >>> drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
> >>> drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
> >>> drivers/gpu/drm/msm/dsi/dsi_host.c | 8
> >>> drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +-
> >>> drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +-
> >>> drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c| 2 +-
> >>> drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++--
> >>> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-simple.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +-
> >>> drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +-
> >>> include/drm/drm_mipi_dsi.h | 8
> >>> 25 files changed, 36 insertions(+), 36 deletions(-)
> >>>
> >>> []
> >>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> >>> index 360e6377e84b..ba91cf22af51 100644
> >>> --- a/include/drm/drm_mipi_dsi.h
> >>> +++ b/include/drm/drm_mipi_dsi.h
> >>> @@ -119,15 +119,15 @@ struct mipi_dsi_host
> >>> *of_find_mipi_dsi_host_by_node(struct device_node *node);
> >>> /* enable hsync-end packets in vsync-pulse and v-porch area */
> >>> #define MIPI_DSI_MODE_VIDEO_HSE BIT(4)
> >> We're mixing bits that enable a feature and bits that disable a feature.
> >> Are these bits defined in the DSI spec, or internal to DRM ? In the
> >> latter case, would it make sense to standardi
Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Mon, Feb 22, 2021 at 3:08 AM Laurent Pinchart
wrote:
>
> Hi Nicolas,
>
> Thank you for the patch.
>
> On Thu, Feb 11, 2021 at 11:33:55AM +0800, Nicolas Boichat wrote:
> > Many of the DSI flags have names opposite to their actual effects,
> > e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually
> > be disabled. Fix this by including _NO_ in the flag names, e.g.
> > MIPI_DSI_MODE_NO_EOT_PACKET.
> >
> > Signed-off-by: Nicolas Boichat
>
> This looks good to me, it increases readability.
>
> Reviewed-by: Laurent Pinchart
>
> Please however see the end of the mail for a comment.
>
> > ---
> > I considered adding _DISABLE_ instead, but that'd make the
> > flag names a big too long.
> >
> > Generated with:
> > flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \
> > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {}
> > flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \
> > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {}
> > flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \
> > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {}
> > flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \
> > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {}
> > (then minor format changes)
>
> Ever tried coccinelle ? :-)
Fun project for next time ,-)
>
> > drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +-
> > drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +-
> > drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++--
> > drivers/gpu/drm/bridge/tc358768.c| 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8
> > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 8
> > drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +-
> > drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +-
> > drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +-
> > drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +-
> > drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +-
> > drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +-
> > drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +-
> > drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +-
> > drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +-
> > drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +-
> > drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c| 2 +-
> > drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++--
> > drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +-
> > drivers/gpu/drm/panel/panel-simple.c | 2 +-
> > drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +-
> > drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +-
> > include/drm/drm_mipi_dsi.h | 8
> > 25 files changed, 36 insertions(+), 36 deletions(-)
> >
> > []
> > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> > index 360e6377e84b..ba91cf22af51 100644
> > --- a/include/drm/drm_mipi_dsi.h
> > +++ b/include/drm/drm_mipi_dsi.h
> > @@ -119,15 +119,15 @@ struct mipi_dsi_host
> > *of_find_mipi_dsi_host_by_node(struct device_node *node);
> > /* enable hsync-end packets in vsync-pulse and v-porch area */
> > #define MIPI_DSI_MODE_VIDEO_HSE BIT(4)
>
> We're mixing bits that enable a feature and bits that disable a feature.
> Are these bits defined in the DSI spec, or internal to DRM ? In the
> latter case, would it make sense to standardize on one "polarity" ? That
> would be a more intrusive change in drivers though.
Yes, that'd require auditing every single code path and reverse the
logic as needed. I'm not volunteering for that ,-P (hopefully the
current change is still an improvement).
Hopefully real DSI experts can comment (Andrzej?), I think the default
are sensible settings?
>
> > /* disable hfront-porch area */
> > -#define MIPI_DSI_MODE_VIDEO_HFP BIT(5)
> > +#define MIPI_DSI_MODE_VIDEO_NO_HFP BIT(5)
> > /* disable hback-porch area */
> > -#define MIPI_DSI_MODE_VIDEO_HBP BIT(6)
> > +#define MIPI_DSI_MODE_VIDEO_NO_HBP BIT(6)
> > /* disable hsync-active area */
> > -#define MIPI_DSI_MODE_VIDEO_HSA BIT(7)
> > +#define MIPI_DSI_MODE_VIDEO_NO_HSA BIT(7)
> > /* flush display FIFO on vsync pulse */
> > #define MIPI_DSI_MODE_VSYNC_FLUSHBIT(8)
> > /* disable EoT packets in HS mode */
> > -#define MIPI_DSI_MODE_EOT_PACKET BIT(9)
> > +#define MIPI_DSI_MODE_NO_EOT_PACKET BIT(9)
> > /* device supports non-continuous clock behavior (DSI spec 5.6.1) */
> > #define MIPI_DSI_CLOCK_NON_CONTINUOUSBIT(10)
> > /* transmit data in low power */
>
> --
> Regards,
>
> Laurent Pinchart
Re: [PATCH v7] vfs: fix copy_file_range regression in cross-fs copies
On Mon, Feb 22, 2021 at 3:57 AM Luis Henriques wrote:
>
> A regression has been reported by Nicolas Boichat, found while using the
> copy_file_range syscall to copy a tracefs file. Before commit
> 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the
> kernel would return -EXDEV to userspace when trying to copy a file across
> different filesystems. After this commit, the syscall doesn't fail anymore
> and instead returns zero (zero bytes copied), as this file's content is
> generated on-the-fly and thus reports a size of zero.
>
> This patch restores some cross-filesystem copy restrictions that existed
> prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
> devices"). Filesystems are still allowed to fall-back to the VFS
> generic_copy_file_range() implementation, but that has now to be done
> explicitly.
>
> nfsd is also modified to fall-back into generic_copy_file_range() in case
> vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV.
>
> Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices")
> Link:
> https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/
> Link:
> https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/
> Link:
> https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/
> Reported-by: Nicolas Boichat
Tested-by: Nicolas Boichat
> Signed-off-by: Luis Henriques
> ---
> Changes since v6
> - restored i_sb checks for the clone operation
> Changes since v5
> - check if ->copy_file_range is NULL before calling it
> Changes since v4
> - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP
> or -EXDEV.
> Changes since v3
> - dropped the COPY_FILE_SPLICE flag
> - kept the f_op's checks early in generic_copy_file_checks, implementing
> Amir's suggestions
> - modified nfsd to use generic_copy_file_range()
> Changes since v2
> - do all the required checks earlier, in generic_copy_file_checks(),
> adding new checks for ->remap_file_range
> - new COPY_FILE_SPLICE flag
> - don't remove filesystem's fallback to generic_copy_file_range()
> - updated commit changelog (and subject)
> Changes since v1 (after Amir review)
> - restored do_copy_file_range() helper
> - return -EOPNOTSUPP if fs doesn't implement CFR
> - updated commit description
>
> fs/nfsd/vfs.c | 8 +++-
> fs/read_write.c | 50 -
> 2 files changed, 32 insertions(+), 26 deletions(-)
> [snip]
Re: [PATCH v3] vfs: fix copy_file_range regression in cross-fs copies
On Thu, Feb 18, 2021 at 1:25 AM Luis Henriques wrote:
>
> A regression has been reported by Nicolas Boichat, found while using the
> copy_file_range syscall to copy a tracefs file. Before commit
> 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the
> kernel would return -EXDEV to userspace when trying to copy a file across
> different filesystems. After this commit, the syscall doesn't fail anymore
> and instead returns zero (zero bytes copied), as this file's content is
> generated on-the-fly and thus reports a size of zero.
>
> This patch restores some cross-filesystems copy restrictions that existed
> prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
> devices").
Note that you also fix intra-filesystem copy_file_range on these
generated filesystems. This is IMHO great, but needs to be mentioned
in the commit message.
> It also introduces a flag (COPY_FILE_SPLICE) that can be used
> by filesystems calling directly into the vfs copy_file_range to override
> these restrictions. Right now, only NFS needs to set this flag.
>
> Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices")
So technically this fixes something much older, presumably ever since
copy_file_range was introduced.
> Link:
> https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/
> Link:
> https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/
> Link:
> https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/
> Reported-by: Nicolas Boichat
Tested-by: Nicolas Boichat
but I guess you should not add to the next revision, I'll keep testing
further revisions ,-)
> Signed-off-by: Luis Henriques
> ---
> Ok, I've tried to address all the issues and comments. Hopefully this v3
> is a bit closer to the final fix.
>
> Changes since v2
> - do all the required checks earlier, in generic_copy_file_checks(),
> adding new checks for ->remap_file_range
> - new COPY_FILE_SPLICE flag
> - don't remove filesystem's fallback to generic_copy_file_range()
> - updated commit changelog (and subject)
> Changes since v1 (after Amir review)
> - restored do_copy_file_range() helper
> - return -EOPNOTSUPP if fs doesn't implement CFR
> - updated commit description
>
> fs/nfsd/vfs.c | 3 ++-
> fs/read_write.c| 44 +---
> include/linux/fs.h | 7 +++
> 3 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 04937e51de56..14e55822c223 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -578,7 +578,8 @@ ssize_t nfsd_copy_file_range(struct file *src, u64
> src_pos, struct file *dst,
> * limit like this and pipeline multiple COPY requests.
> */
> count = min_t(u64, count, 1 << 22);
> - return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0);
> + return vfs_copy_file_range(src, src_pos, dst, dst_pos, count,
> + COPY_FILE_SPLICE);
> }
>
> __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp,
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 75f764b43418..40a16003fb05 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -1410,6 +1410,33 @@ static ssize_t do_copy_file_range(struct file
> *file_in, loff_t pos_in,
>flags);
> }
>
> +/*
> + * This helper function checks whether copy_file_range can actually be used,
> + * depending on the source and destination filesystems being the same.
> + *
> + * In-kernel callers may set COPY_FILE_SPLICE to override these checks.
> + */
> +static int fops_copy_file_checks(struct file *file_in, struct file *file_out,
fops_copy_file_range_checks ?
> +unsigned int flags)
> +{
> + if (WARN_ON_ONCE(flags & ~COPY_FILE_SPLICE))
> + return -EINVAL;
> +
> + if (flags & COPY_FILE_SPLICE)
> + return 0;
> + /*
> +* We got here from userspace, so forbid copies if copy_file_range
> isn't
> +* implemented or if we're doing a cross-fs copy.
> +*/
> + if (!file_out->f_op->copy_file_range)
> + return -EOPNOTSUPP;
After this is merged, should this be added as an error code to the man page?
> + else if (file_out->f_op->copy_file_range !=
> +file_in->f_op->copy_file_range)
Just note, this could be a cross-fs copy (just not a cross-fs_type copy).
> + return -EXDEV;
> +
> +
Re: [PATCH v2] vfs: prevent copy_file_range to copy across devices
On Mon, Feb 15, 2021 at 11:42 PM Luis Henriques wrote:
>
> Nicolas Boichat reported an issue when trying to use the copy_file_range
> syscall on a tracefs file. It failed silently because the file content is
> generated on-the-fly (reporting a size of zero) and copy_file_range needs
> to know in advance how much data is present.
Not sure if you have the whole history, these links and discussion can
help if you want to expand on the commit message:
[1] http://issuetracker.google.com/issues/178332739
[2] https://lkml.org/lkml/2021/1/25/64
[3] https://lkml.org/lkml/2021/1/26/1736
[4]
https://patchwork.kernel.org/project/linux-fsdevel/cover/20210212044405.4120619-1-drink...@chromium.org/
> This commit restores the cross-fs restrictions that existed prior to
> 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") and
> removes generic_copy_file_range() calls from ceph, cifs, fuse, and nfs.
It goes beyond that, I think this also prevents copies within the same
FS if copy_file_range is not implemented. Which is IMHO a good thing
since this has been broken on procfs and friends ever since
copy_file_range was implemented (but I assume that nobody ever hit
that before cross-fs became available).
>
> Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices")
> Link:
> https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/
> Cc: Nicolas Boichat
You could replace that with Reported-by: Nicolas Boichat
> Signed-off-by: Luis Henriques
> ---
> Changes since v1 (after Amir review)
> - restored do_copy_file_range() helper
> - return -EOPNOTSUPP if fs doesn't implement CFR
> - updated commit description
>
> fs/ceph/file.c | 21 +++-
> fs/cifs/cifsfs.c | 3 ---
> fs/fuse/file.c | 21 +++-
> fs/nfs/nfs4file.c | 20 +++
> fs/read_write.c| 49 ++
> include/linux/fs.h | 3 ---
> 6 files changed, 19 insertions(+), 98 deletions(-)
>
[snip]
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 75f764b43418..b217cd62ae0d 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -1358,40 +1358,12 @@ COMPAT_SYSCALL_DEFINE4(sendfile64, int, out_fd, int,
> in_fd,
> }
> #endif
>
> -/**
> - * generic_copy_file_range - copy data between two files
> - * @file_in: file structure to read from
> - * @pos_in:file offset to read from
> - * @file_out: file structure to write data to
> - * @pos_out: file offset to write data to
> - * @len: amount of data to copy
> - * @flags: copy flags
> - *
> - * This is a generic filesystem helper to copy data from one file to another.
> - * It has no constraints on the source or destination file owners - the files
> - * can belong to different superblocks and different filesystem types. Short
> - * copies are allowed.
> - *
> - * This should be called from the @file_out filesystem, as per the
> - * ->copy_file_range() method.
> - *
> - * Returns the number of bytes copied or a negative error indicating the
> - * failure.
> - */
> -
> -ssize_t generic_copy_file_range(struct file *file_in, loff_t pos_in,
> - struct file *file_out, loff_t pos_out,
> - size_t len, unsigned int flags)
> -{
> - return do_splice_direct(file_in, _in, file_out, _out,
> - len > MAX_RW_COUNT ? MAX_RW_COUNT : len, 0);
> -}
> -EXPORT_SYMBOL(generic_copy_file_range);
> -
> static ssize_t do_copy_file_range(struct file *file_in, loff_t pos_in,
> struct file *file_out, loff_t pos_out,
> size_t len, unsigned int flags)
> {
> + ssize_t ret = -EXDEV;
> +
> /*
> * Although we now allow filesystems to handle cross sb copy, passing
> * a file of the wrong filesystem type to filesystem driver can result
> @@ -1400,14 +1372,14 @@ static ssize_t do_copy_file_range(struct file
> *file_in, loff_t pos_in,
> * several different file_system_type structures, but they all end up
> * using the same ->copy_file_range() function pointer.
> */
> - if (file_out->f_op->copy_file_range &&
> - file_out->f_op->copy_file_range == file_in->f_op->copy_file_range)
> - return file_out->f_op->copy_file_range(file_in, pos_in,
> - file_out, pos_out,
> - len, flags);
> + if (!file_out->f_op->copy_file_range)
> + ret = -EOPNOTSUPP;
This doesn't work as the
Re: [PATCH 1/6] fs: Add flag to file_system_type to indicate content is generated
On Mon, Feb 15, 2021 at 9:12 AM Ian Lance Taylor wrote: > > On Sun, Feb 14, 2021 at 4:38 PM Dave Chinner wrote: > > > > On Fri, Feb 12, 2021 at 03:54:48PM -0800, Darrick J. Wong wrote: > > > On Sat, Feb 13, 2021 at 10:27:26AM +1100, Dave Chinner wrote: > > > > > > > If you can't tell from userspace that a file has data in it other > > > > than by calling read() on it, then you can't use cfr on it. > > > > > > I don't know how to do that, Dave. :) > > > > If stat returns a non-zero size, then userspace knows it has at > > least that much data in it, whether it be zeros or previously > > written data. cfr will copy that data. The special zero length > > regular pipe files fail this simple "how much data is there to copy > > in this file" check... > > This suggests that if the Go standard library sees that > copy_file_range reads zero bytes, we should assume that it failed, and > should use the fallback path as though copy_file_range returned > EOPNOTSUPP or EINVAL. This will cause an extra system call for an > empty file, but as long as copy_file_range does not return an error > for cases that it does not support we are going to need an extra > system call anyhow. > > Does that seem like a possible mitigation? That is, are there cases > where copy_file_range will fail to do a correct copy, and will return > success, and will not return zero? I'm a bit worried about the sysfs files that report a 4096 bytes file size, for 2 reasons: - I'm not sure if the content _can_ actually be longer (I couldn't find a counterexample) - If you're unlucky enough to have a partial write in the output filesystem, you'll get a non-zero return value and probably corrupted content. > > Ian
Re: [v2,2/3] thermal: mediatek: Add LVTS drivers for SoC theraml zones
On Fri, Jan 29, 2021 at 5:40 PM Michael Kao wrote:
>
> Add a LVTS (Low voltage thermal sensor) driver to report junction
> temperatures in Mediatek SoC and register the maximum temperature
> of sensors and each sensor as a thermal zone.
>
> Signed-off-by: Yu-Chia Chang
> Signed-off-by: Michael Kao
> ---
> drivers/thermal/mediatek/Kconfig | 10 +
> drivers/thermal/mediatek/Makefile|1 +
> drivers/thermal/mediatek/soc_temp_lvts.c | 1287 ++
> drivers/thermal/mediatek/soc_temp_lvts.h | 312 ++
> 4 files changed, 1610 insertions(+)
> create mode 100644 drivers/thermal/mediatek/soc_temp_lvts.c
> create mode 100644 drivers/thermal/mediatek/soc_temp_lvts.h
>
> [snip]
> diff --git a/drivers/thermal/mediatek/Makefile
> b/drivers/thermal/mediatek/Makefile
> index f75313ddce5e..16ce166e5916 100644
> --- a/drivers/thermal/mediatek/Makefile
> +++ b/drivers/thermal/mediatek/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_MTK_SOC_THERMAL) += soc_temp.o
> +obj-$(CONFIG_MTK_SOC_THERMAL_LVTS) += soc_temp_lvts.o
> diff --git a/drivers/thermal/mediatek/soc_temp_lvts.c
> b/drivers/thermal/mediatek/soc_temp_lvts.c
> new file mode 100644
> index ..b56c2cd3cb39
> --- /dev/null
> +++ b/drivers/thermal/mediatek/soc_temp_lvts.c
> @@ -0,0 +1,1287 @@
> [snip]
> +
> +static unsigned int lvts_temp_to_raw(struct formula_coeff *co, int temp)
> +{
> + unsigned int msr_raw;
> +
> + msr_raw = ((long long)((co->golden_temp * 500 + co->b - temp)) << 14)
> + / (-1 * co->a);
This fails to build test on arm (32-bit):
ERROR: "__aeabi_ldivmod" [drivers/thermal/mediatek/soc_temp_lvts.ko] undefined!
Recommend using div_s64:
msr_raw = div_s64((s64)((co->golden_temp * 500 + co->b - temp)) << 14,
-1 * co->a);
> +
> + return msr_raw;
> +}
> [snip]
Re: [PATCH v3 1/3] drm/mediatek: mtk_dpi: Add check for max clock rate in mode_valid
+Pi-Hsun Shih
On Mon, Feb 8, 2021 at 9:42 AM Jitao Shi wrote:
>
> Add per-platform max clock rate check in mtk_dpi_bridge_mode_valid.
>
> Signed-off-by: Jitao Shi
I believe this patch (and the following) were actually authored by
Pi-Hsun: https://crrev.com/c/2628812 . Would be best to keep the
author information (unless I'm missing something of course).
> ---
> drivers/gpu/drm/mediatek/mtk_dpi.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 52f11a63a330..ffa4a0f1989f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -118,6 +118,7 @@ struct mtk_dpi_yc_limit {
> struct mtk_dpi_conf {
> unsigned int (*cal_factor)(int clock);
> u32 reg_h_fre_con;
> + u32 max_clock_khz;
> bool edge_sel_en;
> };
>
> @@ -555,9 +556,22 @@ static void mtk_dpi_bridge_enable(struct drm_bridge
> *bridge)
> mtk_dpi_set_display_mode(dpi, >mode);
> }
>
> +static enum drm_mode_status
> +mtk_dpi_bridge_mode_valid(struct drm_bridge *bridge,
> + const struct drm_display_mode *mode)
> +{
> + struct mtk_dpi *dpi = bridge_to_dpi(bridge);
> +
> + if (dpi->conf->max_clock_khz && mode->clock >
> dpi->conf->max_clock_khz)
> + return MODE_CLOCK_HIGH;
> +
> + return MODE_OK;
> +}
> +
> static const struct drm_bridge_funcs mtk_dpi_bridge_funcs = {
> .attach = mtk_dpi_bridge_attach,
> .mode_set = mtk_dpi_bridge_mode_set,
> + .mode_valid = mtk_dpi_bridge_mode_valid,
> .disable = mtk_dpi_bridge_disable,
> .enable = mtk_dpi_bridge_enable,
> };
> @@ -673,17 +687,20 @@ static unsigned int mt8183_calculate_factor(int clock)
> static const struct mtk_dpi_conf mt8173_conf = {
> .cal_factor = mt8173_calculate_factor,
> .reg_h_fre_con = 0xe0,
> + .max_clock_khz = 30,
> };
>
> static const struct mtk_dpi_conf mt2701_conf = {
> .cal_factor = mt2701_calculate_factor,
> .reg_h_fre_con = 0xb0,
> .edge_sel_en = true,
> + .max_clock_khz = 15,
> };
>
> static const struct mtk_dpi_conf mt8183_conf = {
> .cal_factor = mt8183_calculate_factor,
> .reg_h_fre_con = 0xe0,
> + .max_clock_khz = 10,
> };
>
> static int mtk_dpi_probe(struct platform_device *pdev)
> --
> 2.25.1
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] fs: Add flag to file_system_type to indicate content is generated
On Fri, Feb 12, 2021 at 3:46 PM Greg KH wrote:
>
> On Fri, Feb 12, 2021 at 12:44:00PM +0800, Nicolas Boichat wrote:
> > Filesystems such as procfs and sysfs generate their content at
> > runtime. This implies the file sizes do not usually match the
> > amount of data that can be read from the file, and that seeking
> > may not work as intended.
> >
> > This will be useful to disallow copy_file_range with input files
> > from such filesystems.
> >
> > Signed-off-by: Nicolas Boichat
> > ---
> > I first thought of adding a new field to struct file_operations,
> > but that doesn't quite scale as every single file creation
> > operation would need to be modified.
>
> Even so, you missed a load of filesystems in the kernel with this patch
> series, what makes the ones you did mark here different from the
> "internal" filesystems that you did not?
Which ones did I miss? (apart from configfs, see the conversation on
patch 6/6). Anyway, hopefully auditing all filesystems is an order of
magnitude easier task, and easier to maintain in the longer run ,-)
> This feels wrong, why is userspace suddenly breaking? What changed in
> the kernel that caused this? Procfs has been around for a _very_ long
> time :)
Some of this is covered in the cover letter 0/6. To expand a bit:
copy_file_range has only supported cross-filesystem copies since 5.3
[1], before that the kernel would return EXDEV and userspace
application would fall back to a read/write based copy. After 5.3,
copy_file_range copies no data as it thinks the input file is empty.
[1] I think it makes little sense to try to use copy_file_range
between 2 files in /proc, but technically, I think that has been
broken since copy_file_range fallback to do_splice_direct was
introduced (eac70053a141 ("vfs: Add vfs_copy_file_range() support for
pagecache copies", ~4.4).
>
> thanks,
>
> greg k-h
Re: [PATCH 6/6] vfs: Disallow copy_file_range on generated file systems
On Fri, Feb 12, 2021 at 12:59 PM Darrick J. Wong wrote: > > On Thu, Feb 11, 2021 at 08:53:47PM -0800, Darrick J. Wong wrote: > > On Fri, Feb 12, 2021 at 12:44:05PM +0800, Nicolas Boichat wrote: > > > copy_file_range (which calls generic_copy_file_checks) uses the > > > inode file size to adjust the copy count parameter. This breaks > > > with special filesystems like procfs/sysfs/debugfs/tracefs, where > > > the file size appears to be zero, but content is actually returned > > > when a read operation is performed. Other issues would also > > > happen on partial writes, as the function would attempt to seek > > > in the input file. > > > > > > Use the newly introduced FS_GENERATED_CONTENT filesystem flag > > > to return -EOPNOTSUPP: applications can then retry with a more > > > usual read/write based file copy (the fallback code is usually > > > already present to handle older kernels). > > > > > > Signed-off-by: Nicolas Boichat > > > --- > > > > > > fs/read_write.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/fs/read_write.c b/fs/read_write.c > > > index 0029ff2b0ca8..80322e89fb0a 100644 > > > --- a/fs/read_write.c > > > +++ b/fs/read_write.c > > > @@ -1485,6 +1485,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, > > > loff_t pos_in, > > > if (flags != 0) > > > return -EINVAL; > > > > > > + if (file_inode(file_in)->i_sb->s_type->fs_flags & > > > FS_GENERATED_CONTENT) > > > + return -EOPNOTSUPP; > > > > Why not declare a dummy copy_file_range_nop function that returns > > EOPNOTSUPP and point all of these filesystems at it? > > > > (Or, I guess in these days where function pointers are the enemy, > > create a #define that is a cast of 0x1, and fix do_copy_file_range to > > return EOPNOTSUPP if it sees that?) I was pondering abusing ERR_PTR(-EOPNOTSUPP) for this purpose ,-P > > Oh, I see, because that doesn't help if the source file is procfs and > the dest file is (say) xfs, because the generic version will try to do > splice magic and *poof*. Yep. I mean, we could still add a check if the file_in->f_op->copy_file_range == copy_file_range_nop in do_copy_file_range... But then we'd need to sprinkle .copy_file_range = copy_file_range_nop in many many places (~700 as a lower bound[1]), since the file operation structure is defined at the file level, not at the FS level, and people are likely to forget... [1] $ git grep "struct file_operations.*=" | grep debug | wc -l 631 $ git grep "struct file_operations.*=" | grep trace | wc -l 84 > > I guess the other nit thatI can think of at this late hour is ... what > about the other virtual filesystems like configfs and whatnot? Should > we have a way to flag them as "this can't be the source of a CFR > request" as well? > > Or is it just trace/debug/proc/sysfs that have these "zero size but > readable" speshul behaviors? I did try to audit the other filesystems. The ones I spotted: - devpts should be fine (only device nodes in there) - I think pstore doesn't need the flag as it's RAM-backed and persistent. But yes, I missed configfs, thanks for catching that. I think we need to add the flag for that one (looks like the sizes are all 4K). > > --D > > > > > --D > > > > > + > > > ret = generic_copy_file_checks(file_in, pos_in, file_out, pos_out, > > > , > > >flags); > > > if (unlikely(ret)) > > > -- > > > 2.30.0.478.g8a0d178c01-goog > > >
Re: [PATCH] fs: generic_copy_file_checks: Do not adjust count based on file size
On Thu, Jan 28, 2021 at 1:57 PM Darrick J. Wong wrote: > > On Thu, Jan 28, 2021 at 08:46:04AM +0800, Nicolas Boichat wrote: [snip] > > Okay, so, based on this and Al's reply, I see 2 things we can do: > > 1. Go should probably not use copy_file_range in a common library > > function, as I don't see any easy way to detect this scenario > > currently (detect 0 size? sure, but that won't work with the example > > you provide above). And the man page should document this behaviour > > more explicitly to prevent further incorrect usage. > > 2. Can procfs/sysfs/debugfs and friends explicitly prevent usage of > > copy_file_range? (based on Al's reply, there seems to be no way to > > implement it correctly as seeking in such files will not work in case > > of short writes) > > One /could/ make those three provide a phony CFR implementation that > would return -EOPNOTSUPP, though like others have said, it's weird to > have regular files that aren't quite regular. Not sure where that > leaves them, though... Not that simple, as the issue happens on cross-filesystem operations where file_operations->copy_file_range is not called (and also, that'd require modifying operations for every single generated file... Anyway, made an attempt here: https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/T/#t > --D > > > > > Thanks, > > > > > > > > Cheers, > > > > > > Dave. > > > -- > > > Dave Chinner > > > da...@fromorbit.com
[PATCH 6/6] vfs: Disallow copy_file_range on generated file systems
copy_file_range (which calls generic_copy_file_checks) uses the inode file size to adjust the copy count parameter. This breaks with special filesystems like procfs/sysfs/debugfs/tracefs, where the file size appears to be zero, but content is actually returned when a read operation is performed. Other issues would also happen on partial writes, as the function would attempt to seek in the input file. Use the newly introduced FS_GENERATED_CONTENT filesystem flag to return -EOPNOTSUPP: applications can then retry with a more usual read/write based file copy (the fallback code is usually already present to handle older kernels). Signed-off-by: Nicolas Boichat --- fs/read_write.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/read_write.c b/fs/read_write.c index 0029ff2b0ca8..80322e89fb0a 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1485,6 +1485,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, if (flags != 0) return -EINVAL; + if (file_inode(file_in)->i_sb->s_type->fs_flags & FS_GENERATED_CONTENT) + return -EOPNOTSUPP; + ret = generic_copy_file_checks(file_in, pos_in, file_out, pos_out, , flags); if (unlikely(ret)) -- 2.30.0.478.g8a0d178c01-goog
[PATCH 1/6] fs: Add flag to file_system_type to indicate content is generated
Filesystems such as procfs and sysfs generate their content at
runtime. This implies the file sizes do not usually match the
amount of data that can be read from the file, and that seeking
may not work as intended.
This will be useful to disallow copy_file_range with input files
from such filesystems.
Signed-off-by: Nicolas Boichat
---
I first thought of adding a new field to struct file_operations,
but that doesn't quite scale as every single file creation
operation would need to be modified.
include/linux/fs.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3482146b11b0..5bd58b928e94 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2335,6 +2335,7 @@ struct file_system_type {
#define FS_ALLOW_IDMAP 32 /* FS has been updated to handle vfs
idmappings. */
#define FS_THP_SUPPORT 8192/* Remove once all fs converted */
#define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during
rename() internally. */
+#define FS_GENERATED_CONTENT 65536 /* FS contains generated content */
int (*init_fs_context)(struct fs_context *);
const struct fs_parameter_spec *parameters;
struct dentry *(*mount) (struct file_system_type *, int,
--
2.30.0.478.g8a0d178c01-goog
[PATCH 2/6] proc: Add FS_GENERATED_CONTENT to filesystem flags
procfs content is generated at runtime.
Signed-off-by: Nicolas Boichat
---
fs/proc/root.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/proc/root.c b/fs/proc/root.c
index c7e3b1350ef8..7ed715a0f807 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -282,7 +282,7 @@ static struct file_system_type proc_fs_type = {
.init_fs_context= proc_init_fs_context,
.parameters = proc_fs_parameters,
.kill_sb= proc_kill_sb,
- .fs_flags = FS_USERNS_MOUNT | FS_DISALLOW_NOTIFY_PERM,
+ .fs_flags = FS_USERNS_MOUNT | FS_DISALLOW_NOTIFY_PERM |
FS_GENERATED_CONTENT,
};
void __init proc_root_init(void)
--
2.30.0.478.g8a0d178c01-goog
[PATCH 5/6] tracefs: Add FS_GENERATED_CONTENT to filesystem flags
tracefs content is generated at runtime.
Signed-off-by: Nicolas Boichat
---
fs/tracefs/inode.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 4b83cbded559..89980312c7a3 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -308,6 +308,7 @@ static struct file_system_type trace_fs_type = {
.name = "tracefs",
.mount =trace_mount,
.kill_sb = kill_litter_super,
+ .fs_flags = FS_GENERATED_CONTENT,
};
MODULE_ALIAS_FS("tracefs");
--
2.30.0.478.g8a0d178c01-goog
[PATCH 4/6] debugfs: Add FS_GENERATED_CONTENT to filesystem flags
debugfs content is generated at runtime.
Signed-off-by: Nicolas Boichat
---
fs/debugfs/inode.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index c35249497b9b..2bbc5e6d3041 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -279,6 +279,7 @@ static struct file_system_type debug_fs_type = {
.name = "debugfs",
.mount =debug_mount,
.kill_sb = kill_litter_super,
+ .fs_flags = FS_GENERATED_CONTENT,
};
MODULE_ALIAS_FS("debugfs");
--
2.30.0.478.g8a0d178c01-goog
[PATCH 3/6] sysfs: Add FS_GENERATED_CONTENT to filesystem flags
sysfs content is generated at runtime.
Signed-off-by: Nicolas Boichat
---
fs/sysfs/mount.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index e747c135c1d1..7e367ae5edc1 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -91,7 +91,7 @@ static struct file_system_type sysfs_fs_type = {
.name = "sysfs",
.init_fs_context= sysfs_init_fs_context,
.kill_sb= sysfs_kill_sb,
- .fs_flags = FS_USERNS_MOUNT,
+ .fs_flags = FS_USERNS_MOUNT | FS_GENERATED_CONTENT,
};
int __init sysfs_init(void)
--
2.30.0.478.g8a0d178c01-goog
[PATCH 0/6] Add generated flag to filesystem struct to block copy_file_range
We hit an issue when upgrading Go compiler from 1.13 to 1.15 [1], as we use Go's `io.Copy` to copy the content of `/sys/kernel/debug/tracing/trace` to a temporary file. Under the hood, Go 1.15 uses `copy_file_range` syscall to optimize the copy operation. However, that fails to copy any content when the input file is from tracefs, with an apparent size of 0 (but there is still content when you `cat` it, of course). >From discussions in [2][3], it is clear that copy_file_range cannot be properly implemented on filesystems where the content is generated at runtime: the file size is incorrect (because it is unknown before the content is generated), and seeking in such files (as required by partial writes) is unlikely to work correctly. With this patch, Go's `io.Copy` gracefully falls back to a normal read/write file copy. I'm not 100% sure which stable tree this should go in, I'd say at least >=5.3 since this is what introduced support for cross-filesystem copy_file_range (and where most users are somewhat likely to hit this issue). But let's discuss the patch series first. [1] http://issuetracker.google.com/issues/178332739 [2] https://lkml.org/lkml/2021/1/25/64 [3] https://lkml.org/lkml/2021/1/26/1736 Nicolas Boichat (6): fs: Add flag to file_system_type to indicate content is generated proc: Add FS_GENERATED_CONTENT to filesystem flags sysfs: Add FS_GENERATED_CONTENT to filesystem flags debugfs: Add FS_GENERATED_CONTENT to filesystem flags tracefs: Add FS_GENERATED_CONTENT to filesystem flags vfs: Disallow copy_file_range on generated file systems fs/debugfs/inode.c | 1 + fs/proc/root.c | 2 +- fs/read_write.c| 3 +++ fs/sysfs/mount.c | 2 +- fs/tracefs/inode.c | 1 + include/linux/fs.h | 1 + 6 files changed, 8 insertions(+), 2 deletions(-) -- 2.30.0.478.g8a0d178c01-goog
[PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
Many of the DSI flags have names opposite to their actual effects,
e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually
be disabled. Fix this by including _NO_ in the flag names, e.g.
MIPI_DSI_MODE_NO_EOT_PACKET.
Signed-off-by: Nicolas Boichat
---
I considered adding _DISABLE_ instead, but that'd make the
flag names a big too long.
Generated with:
flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \
xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {}
flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \
xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {}
flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \
xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {}
flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \
xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {}
(then minor format changes)
drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +-
drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +-
drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++--
drivers/gpu/drm/bridge/tc358768.c| 2 +-
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8
drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
drivers/gpu/drm/msm/dsi/dsi_host.c | 8
drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +-
drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +-
drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +-
drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +-
drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +-
drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +-
drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +-
drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +-
drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +-
drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +-
drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c| 2 +-
drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++--
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +-
drivers/gpu/drm/panel/panel-simple.c | 2 +-
drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +-
drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +-
include/drm/drm_mipi_dsi.h | 8
25 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index aa19d5a40e31..59d718bde8c4 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -165,7 +165,7 @@ int adv7533_attach_dsi(struct adv7511 *adv)
dsi->lanes = adv->num_dsi_lanes;
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
- MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
+ MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
ret = mipi_dsi_attach(dsi);
if (ret < 0) {
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 65cc05982f82..beecfe6bf359 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1334,7 +1334,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx)
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
- MIPI_DSI_MODE_EOT_PACKET|
+ MIPI_DSI_MODE_NO_EOT_PACKET |
MIPI_DSI_MODE_VIDEO_HSE;
if (mipi_dsi_attach(dsi) < 0) {
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
b/drivers/gpu/drm/bridge/cdns-dsi.c
index 76373e31df92..34aa24269a57 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -829,7 +829,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge
*bridge)
tmp = DIV_ROUND_UP(dsi_cfg.htotal, nlanes) -
DIV_ROUND_UP(dsi_cfg.hsa, nlanes);
- if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET))
+ if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
tmp -= DIV_ROUND_UP(DSI_EOT_PKT_SIZE, nlanes);
tx_byte_period = DIV_ROUND_DOWN_ULL((u64)NSEC_PER_SEC * 8,
@@ -902,7 +902,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge
*bridge)
tmp = readl(dsi->regs + MCTL_MAIN_DATA_CTL);
tmp &= ~(IF_VID_SELECT_MASK | HOST_EOT_GEN | IF_VID_MODE);
- if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET))
+ if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
tmp |= HOST_EOT_GEN;
i
Re: [PATCH v4 03/22] media: camss: Replace trace_printk() with dev_dbg()
On Fri, Feb 5, 2021 at 6:44 PM Robert Foss wrote:
>
> trace_printk() should not be used in production code,
> since extra memory is used for special buffers whenever
> trace_puts() is used.
>
> Replace it with dev_dbg() which provides all of the desired
> debugging functionality.
>
> Signed-off-by: Robert Foss
> Suggested-by: Nicolas Boichat
Thanks!
Reviewed-by: Nicolas Boichat
> ---
>
> Changes since v3:
> - Nicolas: Create this patch
>
>
> drivers/media/platform/qcom/camss/camss-vfe-4-1.c | 5 +++--
> drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 5 +++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
> b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
> index a1b56b89130d..85b9bcbc7321 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
> @@ -12,6 +12,7 @@
> #include
> #include
>
> +#include "camss.h"
> #include "camss-vfe.h"
>
> #define VFE_0_HW_VERSION 0x000
> @@ -936,8 +937,8 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>
> vfe->ops->isr_read(vfe, , );
>
> - trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n",
> -value0, value1);
> + dev_dbg(vfe->camss->dev, "VFE: status0 = 0x%08x, status1 = 0x%08x\n",
> + value0, value1);
>
> if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK)
> vfe->isr_ops.reset_ack(vfe);
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
> b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
> index 84c33b8f9fe3..f7e00a2de393 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
> @@ -12,6 +12,7 @@
> #include
> #include
>
> +#include "camss.h"
> #include "camss-vfe.h"
>
> #define VFE_0_HW_VERSION 0x000
> @@ -1069,8 +1070,8 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>
> vfe->ops->isr_read(vfe, , );
>
> - trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n",
> -value0, value1);
> + dev_dbg(vfe->camss->dev, "VFE: status0 = 0x%08x, status1 = 0x%08x\n",
> + value0, value1);
>
> if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK)
> vfe->isr_ops.reset_ack(vfe);
> --
> 2.27.0
>
Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
On Sat, Feb 6, 2021 at 1:55 AM Rob Herring wrote: > > On Tue, 26 Jan 2021 09:17:56 +0800, Nicolas Boichat wrote: > > Define a compatible string for the Mali Bifrost GPU found in > > Mediatek's MT8183 SoCs. > > > > Signed-off-by: Nicolas Boichat > > --- > > > > Changes in v11: > > - binding: power-domain-names not power-domainS-names > > > > Changes in v10: > > - Fix the binding to make sure sram-supply property can be provided. > > > > Changes in v9: None > > Changes in v8: None > > Changes in v7: None > > Changes in v6: > > - Rebased, actually tested with recent mesa driver. > > > > Changes in v5: > > - Rename "2d" power domain to "core2" > > > > Changes in v4: > > - Add power-domain-names description > >(kept Alyssa's reviewed-by as the change is minor) > > > > Changes in v3: None > > Changes in v2: None > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 28 +++ > > 1 file changed, 28 insertions(+) > > > > > Please add Acked-by/Reviewed-by tags when posting new versions. However, > there's no need to repost patches *only* to add the tags. The upstream > maintainer will do that for acks received on the version they apply. > > If a tag was not added on purpose, please state why and what changed. There were changes in v11, I thought you'd want to review again? Anyway, I can resend a v12 with all the Rb/Ab if that works better for you. >
Re: [PATCH] drm/bridge: anx7625: enable DSI EOTP
On Thu, Feb 4, 2021 at 8:59 PM Andrzej Hajda wrote:
>
>
> W dniu 04.02.2021 o 13:34, Nicolas Boichat pisze:
> > On Thu, Feb 4, 2021 at 8:07 PM Robert Foss wrote:
> >> Hi Xin,
> >>
> >> Thanks for the patch.
> >>
> >> On Thu, 28 Jan 2021 at 12:17, Xin Ji wrote:
> >>> Enable DSI EOTP feature for fixing some panel screen constance
> >>> shift issue.
> >>> Removing MIPI flag MIPI_DSI_MODE_EOT_PACKET to enable DSI EOTP.
> >> I don't think I quite understand how removing the
> >> MIPI_DSI_MODE_EOT_PACKET flag will cause DSI EOTP to be enabled. Could
> >> you extrapolate on this in the commit message?
> > That confused me as well, but it turns out that's how the flag is defined:
> > ```
> > /* disable EoT packets in HS mode */
> > #define MIPI_DSI_MODE_EOT_PACKET BIT(9)
> > ```
> > (https://protect2.fireeye.com/v1/url?k=5bd95ebd-044267fb-5bd8d5f2-0cc47a3003e8-ce9db8ea264d6901=1=900556dc-d199-4c18-9432-5c3465a98eae=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Finclude%2Fdrm%2Fdrm_mipi_dsi.h%23L129)
> >
> > I'm almost tempted to put together a mass patch to rename all of these
> > flags...
>
>
> Yes that would be good, many of these flags were just copy pasted from
> some hw datasheet, without good analysis how to adapt them to the framework.
I'll look into it (but that shouldn't block this patch).
>
>
> Regards
>
> Andrzej
>
>
> >
> >>> Signed-off-by: Xin Ji
> >>> ---
> >>> drivers/gpu/drm/bridge/analogix/anx7625.c | 1 -
> >>> 1 file changed, 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>> index 65cc059..e31eeb1b 100644
> >>> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>> @@ -1334,7 +1334,6 @@ static int anx7625_attach_dsi(struct anx7625_data
> >>> *ctx)
> >>> dsi->format = MIPI_DSI_FMT_RGB888;
> >>> dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> >>> MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> >>> - MIPI_DSI_MODE_EOT_PACKET|
> >>> MIPI_DSI_MODE_VIDEO_HSE;
> >>>
> >>> if (mipi_dsi_attach(dsi) < 0) {
> >>> --
> >>> 2.7.4
> >>>
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://protect2.fireeye.com/v1/url?k=457f3f39-1ae4067f-457eb476-0cc47a3003e8-b702072da729d8c9=1=900556dc-d199-4c18-9432-5c3465a98eae=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
> >
Re: [PATCH] drm/bridge: anx7625: enable DSI EOTP
On Thu, Feb 4, 2021 at 8:07 PM Robert Foss wrote:
>
> Hi Xin,
>
> Thanks for the patch.
>
> On Thu, 28 Jan 2021 at 12:17, Xin Ji wrote:
> >
> > Enable DSI EOTP feature for fixing some panel screen constance
> > shift issue.
> > Removing MIPI flag MIPI_DSI_MODE_EOT_PACKET to enable DSI EOTP.
>
> I don't think I quite understand how removing the
> MIPI_DSI_MODE_EOT_PACKET flag will cause DSI EOTP to be enabled. Could
> you extrapolate on this in the commit message?
That confused me as well, but it turns out that's how the flag is defined:
```
/* disable EoT packets in HS mode */
#define MIPI_DSI_MODE_EOT_PACKET BIT(9)
```
(https://elixir.bootlin.com/linux/latest/source/include/drm/drm_mipi_dsi.h#L129)
I'm almost tempted to put together a mass patch to rename all of these flags...
>
> >
> > Signed-off-by: Xin Ji
> > ---
> > drivers/gpu/drm/bridge/analogix/anx7625.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index 65cc059..e31eeb1b 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > @@ -1334,7 +1334,6 @@ static int anx7625_attach_dsi(struct anx7625_data
> > *ctx)
> > dsi->format = MIPI_DSI_FMT_RGB888;
> > dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> > MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> > - MIPI_DSI_MODE_EOT_PACKET|
> > MIPI_DSI_MODE_VIDEO_HSE;
> >
> > if (mipi_dsi_attach(dsi) < 0) {
> > --
> > 2.7.4
> >
Re: [BUG] copy_file_range with sysfs file as input
On Wed, Feb 3, 2021 at 5:04 PM Greg KH wrote: > > On Mon, Jan 25, 2021 at 03:54:31PM +0800, Nicolas Boichat wrote: > > Hi copy_file_range experts, > > > > We hit this interesting issue when upgrading Go compiler from 1.13 to > > 1.15 [1]. Basically we use Go's `io.Copy` to copy the content of > > `/sys/kernel/debug/tracing/trace` to a temporary file. > > Nit, the above file is NOT a sysfs file. Odds are it is either a > debugfs, or a tracefs file, please check your mounts to determine which > it is, as that matters a lot on the kernel backend side for trying to > figure out what is going on here :) Yes yes it's tracefs ,-) But, from the other thread https://lkml.org/lkml/2021/1/26/2029 sysfs (and any other fs that generates files dynamically like procfs) would likely hit issues as well (but maybe in more rare circumstances). Thanks! > > thanks, > > greg k-h
Re: [PATCH] drm/mediatek: fine tune the data lane trail by project dts
On Mon, Feb 1, 2021 at 11:48 AM Jitao Shi wrote:
>
> Some panels or bridges require customized hs_da_trail time.
> So add a property in devicetree for this panels and bridges.
Since this changes the device tree, you also need to upload a binding
document change.
>
> Signed-off-by: Jitao Shi
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 8c70ec39bfe1..6e7092fa2fee 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -194,6 +194,7 @@ struct mtk_dsi {
> struct clk *hs_clk;
>
> u32 data_rate;
> + u32 da_trail_delta;
>
> unsigned long mode_flags;
> enum mipi_dsi_pixel_format format;
> @@ -234,7 +235,7 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
> timing->da_hs_prepare = (80 * data_rate_mhz + 4 * 1000) / 8000;
> timing->da_hs_zero = (170 * data_rate_mhz + 10 * 1000) / 8000 + 1 -
> timing->da_hs_prepare;
> - timing->da_hs_trail = timing->da_hs_prepare + 1;
> + timing->da_hs_trail = timing->da_hs_prepare + 1 + dsi->da_trail_delta;
>
> timing->ta_go = 4 * timing->lpx - 2;
> timing->ta_sure = timing->lpx + 2;
> @@ -1094,6 +1095,13 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> goto err_unregister_host;
> }
>
> + ret = of_property_read_u32_index(dev->of_node, "da_trail_delta", 0,
> +>da_trail_delta);
> + if (ret) {
> + dev_info(dev, "Can't get da_trail_delta, keep it as 0: %d\n",
> ret);
> + dsi->da_trail_delta = 0;
> + }
> +
> comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DSI);
> if (comp_id < 0) {
> dev_err(dev, "Failed to identify by alias: %d\n", comp_id);
> --
> 2.12.5
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 6/8] regulator: mt6359: Add support for MT6359 regulator
On Fri, Jan 22, 2021 at 7:27 PM Hsin-Hsiung Wang
wrote:
>
> From: Wen Su
>
> The MT6359 is a regulator found on boards based on MediaTek MT6779 and
> probably other SoCs. It is a so called pmic and connects as a slave to
> SoC using SPI, wrapped inside the pmic-wrapper.
>
> Signed-off-by: Wen Su
> Signed-off-by: Hsin-Hsiung Wang
> ---
> changes since v4:
> - add enable time of ldo.
> - use the device of mfd driver for the regulator_config.
> - add the regulators_node support.
This doesn't build at all, please at least compile test before sending
new revisions.
> ---
> drivers/regulator/Kconfig | 9 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/mt6359-regulator.c | 669 +
> include/linux/regulator/mt6359-regulator.h | 58 ++
> 4 files changed, 737 insertions(+)
> create mode 100644 drivers/regulator/mt6359-regulator.c
> create mode 100644 include/linux/regulator/mt6359-regulator.h
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 53fa84f4d1e1..3de7bb5be8ac 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -750,6 +750,15 @@ config REGULATOR_MT6358
> This driver supports the control of different power rails of device
> through regulator interface.
>
> +config REGULATOR_MT6359
> + tristate "MediaTek MT6359 PMIC"
> + depends on MFD_MT6397
> + help
> + Say y here to select this option to enable the power regulator of
> + MediaTek MT6359 PMIC.
> + This driver supports the control of different power rails of device
> + through regulator interface.
> +
> config REGULATOR_MT6360
> tristate "MT6360 SubPMIC Regulator"
> depends on MFD_MT6360
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 680e539f6579..4f65eaead82d 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -91,6 +91,7 @@ obj-$(CONFIG_REGULATOR_MPQ7920) += mpq7920.o
> obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o
> obj-$(CONFIG_REGULATOR_MT6323) += mt6323-regulator.o
> obj-$(CONFIG_REGULATOR_MT6358) += mt6358-regulator.o
> +obj-$(CONFIG_REGULATOR_MT6359) += mt6359-regulator.o
> obj-$(CONFIG_REGULATOR_MT6360) += mt6360-regulator.o
> obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o
> obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o
> diff --git a/drivers/regulator/mt6359-regulator.c
> b/drivers/regulator/mt6359-regulator.c
> new file mode 100644
> index ..fabc3f57f334
> --- /dev/null
> +++ b/drivers/regulator/mt6359-regulator.c
> @@ -0,0 +1,669 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2020 MediaTek Inc.
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define MT6359_BUCK_MODE_AUTO 0
> +#define MT6359_BUCK_MODE_FORCE_PWM 1
> +#define MT6359_BUCK_MODE_NORMAL0
> +#define MT6359_BUCK_MODE_LP2
> +
> +/*
> + * MT6359 regulators' information
> + *
> + * @desc: standard fields of regulator description.
> + * @status_reg: for query status of regulators.
> + * @qi: Mask for query enable signal status of regulators.
> + * @modeset_reg: for operating AUTO/PWM mode register.
> + * @modeset_mask: MASK for operating modeset register.
> + * @modeset_shift: SHIFT for operating modeset register.
> + */
> +struct mt6359_regulator_info {
> + struct regulator_desc desc;
> + u32 status_reg;
> + u32 qi;
> + u32 modeset_reg;
> + u32 modeset_mask;
> + u32 modeset_shift;
> + u32 lp_mode_reg;
> + u32 lp_mode_mask;
> + u32 lp_mode_shift;
> +};
> +
> +#define MT6359_BUCK(match, _name, min, max, step, min_sel, \
> + volt_ranges, _enable_reg, _status_reg, \
> + _vsel_reg, _vsel_mask, \
> + _lp_mode_reg, _lp_mode_shift, \
> + _modeset_reg, _modeset_shift) \
> +[MT6359_ID_##_name] = {\
> + .desc = { \
> + .name = #_name, \
> + .of_match = of_match_ptr(match),\
> + .regulators_node = of_match_ptr("regulators"), \
> + .ops = _volt_range_ops, \
> + .type = REGULATOR_VOLTAGE, \
> + .id = MT6359_ID_##_name,\
> + .owner = THIS_MODULE, \
> + .uV_step = (step), \
> + .linear_min_sel = (min_sel),\
> + .linear_min_sel = (min_sel),\
> + .n_voltages = ((max) - (min)) / (step) + 1, \
> +
Re: [PATCH] fs: generic_copy_file_checks: Do not adjust count based on file size
On Wed, Jan 27, 2021 at 7:38 AM Dave Chinner wrote:
>
> On Tue, Jan 26, 2021 at 01:50:22PM +0800, Nicolas Boichat wrote:
> > copy_file_range (which calls generic_copy_file_checks) uses the
> > inode file size to adjust the copy count parameter. This breaks
> > with special filesystems like procfs/sysfs, where the file size
> > appears to be zero, but content is actually returned when a read
> > operation is performed.
> >
> > This commit ignores the source file size, and makes copy_file_range
> > match the end of file behaviour documented in POSIX's "read",
> > where 0 is returned to mark EOF. This would allow "cp" and other
> > standard tools to make use of copy_file_range with the exact same
> > behaviour as they had in the past.
> >
> > Fixes: 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")
> > Signed-off-by: Nicolas Boichat
>
> Nack.
Thanks Dave and Al for the detailed explanations.
>
> As I've explained, this is intentional and bypassing it is not a
> work around for enabling cfr on filesystems that produce ephemeral,
> volatile read-once data using seq-file pipes that masquerade as
> regular files with zero size. These files are behaving like pipes
> and only work because the VFS has to support read() and friends from
> pipes that don't publish the amount of data they contain to the VFS
> inode.
>
> copy_file_range() does not support such behaviour.
>
> copy_file_range() -writes- data, so we have to check that those
> writes do not extend past boundaries that the destination inode
> imposes on the operation. e.g. maximum offset limits, whether the
> ranges overlap in the same file, etc.
>
> Hence we need to know how much data there is present to copy before
> we can check if it is safe to perform the -write- of the data we are
> going to read. Hence we cannot safely support data sources that
> cannot tell us how much data is present before we start the copy
> operation.
>
> IOWs, these source file EOF restrictions are required by the write
> side of copy_file_range(), not the read side.
>
> > ---
> > This can be reproduced with this simple test case:
> > #define _GNU_SOURCE
> > #include
> > #include
> > #include
> > #include
> > #include
> >
> > int
> > main(int argc, char **argv)
> > {
> >int fd_in, fd_out;
> >loff_t ret;
> >
> >fd_in = open("/proc/version", O_RDONLY);
> >fd_out = open("version", O_CREAT | O_WRONLY | O_TRUNC, 0644);
> >
> >do {
> > ret = copy_file_range(fd_in, NULL, fd_out, NULL, 1024, 0);
> > printf("%d bytes copied\n", (int)ret);
> >} while (ret > 0);
> >
> >return 0;
> > }
> >
> > Without this patch, `version` output file is empty, and no bytes
> > are copied:
> > 0 bytes copied
>
> $ ls -l /proc/version
> -r--r--r-- 1 root root 0 Jan 20 17:25 /proc/version
> $
>
> It's a zero length file.
>
> sysfs does this just fine - it's regular files have a size of
> at least PAGE_SIZE rather than zero, and so copy_file_range works
> just fine on them:
>
> $ ls -l /sys/block/nvme0n1/capability
> -r--r--r-- 1 root root 4096 Jan 27 08:41 /sys/block/nvme0n1/capability
> $ cat /sys/block/nvme0n1/capability
> 50
> $ xfs_io -f -c "copy_range -s 0 -d 0 -l 4096 /sys/block/nvme0n1/capability"
> /tmp/foo
> $ sudo cat /tmp/foo
> 50
>
> And the behaviour is exactly as you'd expect a read() loop to copy
> the file to behave:
>
> openat(AT_FDCWD, "/tmp/foo", O_RDWR|O_CREAT, 0600) = 3
>
> openat(AT_FDCWD, "/sys/block/nvme0n1/capability", O_RDONLY) = 4
> copy_file_range(4, [0], 3, [0], 4096, 0) = 3
> copy_file_range(4, [3], 3, [3], 4093, 0) = 0
> close(4)
>
> See? Inode size of 4096 means there's a maximum of 4kB of data that
> can be read from this file. copy_file_range() now behaves exactly
> as read() would, returning a short copy and then 0 bytes to indicate
> EOF.
Unless the content happens to be larger than PAGE_SIZE, then
copy_file_range would only copy the beginning of the file. And as Al
explained, this will still break in case of short writes.
>
> If you want ephemeral data pipes masquerading as regular files to
> work with copy_file_range, then the filesystem implementation needs
> to provide the VFS with a data size that indicates the maximum
> amount of data that the pipe can produce in a continuous read loop.
> Otherwise we cannot validate the range of the write we may be asked
> to perform...
>
> > Under the hood, Go 1.15 uses `copy_file_range` syscall
Re: [PATCH v3 05/22] media: camss: Refactor VFE HW version support
On Wed, Jan 27, 2021 at 10:56 PM Robert Foss wrote:
>
> In order to support Qualcomm ISP hardware architectures that diverge
> from older architectures, the VFE subdevice driver needs to be refactored
> to better abstract the different ISP architectures.
>
> Gen1 represents the CAMSS ISP architecture. The ISP architecture developed
> after CAMSS, Titan, will be referred to as Gen2.
>
> Signed-off-by: Robert Foss
> ---
> [snip]
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
> b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
> new file mode 100644
> index ..153e0e20664e
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
> [snip]
> +/*
> + * vfe_isr - VFE module interrupt handler
> + * @irq: Interrupt line
> + * @dev: VFE device
> + *
> + * Return IRQ_HANDLED on success
> + */
> +static irqreturn_t vfe_isr(int irq, void *dev)
> +{
> + struct vfe_device *vfe = dev;
> + u32 value0, value1;
> + int i, j;
> +
> + vfe->ops->isr_read(vfe, , );
> +
> + trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n",
> +value0, value1);
Please do not use trace_printk in production code [1,2], it is only
meant for debug use. Consider using trace events, or dev_dbg.
[1] https://elixir.bootlin.com/linux/v5.8/source/kernel/trace/trace.c#L3158
[2] https://elixir.bootlin.com/linux/v5.8/source/include/linux/kernel.h#L766
> [snip]
[PATCH] fs: generic_copy_file_checks: Do not adjust count based on file size
copy_file_range (which calls generic_copy_file_checks) uses the
inode file size to adjust the copy count parameter. This breaks
with special filesystems like procfs/sysfs, where the file size
appears to be zero, but content is actually returned when a read
operation is performed.
This commit ignores the source file size, and makes copy_file_range
match the end of file behaviour documented in POSIX's "read",
where 0 is returned to mark EOF. This would allow "cp" and other
standard tools to make use of copy_file_range with the exact same
behaviour as they had in the past.
Fixes: 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")
Signed-off-by: Nicolas Boichat
---
This can be reproduced with this simple test case:
#define _GNU_SOURCE
#include
#include
#include
#include
#include
int
main(int argc, char **argv)
{
int fd_in, fd_out;
loff_t ret;
fd_in = open("/proc/version", O_RDONLY);
fd_out = open("version", O_CREAT | O_WRONLY | O_TRUNC, 0644);
do {
ret = copy_file_range(fd_in, NULL, fd_out, NULL, 1024, 0);
printf("%d bytes copied\n", (int)ret);
} while (ret > 0);
return 0;
}
Without this patch, `version` output file is empty, and no bytes
are copied:
0 bytes copied
With this patch, the loop runs twice and the content of the file
is copied:
315 bytes copied
0 bytes copied
We hit this issue when upgrading Go compiler from 1.13 to 1.15 [1],
as we use Go's `io.Copy` to copy the content of
`/sys/kernel/debug/tracing/trace` to a temporary file.
Under the hood, Go 1.15 uses `copy_file_range` syscall to optimize the
copy operation. However, that fails to copy any content when the input
file is from sysfs/tracefs, with an apparent size of 0 (but there is
still content when you `cat` it, of course).
[1] http://issuetracker.google.com/issues/178332739
fs/read_write.c | 8
1 file changed, 8 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 75f764b43418..7236146f6ad7 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1424,7 +1424,6 @@ static int generic_copy_file_checks(struct file *file_in,
loff_t pos_in,
struct inode *inode_in = file_inode(file_in);
struct inode *inode_out = file_inode(file_out);
uint64_t count = *req_count;
- loff_t size_in;
int ret;
ret = generic_file_rw_checks(file_in, file_out);
@@ -1442,13 +1441,6 @@ static int generic_copy_file_checks(struct file
*file_in, loff_t pos_in,
if (pos_in + count < pos_in || pos_out + count < pos_out)
return -EOVERFLOW;
- /* Shorten the copy to EOF */
- size_in = i_size_read(inode_in);
- if (pos_in >= size_in)
- count = 0;
- else
- count = min(count, size_in - (uint64_t)pos_in);
-
ret = generic_write_check_limits(file_out, pos_out, );
if (ret)
return ret;
--
2.30.0.280.ga3ce27912f-goog
Re: [BUG] copy_file_range with sysfs file as input
On Tue, Jan 26, 2021 at 9:34 AM Dave Chinner wrote: > > On Mon, Jan 25, 2021 at 03:54:31PM +0800, Nicolas Boichat wrote: > > Hi copy_file_range experts, > > > > We hit this interesting issue when upgrading Go compiler from 1.13 to > > 1.15 [1]. Basically we use Go's `io.Copy` to copy the content of > > `/sys/kernel/debug/tracing/trace` to a temporary file. > > > > Under the hood, Go now uses `copy_file_range` syscall to optimize the > > copy operation. However, that fails to copy any content when the input > > file is from sysfs/tracefs, with an apparent size of 0 (but there is > > still content when you `cat` it, of course). > > > > A repro case is available in comment7 (adapted from the man page), > > also copied below [2]. > > > > Output looks like this (on kernels 5.4.89 (chromeos), 5.7.17 and > > 5.10.3 (chromeos)) > > $ ./copyfrom /sys/kernel/debug/tracing/trace x > > 0 bytes copied > > That's basically telling you that copy_file_range() was unable to > copy anything. The man page says: > > RETURN VALUE >Upon successful completion, copy_file_range() will return >the number of bytes copied between files. This could be less >than the length originally requested. If the file offset >of fd_in is at or past the end of file, no bytes are copied, >and copy_file_range() returns zero. > > THe man page explains it perfectly. I'm not that confident the explanation is perfect ,-) How does one define "EOF"? The read manpage (https://man7.org/linux/man-pages/man2/read.2.html) defines it as a zero return value. I don't think using the inode file size is standard. Seems like the kernel is not even trying to read from the source file here. In any case, I can fix this issue by dropping the count check here: https://elixir.bootlin.com/linux/latest/source/fs/read_write.c#L1445 . I'll send a patch so that we can discuss based on that. > Look at the trace file you are > trying to copy: > > $ ls -l /sys/kernel/debug/tracing/trace > -rw-r--r-- 1 root root 0 Jan 19 12:17 /sys/kernel/debug/tracing/trace > $ cat /sys/kernel/debug/tracing/trace > tracer: nop > # > # entries-in-buffer/entries-written: 0/0 #P:8 > # > # _-=> irqs-off > # / _=> need-resched > #| / _---=> hardirq/softirq > #|| / _--=> preempt-depth > #||| / delay > # TASK-PID CPU# TIMESTAMP FUNCTION > # | | | | | > > Yup, the sysfs file reports it's size as zero length, so the CFR > syscall is saying "there's nothing to copy from this empty file" and > so correctly is returning zero without even trying to copy anything > because the file offset is at EOF... > > IOWs, there's no copy_file_range() bug here - it's behaving as > documented. > > 'cat' "works" in this situation because it doesn't check the file > size and just attempts to read unconditionally from the file. Hence > it happily returns non-existent stale data from busted filesystem > implementations that allow data to be read from beyond EOF... `cp` also works, so does `dd` and basically any other file operation. (and I wouldn't call procfs, sysfs, debugfs and friends "busted", they are just... special) > > Cheers, > > Dave. > -- > Dave Chinner > da...@fromorbit.com
Re: [PATCH v10 2/4] arm64: dts: mt8183: Add node for the Mali GPU
On Tue, Jan 26, 2021 at 3:27 AM Rob Herring wrote:
>
[...]
> > + {
> > + supply-names = "mali", "sram";
>
> Not a documented property, nor should it be.
>
> Did you run this against dtbs_check with your schema changes?
I did not, for some reasons I hit a strange issue (kernel build system
wouldn't pick up dt-* tools from ~/.local/bin... solved with a bunch
of symlinks to ~/bin). Gave up too quickly ,-(
Anyway, v11 coming with clean make dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml.
>
> > + mali-supply = <_vgpu_reg>;
> > + sram-supply = <_vsram_gpu_reg>;
> > +};
> > +
> > {
> > pinctrl-names = "default";
> > pinctrl-0 = <_pins_0>;
[PATCH v11 2/4] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat
---
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
< CLK_TOP_MFGPLL_CK>,
< CLK_TOP_MUX_MFG>,
<>,
< CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
(based on discussions, this probably belongs in the clock core)
This only matters for devfreq, that is disabled anyway as we don't
have platform-specific code to handle >1 supplies.
Changes in v11:
- mt8183*.dts: remove incorrect supply-names
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6:
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.
Changes in v5:
- Rename "2d" power domain to "core2" (keep R-B again).
Changes in v4:
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)
Changes in v3: None
Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++
3 files changed, 115 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index cba2d8933e79..1cfbea5a0101 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -42,6 +42,11 @@ {
status = "okay";
};
+ {
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index bf2ad1294dd3..a38315b604df 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -249,6 +249,11 @@ {
proc-supply = <_vproc11_reg>;
};
+ {
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7..5430e05e18a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -964,6 +964,111 @@ mfgcfg: syscon@1300 {
#clock-cells = <1>;
};
+ gpu: gpu@1304 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x1304 0 0x4000>;
+ interrupts =
+ ,
+ ,
+ ;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = < CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ < MT8183_POWER_DOMAIN_MFG_CORE0>,
+ < MT8183_POWER_DOMAIN_MFG_CORE1>,
+ < MT8183_POWER_DOMAIN_MFG_2D>;
+ power-domain-names = "core0", "core1", "core2";
+
+ operating-points-v2 = <_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ opp-microvolt = <625000>, <85>;
+ };
+
+ opp-32000 {
+ opp-hz = /bits/ 64 <32000>;
+ opp-microvolt = <631250>, <85>;
+ };
+
+ opp-34000 {
+ opp-hz = /bits/ 64 <34000>;
+ opp-microvolt = <637500>, <85>;
+ };
+
+ opp-36000 {
+ opp-hz = /bits/ 64 <36000>;
+ opp-m
[PATCH v11 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will
require platform-specific handling for devfreq, for 2 reasons:
1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator)
does not support multiple regulators, so we'll need custom
handlers.
2. Generally, platforms with 2 regulators have platform-specific
constraints on how the voltages should be set (e.g.
minimum/maximum voltage difference between them), so we
should not just create generic handlers that simply
change the voltages without taking care of those constraints.
Disable devfreq for now on those GPUs.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
Reviewed-by: Steven Price
---
Changes in v11: None
Changes in v10: None
Changes in v9:
- Explain why devfreq needs to be disabled for GPUs with >1
regulators.
Changes in v8:
- Use DRM_DEV_INFO instead of ERROR
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- devfreq: New change
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 7c5ffc81dce1..a544ae1e560a 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -93,6 +93,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
+ if (pfdev->comp->num_supplies > 1) {
+ /*
+* GPUs with more than 1 supply require platform-specific
handling:
+* continue without devfreq
+*/
+ DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n");
+ return 0;
+ }
+
opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
pfdev->comp->num_supplies);
if (IS_ERR(opp_table)) {
--
2.30.0.280.ga3ce27912f-goog
[PATCH v11 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
Reviewed-by: Steven Price
---
Changes in v11: None
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.
Changes in v5:
- Change power domain name from 2d to core2.
Changes in v4:
- Add power domain names.
Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
Changes in v2: None
drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 83a461bdeea8..ca07098a6141 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = {
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2"
};
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
+ .pm_domain_names = mediatek_mt8183_pm_domains,
+};
+
static const struct of_device_id dt_match[] = {
/* Set first to probe before the generic compatibles */
{ .compatible = "amlogic,meson-gxm-mali",
@@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t860", .data = _data, },
{ .compatible = "arm,mali-t880", .data = _data, },
{ .compatible = "arm,mali-bifrost", .data = _data, },
+ { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.30.0.280.ga3ce27912f-goog
[PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat --- Changes in v11: - binding: power-domain-names not power-domainS-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: None Changes in v8: None Changes in v7: None Changes in v6: - Rebased, actually tested with recent mesa driver. Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: None Changes in v2: None .../bindings/gpu/arm,mali-bifrost.yaml| 28 +++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..3e758f88e2cd 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -41,6 +42,8 @@ properties: mali-supply: true + sram-supply: true + operating-points-v2: true power-domains: @@ -87,6 +90,31 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + required: +- sram-supply +- power-domains +- power-domain-names +else: + properties: +sram-supply: false examples: - | -- 2.30.0.280.ga3ce27912f-goog
[PATCH v11 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v11: - binding: power-domain-names not power-domainS-names - mt8183*.dts: remove incorrect supply-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. - Add gpu regulators to kukui dtsi as well. - Power domains are now attached to spm, not scpsys - Drop R-B. - devfreq: New change - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Rename "2d" power domain to "core2" - Rename "2d" power domain to "core2" (keep R-B again). - Change power domain name from 2d to core2. Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) - Add power-domain-names to describe the 3 domains. (kept Alyssa's reviewed-by as the change is minor) - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. Changes in v2: - Use sram instead of mali_sram as SRAM supply name. - Rename mali@ to gpu@. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 28 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 162 insertions(+) -- 2.30.0.280.ga3ce27912f-goog
[BUG] copy_file_range with sysfs file as input
Hi copy_file_range experts,
We hit this interesting issue when upgrading Go compiler from 1.13 to
1.15 [1]. Basically we use Go's `io.Copy` to copy the content of
`/sys/kernel/debug/tracing/trace` to a temporary file.
Under the hood, Go now uses `copy_file_range` syscall to optimize the
copy operation. However, that fails to copy any content when the input
file is from sysfs/tracefs, with an apparent size of 0 (but there is
still content when you `cat` it, of course).
A repro case is available in comment7 (adapted from the man page),
also copied below [2].
Output looks like this (on kernels 5.4.89 (chromeos), 5.7.17 and
5.10.3 (chromeos))
$ ./copyfrom /sys/kernel/debug/tracing/trace x
0 bytes copied
$ cat x
$ cat /sys/kernel/debug/tracing/trace
# tracer: nop
#
# entries-in-buffer/entries-written: 0/0 #P:8
#
#_-=> irqs-off
# / _=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# TIMESTAMP FUNCTION
# | | | | |
I can try to dig further, but thought you'd like to get a bug report
as soon as possible.
Thanks,
Nicolas
[1] http://issuetracker.google.com/issues/178332739
[2]
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
int
main(int argc, char **argv)
{
int fd_in, fd_out;
loff_t ret;
if (argc != 3) {
fprintf(stderr, "Usage: %s \n", argv[0]);
exit(EXIT_FAILURE);
}
fd_in = open(argv[1], O_RDONLY);
if (fd_in == -1) {
perror("open (argv[1])");
exit(EXIT_FAILURE);
}
fd_out = open(argv[2], O_CREAT | O_WRONLY | O_TRUNC, 0644);
if (fd_out == -1) {
perror("open (argv[2])");
exit(EXIT_FAILURE);
}
ret = copy_file_range(fd_in, NULL, fd_out, NULL, 1024, 0);
if (ret == -1) {
perror("copy_file_range");
exit(EXIT_FAILURE);
}
printf("%d bytes copied\n", (int)ret);
close(fd_in);
close(fd_out);
exit(EXIT_SUCCESS);
}
Re: [PATCH] pinctrl: mediatek: Fix trigger type setting follow for unexpected interrupt
On Mon, Jan 25, 2021 at 11:15 AM Hailong Fan wrote:
>
> When flipping the polarity will be generated interrupt under certain
> circumstances, but GPIO external signal has not changed.
> Then, mask the interrupt before polarity setting, and clear the
> unexpected interrupt after trigger type setting completed.
I'd add a short note about why you remove mtk_eint_flip_edge, that is,
because mtk_eint_unmask already calls it.
>
> Signed-off-by: Hailong Fan
> ---
> [V2]
> ---
> drivers/pinctrl/mediatek/mtk-eint.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/mtk-eint.c
> b/drivers/pinctrl/mediatek/mtk-eint.c
> index 22736f60c16c..0042f32c7e7e 100644
> --- a/drivers/pinctrl/mediatek/mtk-eint.c
> +++ b/drivers/pinctrl/mediatek/mtk-eint.c
> @@ -157,6 +157,7 @@ static void mtk_eint_ack(struct irq_data *d)
> static int mtk_eint_set_type(struct irq_data *d, unsigned int type)
> {
> struct mtk_eint *eint = irq_data_get_irq_chip_data(d);
> + bool unmasked;
Well, this is true if the interrupt has been masked (or, equivalently,
if we need to unmask it later).
So I think either "masked" or "unmask" are better as variable names.
> u32 mask = BIT(d->hwirq & 0x1f);
> void __iomem *reg;
>
> @@ -173,6 +174,13 @@ static int mtk_eint_set_type(struct irq_data *d,
> unsigned int type)
> else
> eint->dual_edge[d->hwirq] = 0;
>
> + if (!mtk_eint_get_mask(eint, d->hwirq)) {
> + mtk_eint_mask(d);
> + unmasked = true;
> + } else {
> + unmasked = false;
> + }
> +
> if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) {
> reg = mtk_eint_get_offset(eint, d->hwirq,
> eint->regs->pol_clr);
> writel(mask, reg);
> @@ -189,8 +197,9 @@ static int mtk_eint_set_type(struct irq_data *d, unsigned
> int type)
> writel(mask, reg);
> }
>
> - if (eint->dual_edge[d->hwirq])
> - mtk_eint_flip_edge(eint, d->hwirq);
> + mtk_eint_ack(d);
> + if (unmasked)
> + mtk_eint_unmask(d);
>
> return 0;
> }
> --
> 2.18.0
Re: [PATCH RESEND] pinctrl: mediatek: Fix trigger type setting follow for unexpected interrupt
On Fri, Jan 22, 2021 at 10:44 AM mtk15103 wrote:
>
> On Thu, 2021-01-21 at 20:13 +0800, Nicolas Boichat wrote:
> > On Thu, Jan 21, 2021 at 8:09 PM mtk15103 wrote:
> > >
> > > On Thu, 2021-01-21 at 16:55 +0800, Nicolas Boichat wrote:
> > > > On Thu, Jan 21, 2021 at 3:52 PM Hailong Fan
> > > > wrote:
> > > > >
> > > > > When flipping the polarity will be generated interrupt under certain
> > > > > circumstances, but GPIO external signal has not changed.
> > > > > Then, mask the interrupt before polarity setting, and clear the
> > > > > unexpected interrupt after trigger type setting completed.
> > > > >
> > > > > Signed-off-by: Hailong Fan
> > > > > ---
> > > > > Resend since some server reject.
> > > > > ---
> > > > > drivers/pinctrl/mediatek/mtk-eint.c | 13 +++--
> > > > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/pinctrl/mediatek/mtk-eint.c
> > > > > b/drivers/pinctrl/mediatek/mtk-eint.c
> > > > > index 22736f60c16c..3acda6bb401e 100644
> > > > > --- a/drivers/pinctrl/mediatek/mtk-eint.c
> > > > > +++ b/drivers/pinctrl/mediatek/mtk-eint.c
> > > > > @@ -157,6 +157,7 @@ static void mtk_eint_ack(struct irq_data *d)
> > > > > static int mtk_eint_set_type(struct irq_data *d, unsigned int type)
> > > > > {
> > > > > struct mtk_eint *eint = irq_data_get_irq_chip_data(d);
> > > > > + unsigned int unmask;
> > > >
> > > > bool?
> > > Yes,thanks.
> > > >
> > > > > u32 mask = BIT(d->hwirq & 0x1f);
> > > > > void __iomem *reg;
> > > > >
> > > > > @@ -173,6 +174,13 @@ static int mtk_eint_set_type(struct irq_data *d,
> > > > > unsigned int type)
> > > > > else
> > > > > eint->dual_edge[d->hwirq] = 0;
> > > > >
> > > > > + if (!mtk_eint_get_mask(eint, d->hwirq)) {
> > > > > + mtk_eint_mask(d);
> > > > > + unmask = 1;
> > > > > + } else {
> > > > > + unmask = 0;
> > > > > + }
> > > > > +
> > > > > if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) {
> > > > > reg = mtk_eint_get_offset(eint, d->hwirq,
> > > > > eint->regs->pol_clr);
> > > > > writel(mask, reg);
> > > > > @@ -189,8 +197,9 @@ static int mtk_eint_set_type(struct irq_data *d,
> > > > > unsigned int type)
> > > > > writel(mask, reg);
> > > > > }
> > > > >
> > > > > - if (eint->dual_edge[d->hwirq])
> > > > > - mtk_eint_flip_edge(eint, d->hwirq);
> > > >
> > > > Why are you dropping this? Aren't we at risk to miss the first edge
> > > > after mtk_eint_set_type is called?
> > > mtk_eint_unmask() will do it.
> > > If unmask != 1, user need to call mtk_eint_unmask() to enable the
> > > interrupt before use it, thanks.
> >
> > Makes sense, I just have one more worry:
> > https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/mediatek/mtk-eint.c#L122
> >
> > mtk_eint_unmask unmasks the interrupt _before_ the edge is flipped,
> > could this cause a spurious interrupt? On any unmask operation -- not
> > just on mtk_eint_set_type (so this is technically another problem,
> > that should be fixed as a separate patch)
>
> This situation will not happen.
>
> Spurious interrupt occur condition: edge irq polarity value is same with
> input signal.
> e.g.
> Default value: GPIO input is high, trigger type is falling_edge.
> User want modify trigger type is rising_edge under some certain
> circumstances, it will generate spurious interrupt but external signal
> maintain high.
> So we need ack interrupt after trigger_type setting is completed.
>
> mtk_eint_flip_edge is for dual_edge, the polarity setting based on
> current gpio input signal:
> if (input == high)
> polarity = low; /* falling_edge */
> else
> polarity = high; /* rising_edge */
> Then it's not cause a spurious interrupt.
Okay let me try to make sure I follow:
- Say interrupt is currently IRQ_TYPE_EDGE_FALLING (and GPIO is high and stable)
- mtk_eint_set_type(d, IRQ_TYPE_EDGE_BOTH)
- mtk_eint_mask(d)
- (no mask changed, just eint->dual_edge)
- mtk_eint_ack(d) (not actually needed in this example)
- mtk_eint_unmask(d)
- HW reg unmask (can't generate an interrupt as anything that would
have happened is acked already -- even with different previous/new
types than my example)
- flip_edge (will swap polarity as needed but can't generate
interrupt as it won't set the polarity to be the same as the current
GPIO state)
Okay, I think I'm convinced.
Please fix the nits and then you can add
Reviewed-by: Nicolas Boichat
> > > > > + mtk_eint_ack(d);
> > > > > + if (unmask == 1)
> > > >
> > > > Just `if (unmask)`
> > > Yes,thanks.
> > > > > + mtk_eint_unmask(d);
> > > > >
> > > > > return 0;
> > > > > }
> > > > > --
> > > > > 2.18.0
> > >
>
Re: [PATCH RESEND] pinctrl: mediatek: Fix trigger type setting follow for unexpected interrupt
On Thu, Jan 21, 2021 at 8:09 PM mtk15103 wrote:
>
> On Thu, 2021-01-21 at 16:55 +0800, Nicolas Boichat wrote:
> > On Thu, Jan 21, 2021 at 3:52 PM Hailong Fan
> > wrote:
> > >
> > > When flipping the polarity will be generated interrupt under certain
> > > circumstances, but GPIO external signal has not changed.
> > > Then, mask the interrupt before polarity setting, and clear the
> > > unexpected interrupt after trigger type setting completed.
> > >
> > > Signed-off-by: Hailong Fan
> > > ---
> > > Resend since some server reject.
> > > ---
> > > drivers/pinctrl/mediatek/mtk-eint.c | 13 +++--
> > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/mediatek/mtk-eint.c
> > > b/drivers/pinctrl/mediatek/mtk-eint.c
> > > index 22736f60c16c..3acda6bb401e 100644
> > > --- a/drivers/pinctrl/mediatek/mtk-eint.c
> > > +++ b/drivers/pinctrl/mediatek/mtk-eint.c
> > > @@ -157,6 +157,7 @@ static void mtk_eint_ack(struct irq_data *d)
> > > static int mtk_eint_set_type(struct irq_data *d, unsigned int type)
> > > {
> > > struct mtk_eint *eint = irq_data_get_irq_chip_data(d);
> > > + unsigned int unmask;
> >
> > bool?
> Yes,thanks.
> >
> > > u32 mask = BIT(d->hwirq & 0x1f);
> > > void __iomem *reg;
> > >
> > > @@ -173,6 +174,13 @@ static int mtk_eint_set_type(struct irq_data *d,
> > > unsigned int type)
> > > else
> > > eint->dual_edge[d->hwirq] = 0;
> > >
> > > + if (!mtk_eint_get_mask(eint, d->hwirq)) {
> > > + mtk_eint_mask(d);
> > > + unmask = 1;
> > > + } else {
> > > + unmask = 0;
> > > + }
> > > +
> > > if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) {
> > > reg = mtk_eint_get_offset(eint, d->hwirq,
> > > eint->regs->pol_clr);
> > > writel(mask, reg);
> > > @@ -189,8 +197,9 @@ static int mtk_eint_set_type(struct irq_data *d,
> > > unsigned int type)
> > > writel(mask, reg);
> > > }
> > >
> > > - if (eint->dual_edge[d->hwirq])
> > > - mtk_eint_flip_edge(eint, d->hwirq);
> >
> > Why are you dropping this? Aren't we at risk to miss the first edge
> > after mtk_eint_set_type is called?
> mtk_eint_unmask() will do it.
> If unmask != 1, user need to call mtk_eint_unmask() to enable the
> interrupt before use it, thanks.
Makes sense, I just have one more worry:
https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/mediatek/mtk-eint.c#L122
mtk_eint_unmask unmasks the interrupt _before_ the edge is flipped,
could this cause a spurious interrupt? On any unmask operation -- not
just on mtk_eint_set_type (so this is technically another problem,
that should be fixed as a separate patch)
> > > + mtk_eint_ack(d);
> > > + if (unmask == 1)
> >
> > Just `if (unmask)`
> Yes,thanks.
> > > + mtk_eint_unmask(d);
> > >
> > > return 0;
> > > }
> > > --
> > > 2.18.0
>
Re: [PATCH RESEND] pinctrl: mediatek: Fix trigger type setting follow for unexpected interrupt
On Thu, Jan 21, 2021 at 3:52 PM Hailong Fan wrote:
>
> When flipping the polarity will be generated interrupt under certain
> circumstances, but GPIO external signal has not changed.
> Then, mask the interrupt before polarity setting, and clear the
> unexpected interrupt after trigger type setting completed.
>
> Signed-off-by: Hailong Fan
> ---
> Resend since some server reject.
> ---
> drivers/pinctrl/mediatek/mtk-eint.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/mtk-eint.c
> b/drivers/pinctrl/mediatek/mtk-eint.c
> index 22736f60c16c..3acda6bb401e 100644
> --- a/drivers/pinctrl/mediatek/mtk-eint.c
> +++ b/drivers/pinctrl/mediatek/mtk-eint.c
> @@ -157,6 +157,7 @@ static void mtk_eint_ack(struct irq_data *d)
> static int mtk_eint_set_type(struct irq_data *d, unsigned int type)
> {
> struct mtk_eint *eint = irq_data_get_irq_chip_data(d);
> + unsigned int unmask;
bool?
> u32 mask = BIT(d->hwirq & 0x1f);
> void __iomem *reg;
>
> @@ -173,6 +174,13 @@ static int mtk_eint_set_type(struct irq_data *d,
> unsigned int type)
> else
> eint->dual_edge[d->hwirq] = 0;
>
> + if (!mtk_eint_get_mask(eint, d->hwirq)) {
> + mtk_eint_mask(d);
> + unmask = 1;
> + } else {
> + unmask = 0;
> + }
> +
> if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) {
> reg = mtk_eint_get_offset(eint, d->hwirq,
> eint->regs->pol_clr);
> writel(mask, reg);
> @@ -189,8 +197,9 @@ static int mtk_eint_set_type(struct irq_data *d, unsigned
> int type)
> writel(mask, reg);
> }
>
> - if (eint->dual_edge[d->hwirq])
> - mtk_eint_flip_edge(eint, d->hwirq);
Why are you dropping this? Aren't we at risk to miss the first edge
after mtk_eint_set_type is called?
> + mtk_eint_ack(d);
> + if (unmask == 1)
Just `if (unmask)`
> + mtk_eint_unmask(d);
>
> return 0;
> }
> --
> 2.18.0
Re: [PATCH v2 1/2] dt-bindings: drm/bridge: anx7625: add DPI flag and swing setting
On Tue, Jan 12, 2021 at 4:59 PM Xin Ji wrote:
>
> Hi Rob Herring, thanks for the comments.
>
> On Mon, Jan 11, 2021 at 04:14:35PM -0600, Rob Herring wrote:
> > On Thu, Dec 31, 2020 at 10:21:12AM +0800, Xin Ji wrote:
> > > Add DPI flag for distinguish MIPI input signal type, DSI or DPI. Add
> > > swing setting for adjusting DP tx PHY swing
> > >
> > > Signed-off-by: Xin Ji
> > > ---
> > > .../bindings/display/bridge/analogix,anx7625.yaml | 25
> > > --
> > > 1 file changed, 23 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > index 60585a4..4eb0ea3 100644
> > > ---
> > > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > +++
> > > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > @@ -34,6 +34,16 @@ properties:
> > > description: used for reset chip control, RESET_N pin B7.
> > > maxItems: 1
> > >
> > > + analogix,swing-setting:
> > > +type: uint8-array
> >
> > Humm, this should have be rejected by the meta-schema.
> We needs define an array to adjust DP tx PHY swing, the developer hopes these
> settings are changeable, so I moved the register data to DT. Can you
> give me some suggestion if it is rejected by the meta-schema?
> >
> > > +$ref: /schemas/types.yaml#/definitions/uint32-array
> >
> > This is how types are defined other than boolean or nodes (object).
> >
> > > +description: an array of swing register setting for DP tx PHY
> > > +
> > > + analogix,mipi-dpi-in:
> > > +type: int
> > > +$ref: /schemas/types.yaml#/definitions/uint32
> > > +description: indicate the MIPI rx signal type is DPI or DSI
> >
> > Why does this need to be in DT, you should be able to determine this
> > based on what you are connected to.
> As the anx7625 can receive MIPI DSI and DPI data (depends on hardware
> implement, we have a project which have two anx7625, one is DSI input,
> the other is DPI input), we needs to let driver know what kind of MIPI
> rx signal input. And there is no other way to tell driver the MIPI rx
> signal type, we needs define this flag.
> >
> > > +
> > >ports:
> > > type: object
> > >
> > > @@ -49,8 +59,8 @@ properties:
> > >Video port for panel or connector.
> > >
> > > required:
> > > -- port@0
> > > -- port@1
> > > + - port@0
> > > + - port@1
> > >
> > > required:
> > >- compatible
> > > @@ -72,6 +82,17 @@ examples:
> > > reg = <0x58>;
> > > enable-gpios = < 45 GPIO_ACTIVE_HIGH>;
> > > reset-gpios = < 73 GPIO_ACTIVE_HIGH>;
> > > +analogix,swing-setting = <0x00 0x14>, <0x01 0x54>,
> > > +<0x02 0x64>, <0x03 0x74>, <0x04 0x29>,
> > > +<0x05 0x7b>, <0x06 0x77>, <0x07 0x5b>,
> > > +<0x08 0x7f>, <0x0c 0x20>, <0x0d 0x60>,
> > > +<0x10 0x60>, <0x12 0x40>, <0x13 0x60>,
> > > +<0x14 0x14>, <0x15 0x54>, <0x16 0x64>,
> > > +<0x17 0x74>, <0x18 0x29>, <0x19 0x7b>,
> > > +<0x1a 0x77>, <0x1b 0x5b>, <0x1c 0x7f>,
> > > +<0x20 0x20>, <0x21 0x60>, <0x24 0x60>,
> > > +<0x26 0x40>, <0x27 0x60>;
> >
> > This is a matrix, which is different from an array type.
> OK, I'll change to array if this vendor define has been accepted.
I also wonder if we want to split the parameters per lane, and simply
have a flat array (instead of reg/value pairs like you have now).
Registers 0x00 to 0x13 have to do with Lane 0 (and 0x14 to 0x27 with
Lane 1): you can almost tell from the example values, they repeat. I'm
not sure if there's any value splitting those further (I don't think
anybody will be able to tune those without ANX's help).
So, maybe something like:
anx,swing-setting = <<[reg values for lane 0]>, <[reg values for lane 1]>>
where <[reg values for lane 0]> would be something like <0x14, 0x54,
0x64, ...> (that is, all the values between 0x00 and 0x13).
> >
> > > +analogix,mipi-dpi-in = <0>;
> > >
> > > ports {
> > > #address-cells = <1>;
> > > --
> > > 2.7.4
> > >
Re: [PATCH next 12/15] arm64: dts: mediatek: mt8183: fix dtbs_check warning
On Sun, Jan 17, 2021 at 9:58 AM Nicolas Boichat wrote:
>
> On Sat, Jan 16, 2021 at 5:07 PM Chunfeng Yun
> wrote:
> >
> > Harmonize node names, compatibles and properties.
> >
> > Signed-off-by: Chunfeng Yun
> > ---
> > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 9 -
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 5b782a4769e7..a69a033a68ac 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -865,7 +865,7 @@
> > ranges;
> > status = "disabled";
> >
> > - usb_host: xhci@1120 {
> > + usb_host: usb@1120 {
> > compatible = "mediatek,mt8183-xhci",
> > "mediatek,mtk-xhci";
> > reg = <0 0x1120 0 0x1000>;
> > @@ -908,11 +908,11 @@
> > status = "disabled";
> > };
> >
> > - mipi_tx0: mipi-dphy@11e5 {
> > + mipi_tx0: dsi-phy@11e5 {
> > compatible = "mediatek,mt8183-mipi-tx";
> > reg = <0 0x11e5 0 0x1000>;
> > clocks = < CLK_APMIXED_MIPID0_26M>;
> > - clock-names = "ref_clk";
> > + clock-names = "ref";
> > #clock-cells = <0>;
> > #phy-cells = <0>;
> > clock-output-names = "mipi_tx0_pll";
>
> This is unrelated to USB, so this should probably be a separate patch.
Actually, after looking again at the complete stack of patches, I
think this might be ok as part of this overall cleanup (I'll let the
maintainer speak up).
>
> > @@ -931,11 +931,10 @@
> > };
> > };
> >
> > - u3phy: usb-phy@11f4 {
> > + u3phy: t-phy@11f4 {
> > compatible = "mediatek,mt8183-tphy",
> > "mediatek,generic-tphy-v2";
> > #address-cells = <1>;
> > - #phy-cells = <1>;
> > #size-cells = <1>;
> > ranges = <0 0 0x11f4 0x1000>;
> > status = "okay";
> > --
> > 2.18.0
> > ___
> > Linux-mediatek mailing list
> > linux-media...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH next 12/15] arm64: dts: mediatek: mt8183: fix dtbs_check warning
On Sat, Jan 16, 2021 at 5:07 PM Chunfeng Yun wrote:
>
> Harmonize node names, compatibles and properties.
>
> Signed-off-by: Chunfeng Yun
> ---
> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 5b782a4769e7..a69a033a68ac 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -865,7 +865,7 @@
> ranges;
> status = "disabled";
>
> - usb_host: xhci@1120 {
> + usb_host: usb@1120 {
> compatible = "mediatek,mt8183-xhci",
> "mediatek,mtk-xhci";
> reg = <0 0x1120 0 0x1000>;
> @@ -908,11 +908,11 @@
> status = "disabled";
> };
>
> - mipi_tx0: mipi-dphy@11e5 {
> + mipi_tx0: dsi-phy@11e5 {
> compatible = "mediatek,mt8183-mipi-tx";
> reg = <0 0x11e5 0 0x1000>;
> clocks = < CLK_APMIXED_MIPID0_26M>;
> - clock-names = "ref_clk";
> + clock-names = "ref";
> #clock-cells = <0>;
> #phy-cells = <0>;
> clock-output-names = "mipi_tx0_pll";
This is unrelated to USB, so this should probably be a separate patch.
> @@ -931,11 +931,10 @@
> };
> };
>
> - u3phy: usb-phy@11f4 {
> + u3phy: t-phy@11f4 {
> compatible = "mediatek,mt8183-tphy",
> "mediatek,generic-tphy-v2";
> #address-cells = <1>;
> - #phy-cells = <1>;
> #size-cells = <1>;
> ranges = <0 0 0x11f4 0x1000>;
> status = "okay";
> --
> 2.18.0
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH 2/2] of/fdt: Make sure no-map does not remove already reserved regions
On Sat, Jan 16, 2021 at 1:30 AM Rob Herring wrote:
>
> On Fri, 15 Jan 2021 11:45:44 +, Quentin Perret wrote:
> > From: Nicolas Boichat
> >
> > If the device tree is incorrectly configured, and attempts to
> > define a "no-map" reserved memory that overlaps with the kernel
> > data/code, the kernel would crash quickly after boot, with no
> > obvious clue about the nature of the issue.
> >
> > For example, this would happen if we have the kernel mapped at
> > these addresses (from /proc/iomem):
> > 4000-41ff : System RAM
> > 4008-40df : Kernel code
> > 40e0-411f : reserved
> > 4120-413e0fff : Kernel data
> >
> > And we declare a no-map shared-dma-pool region at a fixed address
> > within that range:
> > mem_reserved: mem_region {
> > compatible = "shared-dma-pool";
> > reg = <0 0x4000 0 0x01A0>;
> > no-map;
> > };
> >
> > To fix this, when removing memory regions at early boot (which is
> > what "no-map" regions do), we need to make sure that the memory
> > is not already reserved. If we do, __reserved_mem_reserve_reg
> > will throw an error:
> > [0.00] OF: fdt: Reserved memory: failed to reserve memory
> >for node 'mem_region': base 0x4000, size 26 MiB
> > and the code that will try to use the region should also fail,
> > later on.
> >
> > We do not do anything for non-"no-map" regions, as memblock
> > explicitly allows reserved regions to overlap, and the commit
> > that this fixes removed the check for that precise reason.
> >
> > [ qperret: fixed conflicts caused by the usage of memblock_mark_nomap ]
> >
> > Fixes: 094cb98179f19b7 ("of/fdt: memblock_reserve /memreserve/ regions in
> > the case of partial overlap")
> > Signed-off-by: Nicolas Boichat
> > Reviewed-by: Stephen Boyd
> > Signed-off-by: Quentin Perret
> > ---
> > drivers/of/fdt.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
Thanks for picking up this old patch Quentin ,-)
> Applied, thanks!
[PATCH v2] HID: google: Get HID report on probe to confirm tablet switch state
This forces reading the base folded state anytime the device is
probed, to make sure it's in sync.
This is useful after a reboot, if the device re-enumerates for
any reason (e.g. ESD shock), or if the driver is unbound/rebound
(debugging/testing).
Without this, the tablet switch state is only synchronized after a
key is pressed (since the device would then send a report that
includes the switch state), leading to strange UX (e.g. UI
mode changes when a key is pressed after reboot).
This is not a problem on detachable base attach, as the device,
by itself, sends a report after it is booted up.
Signed-off-by: Nicolas Boichat
---
Instead of all this manual parsing, it'd be easier to just call:
hid_hw_request(hdev, report, HID_REQ_GET_REPORT);
However, that fails silently as hdev->driver_input_lock is held
during probe (or even some callbacks like input_configured.
Changes in v2:
- Improve commit message description.
drivers/hid/hid-google-hammer.c | 85 +
1 file changed, 66 insertions(+), 19 deletions(-)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 85a054f1ce38..d9319622da44 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -392,30 +392,34 @@ static int hammer_input_mapping(struct hid_device *hdev,
struct hid_input *hi,
return 0;
}
-static int hammer_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+static void hammer_folded_event(struct hid_device *hdev, bool folded)
{
unsigned long flags;
- if (usage->hid == HID_USAGE_KBD_FOLDED) {
- spin_lock_irqsave(_ec_lock, flags);
+ spin_lock_irqsave(_ec_lock, flags);
- /*
-* If we are getting events from Whiskers that means that it
-* is attached to the lid.
-*/
- cbas_ec.base_present = true;
- cbas_ec.base_folded = value;
- hid_dbg(hid, "%s: base: %d, folded: %d\n", __func__,
- cbas_ec.base_present, cbas_ec.base_folded);
-
- if (cbas_ec.input) {
- input_report_switch(cbas_ec.input,
- SW_TABLET_MODE, value);
- input_sync(cbas_ec.input);
- }
+ /*
+* If we are getting events from Whiskers that means that it
+* is attached to the lid.
+*/
+ cbas_ec.base_present = true;
+ cbas_ec.base_folded = folded;
+ hid_dbg(hdev, "%s: base: %d, folded: %d\n", __func__,
+ cbas_ec.base_present, cbas_ec.base_folded);
- spin_unlock_irqrestore(_ec_lock, flags);
+ if (cbas_ec.input) {
+ input_report_switch(cbas_ec.input, SW_TABLET_MODE, folded);
+ input_sync(cbas_ec.input);
+ }
+
+ spin_unlock_irqrestore(_ec_lock, flags);
+}
+
+static int hammer_event(struct hid_device *hid, struct hid_field *field,
+ struct hid_usage *usage, __s32 value)
+{
+ if (usage->hid == HID_USAGE_KBD_FOLDED) {
+ hammer_folded_event(hid, value);
return 1; /* We handled this event */
}
@@ -457,6 +461,47 @@ static bool hammer_has_backlight_control(struct hid_device
*hdev)
HID_GD_KEYBOARD, HID_AD_BRIGHTNESS);
}
+static void hammer_get_folded_state(struct hid_device *hdev)
+{
+ struct hid_report *report;
+ char *buf;
+ int len, rlen;
+ int a;
+
+ report = hdev->report_enum[HID_INPUT_REPORT].report_id_hash[0x0];
+
+ if (!report || report->maxfield < 1)
+ return;
+
+ len = hid_report_len(report) + 1;
+
+ buf = kmalloc(len, GFP_KERNEL);
+ if (!buf)
+ return;
+
+ rlen = hid_hw_raw_request(hdev, report->id, buf, len, report->type,
HID_REQ_GET_REPORT);
+
+ if (rlen != len) {
+ hid_warn(hdev, "Unable to read base folded state: %d (expected
%d)\n", rlen, len);
+ goto out;
+ }
+
+ for (a = 0; a < report->maxfield; a++) {
+ struct hid_field *field = report->field[a];
+
+ if (field->usage->hid == HID_USAGE_KBD_FOLDED) {
+ u32 value = hid_field_extract(hdev, buf+1,
+ field->report_offset,
field->report_size);
+
+ hammer_folded_event(hdev, value);
+ break;
+ }
+ }
+
+out:
+ kfree(buf);
+}
+
static int hammer_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -481,6 +526,8 @@ static int hammer_probe(struct hid_device *hdev,
error = hid_hw_open(hdev);
if (error)
Re: [PATCH] HID: google: Get HID report on probe to confirm tablet switch state
On Thu, Jan 14, 2021 at 8:15 PM Jiri Kosina wrote: > > On Thu, 24 Dec 2020, Nicolas Boichat wrote: > > > This forces reading the base folded status anytime the device is > > probed. > > Could you please provide a little bit more verbose changelog (namely what > is the actual problem this patch is fixing)? Thanks. Sure, I should have done this in the first place... v2 on the way. > > -- > Jiri Kosina > SUSE Labs >
[PATCH v10 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will
require platform-specific handling for devfreq, for 2 reasons:
1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator)
does not support multiple regulators, so we'll need custom
handlers.
2. Generally, platforms with 2 regulators have platform-specific
constraints on how the voltages should be set (e.g.
minimum/maximum voltage difference between them), so we
should not just create generic handlers that simply
change the voltages without taking care of those constraints.
Disable devfreq for now on those GPUs.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
---
(no changes since v9)
Changes in v9:
- Explain why devfreq needs to be disabled for GPUs with >1
regulators.
Changes in v8:
- Use DRM_DEV_INFO instead of ERROR
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- New change
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index f44d28fad085..812cfecdee3b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
+ if (pfdev->comp->num_supplies > 1) {
+ /*
+* GPUs with more than 1 supply require platform-specific
handling:
+* continue without devfreq
+*/
+ DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n");
+ return 0;
+ }
+
opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
pfdev->comp->num_supplies);
if (IS_ERR(opp_table)) {
--
2.30.0.284.gd98b1dd5eaa7-goog
[PATCH v10 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat --- Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 28 +++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..eac561582063 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -41,6 +42,8 @@ properties: mali-supply: true + sram-supply: true + operating-points-v2: true power-domains: @@ -87,6 +90,31 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + required: +- sram-supply +- power-domains +- power-domains-names +else: + properties: +sram-supply: false examples: - | -- 2.30.0.284.gd98b1dd5eaa7-goog
[PATCH v10 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
---
(no changes since v7)
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.
Changes in v5:
- Change power domain name from 2d to core2.
Changes in v4:
- Add power domain names.
Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 83a461bdeea8..ca07098a6141 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = {
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2"
};
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
+ .pm_domain_names = mediatek_mt8183_pm_domains,
+};
+
static const struct of_device_id dt_match[] = {
/* Set first to probe before the generic compatibles */
{ .compatible = "amlogic,meson-gxm-mali",
@@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t860", .data = _data, },
{ .compatible = "arm,mali-t880", .data = _data, },
{ .compatible = "arm,mali-bifrost", .data = _data, },
+ { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.30.0.284.gd98b1dd5eaa7-goog
[PATCH v10 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 28 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 164 insertions(+) -- 2.30.0.284.gd98b1dd5eaa7-goog
[PATCH v10 2/4] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat
---
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
< CLK_TOP_MFGPLL_CK>,
< CLK_TOP_MUX_MFG>,
<>,
< CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
(based on discussions, this probably belongs in the clock core)
This only matters for devfreq, that is disabled anyway as we don't
have platform-specific code to handle >1 supplies.
(no changes since v6)
Changes in v6:
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.
Changes in v5:
- Rename "2d" power domain to "core2" (keep R-B again).
Changes in v4:
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)
Changes in v3:
- No changes
Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++
3 files changed, 117 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index cba2d8933e79..0a8c2fad8e16 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -42,6 +42,12 @@ {
status = "okay";
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index bf2ad1294dd3..00d8e112cab9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -249,6 +249,12 @@ {
proc-supply = <_vproc11_reg>;
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7..5430e05e18a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -964,6 +964,111 @@ mfgcfg: syscon@1300 {
#clock-cells = <1>;
};
+ gpu: gpu@1304 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x1304 0 0x4000>;
+ interrupts =
+ ,
+ ,
+ ;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = < CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ < MT8183_POWER_DOMAIN_MFG_CORE0>,
+ < MT8183_POWER_DOMAIN_MFG_CORE1>,
+ < MT8183_POWER_DOMAIN_MFG_2D>;
+ power-domain-names = "core0", "core1", "core2";
+
+ operating-points-v2 = <_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ opp-microvolt = <625000>, <85>;
+ };
+
+ opp-32000 {
+ opp-hz = /bits/ 64 <32000>;
+ opp-microvolt = <631250>, <85>;
+ };
+
+ opp-34000 {
+ opp-hz = /bits/ 64 <34000>;
+ opp-microvolt = <637500>, <85>;
+ };
+
+ opp-36000 {
+ opp-hz = /bits/ 64 <36000>;
+
Re: [PATCH v9 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
On Tue, Jan 12, 2021 at 11:07 PM Rob Herring wrote: > > On Fri, Jan 08, 2021 at 09:10:08AM +0800, Nicolas Boichat wrote: > > Define a compatible string for the Mali Bifrost GPU found in > > Mediatek's MT8183 SoCs. > > > > Signed-off-by: Nicolas Boichat > > Reviewed-by: Alyssa Rosenzweig > > --- > > > > (no changes since v6) > > > > Changes in v6: > > - Rebased, actually tested with recent mesa driver. > > - No change > > > > Changes in v5: > > - Rename "2d" power domain to "core2" > > > > Changes in v4: > > - Add power-domain-names description > >(kept Alyssa's reviewed-by as the change is minor) > > > > Changes in v3: > > - No change > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > index 184492162e7e..71b613ee5bd7 100644 > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > @@ -17,6 +17,7 @@ properties: > > items: > >- enum: > >- amlogic,meson-g12a-mali > > + - mediatek,mt8183-mali > >- realtek,rtd1619-mali > >- rockchip,px30-mali > >- const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully > > discoverable > > @@ -87,6 +88,30 @@ allOf: > > then: > >required: > > - resets > > + - if: > > + properties: > > +compatible: > > + contains: > > +const: mediatek,mt8183-mali > > +then: > > + properties: > > +sram-supply: true > > This has to be defined at the top-level or there will be an error when > it is present (due to additionalProperties). > > In this if/then you can do: > > else: > sram-supply: false > > to disallow it if not 'mediatek,mt8183-mali' I see. Thanks Rob, will send a v10. > > > +power-domains: > > + description: > > +List of phandle and PM domain specifier as documented in > > +Documentation/devicetree/bindings/power/power_domain.txt > > + minItems: 3 > > + maxItems: 3 > > +power-domain-names: > > + items: > > +- const: core0 > > +- const: core1 > > +- const: core2 > > + > > + required: > > +- sram-supply > > +- power-domains > > +- power-domains-names > > > > examples: > >- | > > -- > > 2.29.2.729.g45daf8777d-goog > >
Re: [v5 3/3] thermal: mediatek: add another get_temp ops for thermal sensors
On Tue, Oct 13, 2020 at 6:24 PM Michael Kao wrote:
>
> Provide thermal zone to read thermal sensor
> in the SoC. We can read all the thermal sensors
> value in the SoC by the node /sys/class/thermal/
>
> In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
> on the first read of sensor that often are bogus values.
>
> This can avoid following warning on boot:
>
> thermal thermal_zone6: failed to read out thermal zone (-13)
>
> Signed-off-by: Michael Kao
> Signed-off-by: Hsin-Yi Wang
> ---
> drivers/thermal/mtk_thermal.c | 99 +++
> 1 file changed, 76 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 0bd7aa564bc2..43c7bdbc147f 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -245,6 +245,11 @@ enum mtk_thermal_version {
>
> struct mtk_thermal;
>
> +struct mtk_thermal_zone {
> + struct mtk_thermal *mt;
> + int id;
> +};
> +
> struct thermal_bank_cfg {
> unsigned int num_sensors;
> const int *sensors;
> @@ -637,6 +642,32 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank
> *bank)
> mutex_unlock(>lock);
> }
>
> +static u32 _get_sensor_temp(struct mtk_thermal *mt, int id)
> +{
> + u32 raw;
> + int temp;
> +
> + const struct mtk_thermal_data *conf = mt->conf;
nit: You only use conf once, so I'd just use mt->conf->msr[id] below.
(or at least use conf->version instead of mt->conf->version just below)
> +
> + raw = readl(mt->thermal_base + conf->msr[id]);
> +
> + if (mt->conf->version == MTK_THERMAL_V1)
> + temp = raw_to_mcelsius_v1(mt, id, raw);
> + else
> + temp = raw_to_mcelsius_v2(mt, id, raw);
> +
> + /*
> +* The first read of a sensor often contains very high bogus
> +* temperature value. Filter these out so that the system does
> +* not immediately shut down.
> +*/
> +
> + if (temp > 20)
> + return -EAGAIN;
nit: one space between return and -EAGAIN.
> + else
> + return temp;
ditto.
> +}
> +
> /**
> * mtk_thermal_bank_temperature - get the temperature of a bank
> * @bank: The bank
> @@ -649,26 +680,10 @@ static int mtk_thermal_bank_temperature(struct
> mtk_thermal_bank *bank)
> struct mtk_thermal *mt = bank->mt;
> const struct mtk_thermal_data *conf = mt->conf;
nit: Since this is now only used once, drop this variable?
> int i, temp = INT_MIN, max = INT_MIN;
> - u32 raw;
>
> for (i = 0; i < conf->bank_data[bank->id].num_sensors; i++) {
> - raw = readl(mt->thermal_base + conf->msr[i]);
> -
> - if (mt->conf->version == MTK_THERMAL_V1) {
> - temp = raw_to_mcelsius_v1(
> - mt, conf->bank_data[bank->id].sensors[i],
> raw);
The new version of the code does this instead:
temp = raw_to_mcelsius_v1(mt, i, raw);
What's the difference between conf->bank_data[bank->id].sensors[i] and i?
> - } else {
> - temp = raw_to_mcelsius_v2(
> - mt, conf->bank_data[bank->id].sensors[i],
> raw);
> - }
>
> - /*
> -* The first read of a sensor often contains very high bogus
> -* temperature value. Filter these out so that the system does
> -* not immediately shut down.
> -*/
> - if (temp > 20)
> - temp = 0;
> + temp = _get_sensor_temp(mt, i);
>
> if (temp > max)
> max = temp;
> @@ -679,7 +694,8 @@ static int mtk_thermal_bank_temperature(struct
> mtk_thermal_bank *bank)
>
> static int mtk_read_temp(void *data, int *temperature)
> {
> - struct mtk_thermal *mt = data;
> + struct mtk_thermal_zone *tz = data;
> + struct mtk_thermal *mt = tz->mt;
> int i;
> int tempmax = INT_MIN;
>
> @@ -698,10 +714,28 @@ static int mtk_read_temp(void *data, int *temperature)
> return 0;
> }
>
> +static int mtk_read_sensor_temp(void *data, int *temperature)
> +{
> + struct mtk_thermal_zone *tz = data;
> + struct mtk_thermal *mt = tz->mt;
> + int id = tz->id - 1;
> +
> + if (id < 0)
> + return -EACCES;
nit: one space after return.
> +
> + *temperature = _get_sensor_temp(mt, id);
> +
> + return 0;
> +}
> +
> static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
> .get_temp = mtk_read_temp,
> };
>
> +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> + .get_temp = mtk_read_sensor_temp,
> +};
> +
> static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
> u32 apmixed_phys_base, u32
Re: [PATCH 2/3] soc: mediatek: pm-domains: Add domain regulator supply
On Thu, Jan 7, 2021 at 6:49 PM Hsin-Yi Wang wrote:
>
> Some power domains (eg. mfg) needs to turn on power supply before power
> on.
>
> Signed-off-by: Hsin-Yi Wang
Reviewed-by: Nicolas Boichat
> ---
> drivers/soc/mediatek/mt8183-pm-domains.h | 1 +
> drivers/soc/mediatek/mtk-pm-domains.c| 36 +++-
> drivers/soc/mediatek/mtk-pm-domains.h| 1 +
> 3 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mt8183-pm-domains.h
> b/drivers/soc/mediatek/mt8183-pm-domains.h
> index 8d996c5d2682d..aa5230e6c12f8 100644
> --- a/drivers/soc/mediatek/mt8183-pm-domains.h
> +++ b/drivers/soc/mediatek/mt8183-pm-domains.h
> @@ -38,6 +38,7 @@ static const struct scpsys_domain_data
> scpsys_domain_data_mt8183[] = {
> .ctl_offs = 0x0338,
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(12, 12),
> + .caps = MTK_SCPD_DOMAIN_SUPPLY,
> },
> [MT8183_POWER_DOMAIN_MFG_CORE0] = {
> .sta_mask = BIT(7),
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
> b/drivers/soc/mediatek/mtk-pm-domains.c
> index fb70cb3b07b36..ae255aa7b1a97 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
[snip]
> @@ -275,6 +295,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys
> *scpsys, struct device_no
> {
> const struct scpsys_domain_data *domain_data;
> struct scpsys_domain *pd;
> + struct device_node *np = scpsys->dev->of_node;
> struct property *prop;
> const char *clk_name;
> int i, ret, num_clks;
> @@ -307,6 +328,19 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys
> *scpsys, struct device_no
> pd->data = domain_data;
> pd->scpsys = scpsys;
>
> + if (MTK_SCPD_CAPS(pd, MTK_SCPD_DOMAIN_SUPPLY)) {
> + /* Find regulator in current power domain node */
> + scpsys->dev->of_node = node;
> + pd->supply = devm_regulator_get(scpsys->dev, "domain");
> + scpsys->dev->of_node = np;
This pattern is a bit strange to me. But Hsin-Yi pointed out that
there are precedents:
https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/iio/adc/rcar-gyroadc.c#L397
.
> + if (IS_ERR(pd->supply)) {
> + dev_err_probe(scpsys->dev, PTR_ERR(pd->supply),
> + "%pOF: failed to get power supply.\n",
> + node);
> + return ERR_CAST(pd->supply);
> + }
> + }
> +
> pd->infracfg = syscon_regmap_lookup_by_phandle_optional(node,
> "mediatek,infracfg");
> if (IS_ERR(pd->infracfg))
> return ERR_CAST(pd->infracfg);
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h
> b/drivers/soc/mediatek/mtk-pm-domains.h
> index a2f4d8f97e058..b2770b5266dba 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -7,6 +7,7 @@
> #define MTK_SCPD_FWAIT_SRAMBIT(1)
> #define MTK_SCPD_SRAM_ISO BIT(2)
> #define MTK_SCPD_KEEP_DEFAULT_OFF BIT(3)
> +#define MTK_SCPD_DOMAIN_SUPPLY BIT(4)
> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
>
> #define SPM_VDE_PWR_CON0x0210
> --
> 2.29.2.729.g45daf8777d-goog
>
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
[PATCH v9 2/4] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat
---
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
< CLK_TOP_MFGPLL_CK>,
< CLK_TOP_MUX_MFG>,
<>,
< CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
(based on discussions, this probably belongs in the clock core)
This only matters for devfreq, that is disabled anyway as we don't
have platform-specific code to handle >1 supplies.
(no changes since v6)
Changes in v6:
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.
Changes in v5:
- Rename "2d" power domain to "core2" (keep R-B again).
Changes in v4:
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)
Changes in v3:
- No changes
Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++
3 files changed, 117 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index cba2d8933e79..0a8c2fad8e16 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -42,6 +42,12 @@ {
status = "okay";
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index bf2ad1294dd3..00d8e112cab9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -249,6 +249,12 @@ {
proc-supply = <_vproc11_reg>;
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7..5430e05e18a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -964,6 +964,111 @@ mfgcfg: syscon@1300 {
#clock-cells = <1>;
};
+ gpu: gpu@1304 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x1304 0 0x4000>;
+ interrupts =
+ ,
+ ,
+ ;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = < CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ < MT8183_POWER_DOMAIN_MFG_CORE0>,
+ < MT8183_POWER_DOMAIN_MFG_CORE1>,
+ < MT8183_POWER_DOMAIN_MFG_2D>;
+ power-domain-names = "core0", "core1", "core2";
+
+ operating-points-v2 = <_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ opp-microvolt = <625000>, <85>;
+ };
+
+ opp-32000 {
+ opp-hz = /bits/ 64 <32000>;
+ opp-microvolt = <631250>, <85>;
+ };
+
+ opp-34000 {
+ opp-hz = /bits/ 64 <34000>;
+ opp-microvolt = <637500>, <85>;
+ };
+
+ opp-36000 {
+ opp-hz = /bits/ 64 <36000>;
+
[PATCH v9 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will
require platform-specific handling for devfreq, for 2 reasons:
1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator)
does not support multiple regulators, so we'll need custom
handlers.
2. Generally, platforms with 2 regulators have platform-specific
constraints on how the voltages should be set (e.g.
minimum/maximum voltage difference between them), so we
should not just create generic handlers that simply
change the voltages without taking care of those constraints.
Disable devfreq for now on those GPUs.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
---
Changes in v9:
- Explain why devfreq needs to be disabled for GPUs with >1
regulators.
Changes in v8:
- Use DRM_DEV_INFO instead of ERROR
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- New change
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index f44d28fad085..812cfecdee3b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
+ if (pfdev->comp->num_supplies > 1) {
+ /*
+* GPUs with more than 1 supply require platform-specific
handling:
+* continue without devfreq
+*/
+ DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n");
+ return 0;
+ }
+
opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
pfdev->comp->num_supplies);
if (IS_ERR(opp_table)) {
--
2.29.2.729.g45daf8777d-goog
[PATCH v9 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- (no changes since v6) Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..71b613ee5bd7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -87,6 +88,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.29.2.729.g45daf8777d-goog
[PATCH v9 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
---
(no changes since v7)
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.
Changes in v5:
- Change power domain name from 2d to core2.
Changes in v4:
- Add power domain names.
Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 83a461bdeea8..ca07098a6141 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = {
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2"
};
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
+ .pm_domain_names = mediatek_mt8183_pm_domains,
+};
+
static const struct of_device_id dt_match[] = {
/* Set first to probe before the generic compatibles */
{ .compatible = "amlogic,meson-gxm-mali",
@@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t860", .data = _data, },
{ .compatible = "arm,mali-t880", .data = _data, },
{ .compatible = "arm,mali-bifrost", .data = _data, },
+ { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.29.2.729.g45daf8777d-goog
[PATCH v9 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 25 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 161 insertions(+) -- 2.29.2.729.g45daf8777d-goog
Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
On Thu, Jan 7, 2021 at 11:59 PM Steven Price wrote:
>
> On 05/01/2021 00:11, Nicolas Boichat wrote:
> > GPUs with more than a single regulator (e.g. G-57 on MT8183) will
> > require platform-specific handling, disable devfreq for now.
>
> Can you explain what actually goes wrong here? AFAICT the existing code
> does support controlling multiple regulators - but clearly this is the
> first platform that exercises that code with num_supplies>1.
Sure, I should have expanded in the commit message, will do in v9.
We have support for >1 supplies, and we need to enable them to get the
GPU running _at all_ (and the default voltages should be safe by
design).
For devfreq though:
1. There are constraints on the voltage difference between the core
and SRAM, we have this caterpillar logic downstream [1], so somebody
will need to port it (TBH I don't think it's overly critical at this
point, as Bifrost support is still not very mature from what I can
see, and devfreq is purely a performance thing).
2. The core [2] does not support multiple regulators, so we'll need
custom code anyway. Even if we didn't have constraints.
[1]
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-4.19/drivers/gpu/arm/bifrost/platform/mediatek/mali_kbase_runtime_pm.c#367
[2] https://elixir.bootlin.com/linux/latest/source/drivers/opp/core.c#L679
>
> Steve
>
> >
> > Signed-off-by: Nicolas Boichat
> > ---
> >
> > Changes in v6:
> > - New change
> >
> > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > index f44d28fad085..1f49043aae73 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
> > struct thermal_cooling_device *cooling;
> > struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
> >
> > + if (pfdev->comp->num_supplies > 1) {
> > + /*
> > + * GPUs with more than 1 supply require platform-specific
> > handling:
> > + * continue without devfreq
> > + */
> > + DRM_DEV_ERROR(dev, "More than 1 supply is not supported
> > yet\n");
> > + return 0;
> > + }
> > +
> > opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
> > pfdev->comp->num_supplies);
> > if (IS_ERR(opp_table)) {
> >
>
[PATCH v8 2/4] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat
---
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
< CLK_TOP_MFGPLL_CK>,
< CLK_TOP_MUX_MFG>,
<>,
< CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
(based on discussions, this probably belongs in the clock core)
This only matters for devfreq, that is disabled anyway as we don't
have platform-specific code to handle >1 supplies.
(no changes since v6)
Changes in v6:
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.
Changes in v5:
- Rename "2d" power domain to "core2" (keep R-B again).
Changes in v4:
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)
Changes in v3:
- No changes
Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++
3 files changed, 117 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index cba2d8933e79..0a8c2fad8e16 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -42,6 +42,12 @@ {
status = "okay";
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index bf2ad1294dd3..00d8e112cab9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -249,6 +249,12 @@ {
proc-supply = <_vproc11_reg>;
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7..5430e05e18a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -964,6 +964,111 @@ mfgcfg: syscon@1300 {
#clock-cells = <1>;
};
+ gpu: gpu@1304 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x1304 0 0x4000>;
+ interrupts =
+ ,
+ ,
+ ;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = < CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ < MT8183_POWER_DOMAIN_MFG_CORE0>,
+ < MT8183_POWER_DOMAIN_MFG_CORE1>,
+ < MT8183_POWER_DOMAIN_MFG_2D>;
+ power-domain-names = "core0", "core1", "core2";
+
+ operating-points-v2 = <_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ opp-microvolt = <625000>, <85>;
+ };
+
+ opp-32000 {
+ opp-hz = /bits/ 64 <32000>;
+ opp-microvolt = <631250>, <85>;
+ };
+
+ opp-34000 {
+ opp-hz = /bits/ 64 <34000>;
+ opp-microvolt = <637500>, <85>;
+ };
+
+ opp-36000 {
+ opp-hz = /bits/ 64 <36000>;
+
[PATCH v8 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will
require platform-specific handling, disable devfreq for now.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
---
Changes in v8:
- Use DRM_DEV_INFO instead of ERROR
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- New change
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index f44d28fad085..812cfecdee3b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
+ if (pfdev->comp->num_supplies > 1) {
+ /*
+* GPUs with more than 1 supply require platform-specific
handling:
+* continue without devfreq
+*/
+ DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n");
+ return 0;
+ }
+
opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
pfdev->comp->num_supplies);
if (IS_ERR(opp_table)) {
--
2.29.2.729.g45daf8777d-goog
[PATCH v8 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost.
Signed-off-by: Nicolas Boichat
Reviewed-by: Tomeu Vizoso
---
(no changes since v7)
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.
Changes in v5:
- Change power domain name from 2d to core2.
Changes in v4:
- Add power domain names.
Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 83a461bdeea8..ca07098a6141 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = {
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2"
};
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
+ .pm_domain_names = mediatek_mt8183_pm_domains,
+};
+
static const struct of_device_id dt_match[] = {
/* Set first to probe before the generic compatibles */
{ .compatible = "amlogic,meson-gxm-mali",
@@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t860", .data = _data, },
{ .compatible = "arm,mali-t880", .data = _data, },
{ .compatible = "arm,mali-bifrost", .data = _data, },
+ { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.29.2.729.g45daf8777d-goog
[PATCH v8 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 25 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 161 insertions(+) -- 2.29.2.729.g45daf8777d-goog
[PATCH v8 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- (no changes since v6) Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..71b613ee5bd7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -87,6 +88,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.29.2.729.g45daf8777d-goog
Re: [PATCH v7 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
On Thu, Jan 7, 2021 at 4:31 PM Tomeu Vizoso wrote:
>
> On 1/7/21 9:26 AM, Nicolas Boichat wrote:
> > GPUs with more than a single regulator (e.g. G72 on MT8183) will
> > require platform-specific handling, disable devfreq for now.
> >
> > Signed-off-by: Nicolas Boichat
> > ---
> >
> > Changes in v7:
> > - Fix GPU ID in commit message
> >
> > Changes in v6:
> > - New change
> >
> > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > index f44d28fad085..1f49043aae73 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
> > struct thermal_cooling_device *cooling;
> > struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
> >
> > + if (pfdev->comp->num_supplies > 1) {
> > + /*
> > + * GPUs with more than 1 supply require platform-specific
> > handling:
> > + * continue without devfreq
> > + */
> > + DRM_DEV_ERROR(dev, "More than 1 supply is not supported
> > yet\n");
>
> Should this be info instead?
Sure, will fix in v8.
>
> Regards,
>
> Tomeu
>
> > + return 0;
> > + }
> > +
> > opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
> > pfdev->comp->num_supplies);
> > if (IS_ERR(opp_table)) {
> >
[PATCH v7 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- Changes in v7: - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..71b613ee5bd7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -87,6 +88,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.29.2.729.g45daf8777d-goog
[PATCH v7 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will
require platform-specific handling, disable devfreq for now.
Signed-off-by: Nicolas Boichat
---
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- New change
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index f44d28fad085..1f49043aae73 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
+ if (pfdev->comp->num_supplies > 1) {
+ /*
+* GPUs with more than 1 supply require platform-specific
handling:
+* continue without devfreq
+*/
+ DRM_DEV_ERROR(dev, "More than 1 supply is not supported yet\n");
+ return 0;
+ }
+
opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
pfdev->comp->num_supplies);
if (IS_ERR(opp_table)) {
--
2.29.2.729.g45daf8777d-goog
[PATCH v7 2/4] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat
---
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
< CLK_TOP_MFGPLL_CK>,
< CLK_TOP_MUX_MFG>,
<>,
< CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
(based on discussions, this probably belongs in the clock core)
This only matters for devfreq, that is disabled anyway as we don't
have platform-specific code to handle >1 supplies.
(no changes since v6)
Changes in v6:
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.
Changes in v5:
- Rename "2d" power domain to "core2" (keep R-B again).
Changes in v4:
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)
Changes in v3:
- No changes
Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++
3 files changed, 117 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index cba2d8933e79..0a8c2fad8e16 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -42,6 +42,12 @@ {
status = "okay";
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index bf2ad1294dd3..00d8e112cab9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -249,6 +249,12 @@ {
proc-supply = <_vproc11_reg>;
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7..5430e05e18a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -964,6 +964,111 @@ mfgcfg: syscon@1300 {
#clock-cells = <1>;
};
+ gpu: gpu@1304 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x1304 0 0x4000>;
+ interrupts =
+ ,
+ ,
+ ;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = < CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ < MT8183_POWER_DOMAIN_MFG_CORE0>,
+ < MT8183_POWER_DOMAIN_MFG_CORE1>,
+ < MT8183_POWER_DOMAIN_MFG_2D>;
+ power-domain-names = "core0", "core1", "core2";
+
+ operating-points-v2 = <_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ opp-microvolt = <625000>, <85>;
+ };
+
+ opp-32000 {
+ opp-hz = /bits/ 64 <32000>;
+ opp-microvolt = <631250>, <85>;
+ };
+
+ opp-34000 {
+ opp-hz = /bits/ 64 <34000>;
+ opp-microvolt = <637500>, <85>;
+ };
+
+ opp-36000 {
+ opp-hz = /bits/ 64 <36000>;
+
[PATCH v7 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost.
Signed-off-by: Nicolas Boichat
---
Changes in v7:
- Fix GPU ID in commit message
Changes in v6:
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.
Changes in v5:
- Change power domain name from 2d to core2.
Changes in v4:
- Add power domain names.
Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 83a461bdeea8..ca07098a6141 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = {
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2"
};
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
+ .pm_domain_names = mediatek_mt8183_pm_domains,
+};
+
static const struct of_device_id dt_match[] = {
/* Set first to probe before the generic compatibles */
{ .compatible = "amlogic,meson-gxm-mali",
@@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t860", .data = _data, },
{ .compatible = "arm,mali-t880", .data = _data, },
{ .compatible = "arm,mali-bifrost", .data = _data, },
+ { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.29.2.729.g45daf8777d-goog
[PATCH v7 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v7: - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 25 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 161 insertions(+) -- 2.29.2.729.g45daf8777d-goog
Re: [PATCH v2 1/2] arm64: dts: mt8183: config dsi node
On Thu, Jan 7, 2021 at 1:22 PM Hsin-Yi Wang wrote:
>
> Config dsi node for mt8183 kukui. Set panel and ports.
>
> Several kukui boards share the same panel property and only compatible
> is different. So compatible will be set in board dts for comparison
> convenience.
I like this, but maybe others have different opinions ,-)
Reviewed-by: Nicolas Boichat
> Signed-off-by: Hsin-Yi Wang
> ---
> Change:
> v2: move compatible to board dts
> ---
> .../mediatek/mt8183-kukui-krane-sku176.dts| 5 +++
> .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 37 +++
> 2 files changed, 42 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku176.dts
> b/arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku176.dts
> index 47113e275cb52..721d16f9c3b4f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku176.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku176.dts
> @@ -16,3 +16,8 @@ / {
> model = "MediaTek krane sku176 board";
> compatible = "google,krane-sku176", "google,krane", "mediatek,mt8183";
> };
> +
> + {
> +status = "okay";
> +compatible = "boe,tv101wum-nl6";
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index bf2ad1294dd30..d3d20e4773cf1 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> @@ -249,6 +249,35 @@ {
> proc-supply = <_vproc11_reg>;
> };
>
> + {
> + status = "okay";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + panel: panel@0 {
> + // compatible will be set in board dts
> + reg = <0>;
> + enable-gpios = < 45 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins_default>;
> + avdd-supply = <_lcd>;
> + avee-supply = <_lcd>;
> + pp1800-supply = <_lcd>;
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +
> + ports {
> + port {
> + dsi_out: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> +};
> +
> {
> pinctrl-names = "default";
> pinctrl-0 = <_pins>;
> @@ -547,6 +576,14 @@ pins_clk {
> };
> };
>
> + panel_pins_default: panel_pins_default {
> + panel_reset {
> + pinmux = ;
> + output-low;
> + bias-pull-up;
> + };
> + };
> +
> pwm0_pin_default: pwm0_pin_default {
> pins1 {
> pinmux = ;
> --
> 2.29.2.729.g45daf8777d-goog
>
Re: [PATCH 1/2] arm64: dts: mt8183: config dsi node
On Wed, Jan 6, 2021 at 6:47 PM Nicolas Boichat wrote:
>
> On Wed, Jan 6, 2021 at 4:46 PM Hsin-Yi Wang wrote:
> >
> > Config dsi node for mt8183 kukui. Set panel and ports.
> >
> > Signed-off-by: Hsin-Yi Wang
> > ---
> > .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 38 +++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > index bf2ad1294dd30..4cfb3560e5d11 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > @@ -249,6 +249,36 @@ {
> > proc-supply = <_vproc11_reg>;
> > };
> >
> > + {
> > + status = "okay";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + panel: panel@0 {
> > + compatible = "boe,tv101wum-nl6";
>
> We're going to have many panels in the kukui family, so I think I'd
> prefer it if you moved the compatible string to krane-sku0 dts: it
> makes it easier to figure out what's different with sku0.
I meant sku176 (since you are adding sku0 with a different panel
compatible later).
>
> Then maybe leave all the other properties in this file, as it seems
> like all MIPI panels use the exact same pin/supplies?
>
> (And add a comment here saying that the compatible needs to be set in
> board dts?)
>
> > + reg = <0>;
> > + enable-gpios = < 45 0>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <_pins_default>;
> > + avdd-supply = <_lcd>;
> > + avee-supply = <_lcd>;
> > + pp1800-supply = <_lcd>;
> > + status = "okay";
> > + port {
> > + panel_in: endpoint {
> > + remote-endpoint = <_out>;
> > + };
> > + };
> > + };
> > +
> > + ports {
> > + port {
> > + dsi_out: endpoint {
> > + remote-endpoint = <_in>;
> > + };
> > + };
> > + };
> > +};
> > +
> > {
> > pinctrl-names = "default";
> > pinctrl-0 = <_pins>;
> > @@ -547,6 +577,14 @@ pins_clk {
> > };
> > };
> >
> > + panel_pins_default: panel_pins_default {
> > + panel_reset {
> > + pinmux = ;
> > + output-low;
> > + bias-pull-up;
> > + };
> > + };
> > +
> > pwm0_pin_default: pwm0_pin_default {
> > pins1 {
> > pinmux = ;
> > --
> > 2.29.2.729.g45daf8777d-goog
> >
Re: [PATCH 1/2] arm64: dts: mt8183: config dsi node
On Wed, Jan 6, 2021 at 4:46 PM Hsin-Yi Wang wrote:
>
> Config dsi node for mt8183 kukui. Set panel and ports.
>
> Signed-off-by: Hsin-Yi Wang
> ---
> .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 38 +++
> 1 file changed, 38 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index bf2ad1294dd30..4cfb3560e5d11 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> @@ -249,6 +249,36 @@ {
> proc-supply = <_vproc11_reg>;
> };
>
> + {
> + status = "okay";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + panel: panel@0 {
> + compatible = "boe,tv101wum-nl6";
We're going to have many panels in the kukui family, so I think I'd
prefer it if you moved the compatible string to krane-sku0 dts: it
makes it easier to figure out what's different with sku0.
Then maybe leave all the other properties in this file, as it seems
like all MIPI panels use the exact same pin/supplies?
(And add a comment here saying that the compatible needs to be set in
board dts?)
> + reg = <0>;
> + enable-gpios = < 45 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins_default>;
> + avdd-supply = <_lcd>;
> + avee-supply = <_lcd>;
> + pp1800-supply = <_lcd>;
> + status = "okay";
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +
> + ports {
> + port {
> + dsi_out: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> +};
> +
> {
> pinctrl-names = "default";
> pinctrl-0 = <_pins>;
> @@ -547,6 +577,14 @@ pins_clk {
> };
> };
>
> + panel_pins_default: panel_pins_default {
> + panel_reset {
> + pinmux = ;
> + output-low;
> + bias-pull-up;
> + };
> + };
> +
> pwm0_pin_default: pwm0_pin_default {
> pins1 {
> pinmux = ;
> --
> 2.29.2.729.g45daf8777d-goog
>
Re: [PATCH v10 3/7] [v10, 3/7]: soc: mediatek: SVS: introduce MTK SVS engine
On Wed, Jan 6, 2021 at 4:41 PM Roger Lu wrote:
>
> Hi Nicolas,
>
> [snip]
> > >
> > > > +
> > > > + /* Svs efuse parsing */
> > > > + ft_pgm = (svsp->efuse[0] >> 4) & GENMASK(3, 0);
> > > > +
> > > > + for (idx = 0; idx < svsp->bank_num; idx++) {
> > > > + svsb = >banks[idx];
> > > > +
> > > > + if (ft_pgm <= 1)
> > > > + svsb->init01_volt_flag =
> > > > SVSB_INIT01_VOLT_IGNORE;
> > > > +
> > > > + switch (svsb->sw_id) {
> > > > + case SVSB_CPU_LITTLE:
> > > > + svsb->bdes = svsp->efuse[16] & GENMASK(7, 0);
> > > > + svsb->mdes = (svsp->efuse[16] >> 8) &
> > > > GENMASK(7, 0);
> > > > + svsb->dcbdet = (svsp->efuse[16] >> 16) &
> > > > GENMASK(7, 0);
> > > > + svsb->dcmdet = (svsp->efuse[16] >> 24) &
> > > > GENMASK(7, 0);
> > > > + svsb->mtdes = (svsp->efuse[17] >> 16) &
> > > > GENMASK(7, 0);
> > >
> > > Again, if all of those values were u8, there'd be no need for these
> > > GENMASK
> >
> > Ok, I'll use u8 instead of GENMASK. Thanks.
>
> After refining the codes, I think it's much explicit to assign the bits
> I want by GENMASK() and will remove other GENMASK() that are repetitive
> like in svs_set_bank_phase() or svs_set_freqs_pct_v2().
I'm not sure what you mean, but, sure, you can go ahead with v11 and
we'll see how it looks.
Thanks,
> [snip]
>
Re: [PATCH v5 3/4] spmi: mediatek: Add support for MT6873/8192
On Wed, Dec 23, 2020 at 10:45 AM Hsin-Hsiung Wang
wrote:
>
> Add spmi support for MT6873/8192.
>
> Signed-off-by: Hsin-Hsiung Wang
> ---
> drivers/spmi/Kconfig | 9 +
> drivers/spmi/Makefile| 1 +
> drivers/spmi/spmi-mtk-pmif.c | 504 +++
> 3 files changed, 514 insertions(+)
> create mode 100644 drivers/spmi/spmi-mtk-pmif.c
>
> diff --git a/drivers/spmi/Kconfig b/drivers/spmi/Kconfig
> index a53bad541f1a..418848840999 100644
> --- a/drivers/spmi/Kconfig
> +++ b/drivers/spmi/Kconfig
> @@ -25,4 +25,13 @@ config SPMI_MSM_PMIC_ARB
> This is required for communicating with Qualcomm PMICs and
> other devices that have the SPMI interface.
>
> +config SPMI_MTK_PMIF
> + tristate "Mediatek SPMI Controller (PMIC Arbiter)"
> + help
> + If you say yes to this option, support will be included for the
> + built-in SPMI PMIC Arbiter interface on Mediatek family
> + processors.
> +
> + This is required for communicating with Mediatek PMICs and
> + other devices that have the SPMI interface.
> endif
> diff --git a/drivers/spmi/Makefile b/drivers/spmi/Makefile
> index 55a94cadeffe..91f303b96925 100644
> --- a/drivers/spmi/Makefile
> +++ b/drivers/spmi/Makefile
> @@ -5,3 +5,4 @@
> obj-$(CONFIG_SPMI) += spmi.o
>
> obj-$(CONFIG_SPMI_MSM_PMIC_ARB)+= spmi-pmic-arb.o
> +obj-$(CONFIG_SPMI_MTK_PMIF)+= spmi-mtk-pmif.o
> \ No newline at end of file
Newline at end of file please.
> diff --git a/drivers/spmi/spmi-mtk-pmif.c b/drivers/spmi/spmi-mtk-pmif.c
> new file mode 100644
> index ..711d3973800b
> --- /dev/null
> +++ b/drivers/spmi/spmi-mtk-pmif.c
> @@ -0,0 +1,504 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2020 MediaTek Inc.
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define SWINF_IDLE 0x00
> +#define SWINF_WFVLDCLR 0x06
> +
> +#define GET_SWINF(x) (((x) >> 1) & 0x7)
> +
> +#define PMIF_CMD_REG_0 0
> +#define PMIF_CMD_REG 1
> +#define PMIF_CMD_EXT_REG 2
> +#define PMIF_CMD_EXT_REG_LONG 3
> +
> +#define PMIF_DELAY_US 10
> +#define PMIF_TIMEOUT(10 * 1000)
PMIF_TIMEOUT_US
> +
> +#define PMIF_CHAN_OFFSET 0x5
> +
> +#define SPMI_OP_ST_BUSY 1
> +
> +struct ch_reg {
> + u32 ch_sta;
> + u32 wdata;
> + u32 rdata;
> + u32 ch_send;
> + u32 ch_rdy;
> +};
> +
> +struct pmif {
> + void __iomem*base;
> + const u32 *regs;
> + void __iomem*spmimst_base;
> + const u32 *spmimst_regs;
> + u32 soc_chan;
> + int grpid;
Drop this, never used.
> + raw_spinlock_t lock;
> + struct clk *pmif_sys_ck;
> + struct clk *pmif_tmr_ck;
> + struct clk *spmimst_clk_mux;
> + struct ch_reg chan;
> +};
> +
> +enum pmif_regs {
> + PMIF_INIT_DONE,
> + PMIF_INF_EN,
> + PMIF_ARB_EN,
> + PMIF_CMDISSUE_EN,
> + PMIF_TIMER_CTRL,
> + PMIF_SPI_MODE_CTRL,
> + PMIF_IRQ_EVENT_EN_0,
> + PMIF_IRQ_FLAG_0,
> + PMIF_IRQ_CLR_0,
> + PMIF_IRQ_EVENT_EN_1,
> + PMIF_IRQ_FLAG_1,
> + PMIF_IRQ_CLR_1,
> + PMIF_IRQ_EVENT_EN_2,
> + PMIF_IRQ_FLAG_2,
> + PMIF_IRQ_CLR_2,
> + PMIF_IRQ_EVENT_EN_3,
> + PMIF_IRQ_FLAG_3,
> + PMIF_IRQ_CLR_3,
> + PMIF_IRQ_EVENT_EN_4,
> + PMIF_IRQ_FLAG_4,
> + PMIF_IRQ_CLR_4,
> + PMIF_WDT_EVENT_EN_0,
> + PMIF_WDT_FLAG_0,
> + PMIF_WDT_EVENT_EN_1,
> + PMIF_WDT_FLAG_1,
> + PMIF_SWINF_0_STA,
> + PMIF_SWINF_0_WDATA_31_0,
> + PMIF_SWINF_0_RDATA_31_0,
> + PMIF_SWINF_0_ACC,
> + PMIF_SWINF_0_VLD_CLR,
> + PMIF_SWINF_1_STA,
> + PMIF_SWINF_1_WDATA_31_0,
> + PMIF_SWINF_1_RDATA_31_0,
> + PMIF_SWINF_1_ACC,
> + PMIF_SWINF_1_VLD_CLR,
> + PMIF_SWINF_2_STA,
> + PMIF_SWINF_2_WDATA_31_0,
> + PMIF_SWINF_2_RDATA_31_0,
> + PMIF_SWINF_2_ACC,
> + PMIF_SWINF_2_VLD_CLR,
> + PMIF_SWINF_3_STA,
> + PMIF_SWINF_3_WDATA_31_0,
> + PMIF_SWINF_3_RDATA_31_0,
> + PMIF_SWINF_3_ACC,
> + PMIF_SWINF_3_VLD_CLR,
> +};
> +
> +static const u32 mt6873_regs[] = {
> + [PMIF_INIT_DONE] = 0x,
> + [PMIF_INF_EN] = 0x0024,
> + [PMIF_ARB_EN] = 0x0150,
> + [PMIF_CMDISSUE_EN] =0x03B4,
> + [PMIF_TIMER_CTRL] = 0x03E0,
> + [PMIF_SPI_MODE_CTRL] = 0x0400,
> + [PMIF_IRQ_EVENT_EN_0] = 0x0418,
> + [PMIF_IRQ_FLAG_0] = 0x0420,
> + [PMIF_IRQ_CLR_0] = 0x0424,
> + [PMIF_IRQ_EVENT_EN_1] = 0x0428,
> + [PMIF_IRQ_FLAG_1] = 0x0430,
> + [PMIF_IRQ_CLR_1] = 0x0434,
> + [PMIF_IRQ_EVENT_EN_2] = 0x0438,
> + [PMIF_IRQ_FLAG_2] = 0x0440,
> + [PMIF_IRQ_CLR_2] = 0x0444,
> + [PMIF_IRQ_EVENT_EN_3] = 0x0448,
> + [PMIF_IRQ_FLAG_3] =
Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
On Tue, Jan 5, 2021 at 8:34 AM Alyssa Rosenzweig wrote: > > > GPUs with more than a single regulator (e.g. G-57 on MT8183) will > > G72 Duh, sorry, yes. I will fix that in v7.
[PATCH v6 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G-57 Bifrost.
Signed-off-by: Nicolas Boichat
---
Changes in v6:
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.
Changes in v5:
- Change power domain name from 2d to core2.
Changes in v4:
- Add power domain names.
Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 83a461bdeea8..ca07098a6141 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = {
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2"
};
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
+ .pm_domain_names = mediatek_mt8183_pm_domains,
+};
+
static const struct of_device_id dt_match[] = {
/* Set first to probe before the generic compatibles */
{ .compatible = "amlogic,meson-gxm-mali",
@@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t860", .data = _data, },
{ .compatible = "arm,mali-t880", .data = _data, },
{ .compatible = "arm,mali-bifrost", .data = _data, },
+ { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.29.2.729.g45daf8777d-goog
[PATCH v6 2/4] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat
---
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
< CLK_TOP_MFGPLL_CK>,
< CLK_TOP_MUX_MFG>,
<>,
< CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
(based on discussions, this probably belongs in the clock core)
This only matters for devfreq, that is disabled anyway as we don't
have platform-specific code to handle >1 supplies.
Changes in v6:
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.
Changes in v5:
- Rename "2d" power domain to "core2" (keep R-B again).
Changes in v4:
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)
Changes in v3:
- No changes
Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++
3 files changed, 117 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index cba2d8933e79..0a8c2fad8e16 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -42,6 +42,12 @@ {
status = "okay";
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index bf2ad1294dd3..00d8e112cab9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -249,6 +249,12 @@ {
proc-supply = <_vproc11_reg>;
};
+ {
+ supply-names = "mali", "sram";
+ mali-supply = <_vgpu_reg>;
+ sram-supply = <_vsram_gpu_reg>;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7..5430e05e18a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -964,6 +964,111 @@ mfgcfg: syscon@1300 {
#clock-cells = <1>;
};
+ gpu: gpu@1304 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x1304 0 0x4000>;
+ interrupts =
+ ,
+ ,
+ ;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = < CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ < MT8183_POWER_DOMAIN_MFG_CORE0>,
+ < MT8183_POWER_DOMAIN_MFG_CORE1>,
+ < MT8183_POWER_DOMAIN_MFG_2D>;
+ power-domain-names = "core0", "core1", "core2";
+
+ operating-points-v2 = <_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ opp-microvolt = <625000>, <85>;
+ };
+
+ opp-32000 {
+ opp-hz = /bits/ 64 <32000>;
+ opp-microvolt = <631250>, <85>;
+ };
+
+ opp-34000 {
+ opp-hz = /bits/ 64 <34000>;
+ opp-microvolt = <637500>, <85>;
+ };
+
+ opp-36000 {
+ opp-hz = /bits/ 64 <36000>;
+ opp-microvolt
[PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G-57 on MT8183) will
require platform-specific handling, disable devfreq for now.
Signed-off-by: Nicolas Boichat
---
Changes in v6:
- New change
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index f44d28fad085..1f49043aae73 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = >pfdevfreq;
+ if (pfdev->comp->num_supplies > 1) {
+ /*
+* GPUs with more than 1 supply require platform-specific
handling:
+* continue without devfreq
+*/
+ DRM_DEV_ERROR(dev, "More than 1 supply is not supported yet\n");
+ return 0;
+ }
+
opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
pfdev->comp->num_supplies);
if (IS_ERR(opp_table)) {
--
2.29.2.729.g45daf8777d-goog
[PATCH v6 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..71b613ee5bd7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -87,6 +88,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.29.2.729.g45daf8777d-goog
