Re: [PATCH v15 4/7] drm/bridge: dw-hdmi: repair interworking with hdmi-connector for jz4780

2022-02-21 Thread Neil Armstrong

Hi,

On 12/02/2022 16:50, H. Nikolaus Schaller wrote:

Commit 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts 
callbacks")

introduced a new mechanism to negotiate bus formats between hdmi connector
and the synopsys hdmi driver inside the jz4780.

By this, the dw-hdmi is no longer the only bridge and sets up a list
of formats in dw_hdmi_bridge_atomic_get_output_bus_fmts().

This includes MEDIA_BUS_FMT_UYVY8_1X16 which is chosen for the jz4780 but only
produces a black screen.

This fix is based on the observation that max_bpc = 0 when running this
function while info->bpc = 8. Since the formats checks before this always test
for max_bpc >= info->pbc indirectly my assumption is that we must check it
here as well.

Adding the proposed patch makes the CI20/jz4780 panel work again in
MEDIA_BUS_FMT_RGB888_1X24 mode.

Fixes: 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts 
callbacks")
Signed-off-by: H. Nikolaus Schaller 
---
  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index b0d8110dd412c..826a055a7a273 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2620,10 +2620,10 @@ static u32 
*dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
}
  
-	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)

+   if (max_bpc >= info->bpc && info->color_formats & 
DRM_COLOR_FORMAT_YCBCR422)
output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
  
-	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)

+   if (max_bpc >= info->bpc && info->color_formats & 
DRM_COLOR_FORMAT_YCBCR444)
output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
  
  	/* Default 8bit RGB fallback */


Please do the same for all other cases and change the patch subject to 
something more accurate like:
"drm/bridge: dw-hdmi: take display info bpc in account for bus formats 
negociation"

Neil


Re: [PATCH v16 21/40] pwm: tegra: Add runtime PM and OPP support

2022-02-21 Thread Uwe Kleine-König
Hello,

On Wed, Dec 01, 2021 at 02:23:28AM +0300, Dmitry Osipenko wrote:
> The PWM on Tegra belongs to the core power domain and we're going to
> enable GENPD support for the core domain. Now PWM must be resumed using
> runtime PM API in order to initialize the PWM power state. The PWM clock
> rate must be changed using OPP API that will reconfigure the power domain
> performance state in accordance to the rate. Add runtime PM and OPP
> support to the PWM driver.
> 
> Reviewed-by: Ulf Hansson 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/pwm/pwm-tegra.c | 82 -
>  1 file changed, 64 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 11a10b575ace..18cf974ac776 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -42,12 +42,16 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> +#include 
> +
>  #define PWM_ENABLE   (1 << 31)
>  #define PWM_DUTY_WIDTH   8
>  #define PWM_DUTY_SHIFT   16
> @@ -145,7 +149,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct 
> pwm_device *pwm,
>   required_clk_rate =
>   (NSEC_PER_SEC / period_ns) << PWM_DUTY_WIDTH;
>  
> - err = clk_set_rate(pc->clk, required_clk_rate);
> + err = dev_pm_opp_set_rate(pc->dev, required_clk_rate);
>   if (err < 0)
>   return -EINVAL;
>  
> @@ -181,8 +185,8 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct 
> pwm_device *pwm,
>* before writing the register. Otherwise, keep it enabled.
>*/
>   if (!pwm_is_enabled(pwm)) {
> - err = clk_prepare_enable(pc->clk);
> - if (err < 0)
> + err = pm_runtime_resume_and_get(pc->dev);
> + if (err)
>   return err;
>   } else
>   val |= PWM_ENABLE;
> @@ -193,7 +197,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct 
> pwm_device *pwm,
>* If the PWM is not enabled, turn the clock off again to save power.
>*/
>   if (!pwm_is_enabled(pwm))
> - clk_disable_unprepare(pc->clk);
> + pm_runtime_put(pc->dev);
>  
>   return 0;
>  }
> @@ -204,8 +208,8 @@ static int tegra_pwm_enable(struct pwm_chip *chip, struct 
> pwm_device *pwm)
>   int rc = 0;
>   u32 val;
>  
> - rc = clk_prepare_enable(pc->clk);
> - if (rc < 0)
> + rc = pm_runtime_resume_and_get(pc->dev);
> + if (rc)
>   return rc;
>  
>   val = pwm_readl(pc, pwm->hwpwm);
> @@ -224,7 +228,7 @@ static void tegra_pwm_disable(struct pwm_chip *chip, 
> struct pwm_device *pwm)
>   val &= ~PWM_ENABLE;
>   pwm_writel(pc, pwm->hwpwm, val);
>  
> - clk_disable_unprepare(pc->clk);
> + pm_runtime_put_sync(pc->dev);
>  }
>  
>  static const struct pwm_ops tegra_pwm_ops = {
> @@ -256,11 +260,20 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>   if (IS_ERR(pwm->clk))
>   return PTR_ERR(pwm->clk);
>  
> + ret = devm_tegra_core_dev_init_opp_table_common(&pdev->dev);
> + if (ret)
> + return ret;
> +
> + pm_runtime_enable(&pdev->dev);
> + ret = pm_runtime_resume_and_get(&pdev->dev);
> + if (ret)
> + return ret;
> +
>   /* Set maximum frequency of the IP */
> - ret = clk_set_rate(pwm->clk, pwm->soc->max_frequency);
> + ret = dev_pm_opp_set_rate(pwm->dev, pwm->soc->max_frequency);
>   if (ret < 0) {
>   dev_err(&pdev->dev, "Failed to set max frequency: %d\n", ret);
> - return ret;
> + goto put_pm;
>   }
>  
>   /*
> @@ -278,7 +291,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>   if (IS_ERR(pwm->rst)) {
>   ret = PTR_ERR(pwm->rst);
>   dev_err(&pdev->dev, "Reset control is not found: %d\n", ret);
> - return ret;
> + goto put_pm;
>   }
>  
>   reset_control_deassert(pwm->rst);
> @@ -291,10 +304,16 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>   if (ret < 0) {
>   dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
>   reset_control_assert(pwm->rst);
> - return ret;
> + goto put_pm;
>   }
>  
> + pm_runtime_put(&pdev->dev);
> +
>   return 0;
> +put_pm:
> + pm_runtime_put_sync_suspend(&pdev->dev);
> + pm_runtime_force_suspend(&pdev->dev);
> + return ret;
>  }
>  
>  static int tegra_pwm_remove(struct platform_device *pdev)
> @@ -305,20 +324,44 @@ static int tegra_pwm_remove(struct platform_device 
> *pdev)
>  
>   reset_control_assert(pc->rst);
>  
> + pm_runtime_force_suspend(&pdev->dev);
> +
>   return 0;
>  }
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int tegra_pwm_suspend(struct device *dev)
> +static int __maybe_unused tegra_pwm_runtime_susp

Re: [PATCH] drm/amdgpu: check vm bo eviction valuable at last

2022-02-21 Thread Christian König

Am 21.02.22 um 04:28 schrieb Qiang Yu:

On Fri, Feb 18, 2022 at 6:24 PM Christian König
 wrote:

Am 18.02.22 um 11:16 schrieb Qiang Yu:

[SNIP]

If amdgpu_vm_ready() use evicting flag, it's still not equivalent to check
vm idle: true -> vm idle, false -> vm may be idle or busy.

Yeah, but why should that be relevant?

The amdgpu_vm_ready() return if we can do page table updates or not. If
the VM is idle or not is only relevant for eviction.

In other words any CS or page table update makes the VM busy, but that
only affects if the VM can be evicted or not.


My point is: we can't use amdgpu_vm_ready() to replace vm_is_busy(), so
currently we update vm even when vm is busy. So why not use:

Sorry, should be "vm is idle".


if (!amdgpu_vm_ready() || vm_is_busy()) return;
in amdgpu_gem_va_update_vm(), as you mentioned we prefer to not
update vm when it's idle.

Because updating the VM while it is busy is perfectly fine, we do it all
the time.


Yeah, as above, my typo.


We should just not update it when it is already idle and was considered
for eviction.

"and", not "or"?


In this situation it makes most of the time sense to keep
it idle and postpone the update till the next command submission.


Then we can keep the evicting flag accurate (after solving your
concern for this patch that eviction may fail latter by further delay
the flag update after eviction success).

That won't work. See we need to mark the VM as evicted before we
actually evict them because otherwise somebody could use the VM in
parallel and add another fence to it.


I see, make this too accurate should cost too much like holding the
eviction_lock when eviction. But just delay it in
amdgpu_ttm_bo_eviction_valuable()
could avoid most false positive case.

Partially correct. Another fundamental problem is that we can't hold the
eviction lock because that would result in lock inversion and potential
deadlock.

We could set the flag later on, but as I said before that when we set
the evicted flag when the VM is already idle is a desired effect.


As above, this confuse me as we can explicitly check vm idle when
user update vm, why bother to embed it in evicting flag implicitly?

Well as I said it's irrelevant for the update if the VM is idle or not.

To summarize the rules once more:
1. When VM page tables are used by CS or page tables updates it is
considered busy, e.g. not idle.

2. When we want to evict a VM it must be idle. As soon as we considered
this we should set the evicted flag to make sure to keep it idle as much
as possible.

3. When we want to update the page tables we just need to check if the
VM is idle or not.


But now we does not check vm idle directly in amdgpu_gem_va_update_vm().
If VM bo has not been considered for eviction, it could be either idle or busy.

Just want to confirm if the fix should be only change amdgpu_vm_ready()
to use evicting flag or besides using evicting flag, also check vm_idle() in
amdgpu_gem_va_update_vm().


Only changing the amdgpu_vm_ready() should be enough. It can be that 
this then bubbles up more issue, but those need to be taken care of 
separately then.


Regards,
Christian.



Regards,
Qiang


4. When a CS happens we don't have another chance and make the VM busy
again. And do all postponed page table updates.


Anyway,


Regards,
Christian.


Check vm idle need to hold resv lock. Read your patch for adding
evicting flag is to update vm without resv lock. But user vm ops in
amdgpu_gem_va_update_vm() do hold the resv lock, so the difference
happens when calling amdgpu_vm_bo_update_mapping() from
svm_range_(un)map_to_gpu(). So embed vm idle in evicting flag
is for svm_range_(un)map_to_gpu() also do nothing when vm idle?




Regards,
Qiang


Regards,
Christian.


Regards,
Qiang


Regards,
Christian.


Regards,
Qiang



Regards,
Christian.


Regards,
Qiang


Regards,
Christian.


Regards,
Qiang


Regards,
Christian.


Regards,
Qiang


What we should rather do is to fix amdgpu_vm_ready() to take a look at
the flag instead of the linked list.

Regards,
Christian.


Signed-off-by: Qiang Yu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 85 ++---
 1 file changed, 47 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 5a32ee66d8c8..88a27911054f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1306,45 +1306,11 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device 
*adev, struct ttm_tt *ttm,
 return flags;
 }

-/*
- * amdgpu_ttm_bo_eviction_valuable - Check to see if we can evict a buffer
- * object.
- *
- * Return true if eviction is sensible. Called by ttm_mem_evict_first() on
- * behalf of ttm_bo_mem_force_space() which tries to evict buffer objects until
- * it can find space for a new object and by ttm_bo_force_list_clean() which is
- * used to clean out a memory space.
- */
-static bool amdgpu_ttm_b

Re: Regression from 3c196f056666 ("drm/amdgpu: always reset the asic in suspend (v2)") on suspend?

2022-02-21 Thread Eric Valette

On 20/02/2022 16:48, Dominique Dumont wrote:

On Monday, 14 February 2022 22:52:27 CET Alex Deucher wrote:

Does the system actually suspend?


Not really. The screens looks like it's going to suspend, but it does come
back after 10s or so. The light mounted in the middle of the power button does
not switch off.



As I have a very similar problem and also commented on the original 
debian bug report 
(https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005005), I will add 
some information here on another amd only laptop (renoir AMD Ryzen 7 
4800H with Radeon Graphics + Radeon RX 5500/5500M / Pro 5500M).


For me the suspend works once, but after the first resume (I do know 
know if it is in the suspend path or the resume path I see a RIP in the 
dmesg (see aditional info in debian bug))  and later suspend do not 
work: It only go to the kde login screen.


I was unable due to network connectivity to do a full bisect but tested 
with the patch I had on my laptop:


5.10.101 works, 5.10 from debian works
5.11 works
5.12 works
5.13 suspend works but when resuming the PC is dead I have to reboot
5.14 seems to work but looking at dmesg it is full of RIP messages at 
various places.

5.15.24 is a described 5.15 from debian is behaving identically
5.16 from debian is behaving identically.


Is this system S0i3 or regular S3?


For me it is real S3.

The proposed patch is intended for INTEl + intel gpu + amdgpu but I have 
dual amd GPU.


--eric




Re: Regression from 3c196f056666 ("drm/amdgpu: always reset the asic in suspend (v2)") on suspend?

2022-02-21 Thread Dominique Dumont
On Monday, 14 February 2022 22:52:27 CET Alex Deucher wrote:
> Does the system actually suspend?  

Not really. The screens looks like it's going to suspend, but it does come 
back after 10s or so. The light mounted in the middle of the power button does 
not switch off.

> Is this system S0i3 or regular S3?

I'm not sure how to check that. After a bit of reading on the Internet [1], I 
hope that the following information answers that question. Please get back to 
me if that's not the case.

Looks like my system supports both Soi3 and S3

$ cat /sys/power/state 
freeze mem disk

I get the same result running these 2 commands as root:
# echo freeze > /sys/power/state
# echo mem > /sys/power/state

>  Does this patch help by any chance?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i
> d=e55a3aea418269266d84f426b3bd70794d3389c8

yes, with this patch:
- the suspend issue is solved
- kernel logs no longer show messages like "failed to send message" or 
"*ERROR* suspend of IP block  failed" while suspending

All the best

[1] https://01.org/blogs/rzhang/2015/best-practice-debug-linux-suspend/
hibernate-issues




Re: [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()

2022-02-21 Thread Boris Brezillon
Hello Christoph,

On Sat, 19 Feb 2022 09:28:44 +
Christoph Niedermaier  wrote:

> From: Max Krummenacher [mailto:max.oss...@gmail.com]
> Sent: Wednesday, February 9, 2022 10:38 AM
> >> If display timings were read from the devicetree using
> >> of_get_display_timing() and pixelclk-active is defined
> >> there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is
> >> automatically generated. Through the function
> >> drm_bus_flags_from_videomode() e.g. called in the
> >> panel-simple driver this flag got into the bus flags,
> >> but then in imx_pd_bridge_atomic_check() the bus flag
> >> check failed and will not initialize the display. The
> >> original commit fe141cedc433 does not explain why this
> >> check was introduced. So remove the bus flags check,
> >> because it stops the initialization of the display with
> >> valid bus flags.
> >>
> >> Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by the 
> >> bridge when available")
> >> Signed-off-by: Christoph Niedermaier 
> >> Cc: Marek Vasut 
> >> Cc: Boris Brezillon 
> >> Cc: Philipp Zabel 
> >> Cc: David Airlie 
> >> Cc: Daniel Vetter 
> >> Cc: Shawn Guo 
> >> Cc: Sascha Hauer 
> >> Cc: Pengutronix Kernel Team 
> >> Cc: Fabio Estevam 
> >> Cc: NXP Linux Team 
> >> Cc: linux-arm-ker...@lists.infradead.org
> >> To: dri-devel@lists.freedesktop.org
> >> ---
> >> V2: - Add Boris to the Cc list
> >> ---
> >>  drivers/gpu/drm/imx/parallel-display.c | 8 
> >>  1 file changed, 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/imx/parallel-display.c 
> >> b/drivers/gpu/drm/imx/parallel-display.c
> >> index a8aba0141ce7..06cb1a59b9bc 100644
> >> --- a/drivers/gpu/drm/imx/parallel-display.c
> >> +++ b/drivers/gpu/drm/imx/parallel-display.c
> >> @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct 
> >> drm_bridge *bridge,
> >>   if (!imx_pd_format_supported(bus_fmt))
> >>   return -EINVAL;
> >>
> >> - if (bus_flags &
> >> - ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH |
> >> -   DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> >> -   DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) {
> >> - dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", bus_flags);
> >> - return -EINVAL;
> >> - }
> >> -
> >>   bridge_state->output_bus_cfg.flags = bus_flags;
> >>   bridge_state->input_bus_cfg.flags = bus_flags;
> >>   imx_crtc_state->bus_flags = bus_flags;  
> > 
> > Tested on a Colibri iMX6DL with a panel-dpi based panel.
> > 
> > Tested-by: Max Krummenacher   
> 
> I still ask myself why this bus flag check is in the code.
> Is there a reason not to remove that?

The reasoning was that DE_{LOW,HIGH} and
FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE were the only bus_flags taken into
account by the crtc logic, so anything else is simply ignored. This was
definitely wrong since the driver supports at least one of the VSYNC
polarity (perhaps both if there's a way to configure it that's not
hooked-up yet).

So I guess figuring out the default VSYNC polarity and accepting the
according DISPLAY_FLAGS_SYNC_XXXEDGE flag is what makes most sense here.

Regards,

Boris


Aw: [PATCH v6 17/23] arm64: dts: rockchip: rk356x: Add HDMI nodes

2022-02-21 Thread Frank Wunderlich
Hi

> Gesendet: Donnerstag, 17. Februar 2022 um 09:29 Uhr
> Von: "Sascha Hauer" 

> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -10,7 +10,6 @@
>  #include 
>  #include 
>  #include 
> -#include 

why dropping this after adding in part 16?

>  #include 

it looks like you moved this to board includes...imho this should stay in the 
rk356x.dtsi, because compilation will fail if a board without the vop2 (and 
missing the include) is derived from rk356x.dtsi.

regards Frank


Re: [PATCH] drm/bridge: anx7625: switch to devm_drm_of_get_bridge

2022-02-21 Thread Maxime Ripard
On Mon, Feb 21, 2022 at 08:28:35AM +0100, José Expósito wrote:
> The function "drm_of_find_panel_or_bridge" has been deprecated in
> favor of "devm_drm_of_get_bridge".
> 
> Switch to the new function and reduce boilerplate.
> 
> Signed-off-by: José Expósito 

Reviewed-by: Maxime Ripard 

Maxime


signature.asc
Description: PGP signature


Re: [PATCH] drm/msm/dp: switch to devm_drm_of_get_bridge

2022-02-21 Thread Maxime Ripard
On Mon, Feb 21, 2022 at 08:33:39AM +0100, José Expósito wrote:
> The function "drm_of_find_panel_or_bridge" has been deprecated in
> favor of "devm_drm_of_get_bridge".
> 
> Switch to the new function and reduce boilerplate.
> 
> Signed-off-by: José Expósito 

Reviewed-by: Maxime Ripard 

Maxime


signature.asc
Description: PGP signature


Re: [PATCH] drm: rcar-du: switch to devm_drm_of_get_bridge

2022-02-21 Thread Maxime Ripard
On Mon, Feb 21, 2022 at 08:37:57AM +0100, José Expósito wrote:
> The function "drm_of_find_panel_or_bridge" has been deprecated in
> favor of "devm_drm_of_get_bridge".
> 
> Switch to the new function and reduce boilerplate.
> 
> Signed-off-by: José Expósito 
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 16 +---
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 72a272cfc11e..99b0febc56d1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -712,18 +712,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  {
>   int ret;
>  
> - ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0,
> -   &lvds->panel, &lvds->next_bridge);

I guess lvds->panel can be removed from the rcar_lvds struct as well?

Maxime


signature.asc
Description: PGP signature


Re: [PATCH] drm/bridge: chipone-icn6211: switch to devm_drm_of_get_bridge

2022-02-21 Thread Maxime Ripard
On Mon, Feb 21, 2022 at 08:42:24AM +0100, José Expósito wrote:
> The function "drm_of_find_panel_or_bridge" has been deprecated in
> favor of "devm_drm_of_get_bridge".
> 
> Switch to the new function and reduce boilerplate.
> 
> Signed-off-by: José Expósito 

Reviewed-by: Maxime Ripard 

Maxime


signature.asc
Description: PGP signature


Re: [PATCH V2 04/11] drm/bridge: tc358767: Implement atomic_check callback

2022-02-21 Thread Maxime Ripard
On Sat, Feb 19, 2022 at 03:26:40AM +0100, Marek Vasut wrote:
> On 2/18/22 18:34, Lucas Stach wrote:
> 
> Hi
> 
> [...]
> 
> > >   drivers/gpu/drm/bridge/tc358767.c | 26 ++
> > >   1 file changed, 26 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/tc358767.c 
> > > b/drivers/gpu/drm/bridge/tc358767.c
> > > index 522c2c4d8514f..01d11adee6c74 100644
> > > --- a/drivers/gpu/drm/bridge/tc358767.c
> > > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > > @@ -1289,6 +1289,31 @@ static bool tc_bridge_mode_fixup(struct drm_bridge 
> > > *bridge,
> > >   return true;
> > >   }
> > > +static int tc_edp_common_atomic_check(struct drm_bridge *bridge,
> > 
> > Drop the edp in the name here? Later in the series you call this
> > function from the DPI code, so this breaks the nice clean naming
> > separation from patch 1.
> > 
> > > +   struct drm_bridge_state *bridge_state,
> > > +   struct drm_crtc_state *crtc_state,
> > > +   struct drm_connector_state *conn_state,
> > > +   const unsigned int max_khz)
> > > +{
> > > + tc_bridge_mode_fixup(bridge, &crtc_state->mode,
> > > +  &crtc_state->adjusted_mode);
> > > +
> > > + if (crtc_state->adjusted_mode.clock > max_khz)
> > > + crtc_state->adjusted_mode.clock = max_khz;
> > 
> > I don't think this is correct. The adjusted most is just for minor
> > adjustments if the bridge can not fully match the mode. If userspace
> > supplies a invalid high modeclock I think it would be better to fail
> > the atomic check -> return -EINVAL
> 
> Maxime was telling me that returning -EINVAL from atomic_check is weird, so
> maybe we should also wait for his opinion on this part.

That was in a completely different context?

Our discussion was about how you would propagate clock constraints
across a pipeline, and I was telling you that it would be weird to
return -EINVAL for a mode that was reported on a connector as supported
(or even preferred).

My argument was for mode_valid to filter them out.

If your clock is way above what you can support on your device, then
returning an error in atomic_check is the right thing to do.

Maxime


signature.asc
Description: PGP signature


Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver

2022-02-21 Thread Maxime Ripard
On Sun, Feb 20, 2022 at 10:32:25PM +0100, Sam Ravnborg wrote:
> Hi Noralf,
> 
> On Sun, Feb 20, 2022 at 04:59:34PM +0100, Noralf Trønnes wrote:
> > 
> > 
> > Den 19.02.2022 23.10, skrev Sam Ravnborg:
> > > Hi Noralf,
> > > On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote:
> > >> Add a driver that will work with most MIPI DBI compatible SPI panels.
> > >> This avoids adding a driver for every new MIPI DBI compatible controller
> > >> that is to be used by Linux. The 'compatible' Device Tree property with
> > >> a '.bin' suffix will be used to load a firmware file that contains the
> > >> controller configuration.
> > > A solution where we have the command sequences as part of the DT-overlay
> > > is IMO a much better way to solve this:
> > > - The users needs to deal only with a single file, so there is less that
> > >   goes wrong
> > > - The user need not reading special instructions how to handle a .bin
> > >   file, if the overlay is present everything is fine
> > > - The file that contains the command sequences can be properly annotated
> > >   with comments
> > > - The people that creates the command sequences has no need for a special
> > >   script to create the file for a display - this is all readable ascii.
> > > - Using a DT-overlay the parsing of the DT-overlay can be done by
> > >   well-tested functions, no need to invent something new to parse the
> > >   file
> > > 
> > > 
> > > The idea with a common mipi DBI SPI driver is super, it is the detail
> > > with the .bin file that I am against.
> > > 
> > 
> > The fbtft drivers has an init= DT property that contains the command
> > sequence. Example for the MZ61581 display:
> > 
> > init = <0x1b0 00
> > 0x111
> > 0x2ff
> > 0x1b3 0x02 0x00 0x00 0x00
> > 0x1c0 0x13 0x3b 0x00 0x02 0x00 0x01 
> > 0x00 0x43
> > 0x1c1 0x08 0x16 0x08 0x08
> > 0x1c4 0x11 0x07 0x03 0x03
> > 0x1c6 0x00
> > 0x1c8 0x03 0x03 0x13 0x5c 0x03 0x07 
> > 0x14 0x08 0x00 0x21 0x08
> > 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00
> > 0x135 0x00
> > 0x136 0xa0
> > 0x13a 0x55
> > 0x144 0x00 0x01
> > 0x1d0 0x07 0x07 0x1d 0x03
> > 0x1d1 0x03 0x30 0x10
> > 0x1d2 0x03 0x14 0x04
> > 0x129
> > 0x12c>;
> > 
> > Parsed here:
> > https://elixir.bootlin.com/linux/latest/C/ident/fbtft_init_display_from_property
> > 
> > Before fbdev was closed for new drivers I looked at fixing up fbtft for
> > proper inclusion and asked on the Device Tree mailinglist about the init
> > property and how to handle the controller configuration generically.
> > I was politely told that this should be done in the driver, so when I
> > made my first DRM driver I made a driver specifically for a panel
> > (mi0283qt.c).
> > 
> > Later I found another post that in clear words stated that setting
> > random registers from DT was not acceptable.
>
> Understood and thanks for the explanation.
> It is a shame that the users loose here :-(

From a user point-of-view, nothing prevents the overlays and the
firmware to be in the same package, provided by whatever distro or
build-system they would use.

The only case where it's a bit less efficient is for a panel that isn't
supported already, but it's just a documentation and tooling issue, and
Noralf has an awesome track record for both.

And the DT syntax throw so much people off that I'm not sure it's such a
benefit :)

Maxime


signature.asc
Description: PGP signature


Re: [PATCH V2 05/11] drm/bridge: tc358767: Move hardware init to enable callback

2022-02-21 Thread Lucas Stach
Am Samstag, dem 19.02.2022 um 03:39 +0100 schrieb Marek Vasut:
> On 2/18/22 18:49, Lucas Stach wrote:
> > Am Freitag, dem 18.02.2022 um 02:00 +0100 schrieb Marek Vasut:
> > > The TC358767/TC358867/TC9595 are all capable of operating either from
> > > attached Xtal or from DSI clock lane clock. In case the later is used,
> > > all I2C accesses will fail until the DSI clock lane is running and
> > > supplying clock to the chip.
> > > 
> > > Move all hardware initialization to enable callback to guarantee the
> > > DSI clock lane is running before accessing the hardware. In case Xtal
> > > is attached to the chip, this change has no effect.
> > 
> > I'm not sure if that last statement is correct. When the chip is
> > bridging to eDP, the aux channel and HPD handling is needed to be
> > functional way before the atomic enable happen. I have no idea how this
> > would interact with the clock supplied from the DSI lanes. Maybe it
> > doesn't work at all and proper eDP support always needs a external
> > reference clock?
> 
> The driver currently assumes the TC358767 always gets RefClk from Xtal.
> 
> There is subsequent series which adds support for deriving clock which 
> drive the TC358767 PLLs from DSI HS clock instead of Xtal in case the 
> bridge operates in DSI-to-DPI mode. That needs additional plumbing, as 
> the TC358767 must be able to select specific clock frequency on the DSI 
> HS clock lane, because its PLLs need specific frequencies, see:
> 
> [RFC][PATCH 0/7] drm/bridge: Add support for selecting DSI host HS clock 
> from DSI bridge
> 
> If someone needs to implement DSI-to-(e)DP mode without Xtal, ugh, that 
> would likely need to have a way to figure out the DSI HS clock frequency 
> already in probe and then enable those DSI HS clock very early on too ?
> 
Probably, but that was really just something I wondered about while
passing by.

The real issue is that I think _this_ patch breaks eDP operation, as
now the chip is initialized way too late, as the AUX channel
functionality needs to be available long before the atomic bridge
enable callback is called. The AUX channel is used to fetch the display
EDID, so before you can even set a mode it needs to be functional.
Unconditionally moving the chip init is probably (I haven't tested it
yet) going to break this.

Regards,
Lucas

> > I think we should make the "ref" clock a optional clock to properly
> > describe the fact that the chip can operate without this clock in DSI
> > input mode and then either do the chip init in the probe routine when
> > the ref clock is present, or defer it to atomic enable when the ref
> > clock is absent.
> 
> See the RFC patchset above, that patchset does exactly that, it makes 
> RefClk optional.
> 
> [...]




Re: [PATCH v4 7/9] drm: vkms: Refactor the plane composer to accept new formats

2022-02-21 Thread Pekka Paalanen
On Sun, 20 Feb 2022 22:02:12 -0300
Igor Torrente  wrote:

> Hi Melissa,
> 
> On 2/9/22 18:45, Melissa Wen wrote:
> > On 02/08, Igor Torrente wrote:  
> >> Hi Melissa,
> >>
> >> On 2/8/22 07:40, Melissa Wen wrote:  
> >>> On 01/21, Igor Torrente wrote:  
>  Currently the blend function only accepts XRGB_ and ARGB_
>  as a color input.
> 
>  This patch refactors all the functions related to the plane composition
>  to overcome this limitation.
> 
>  A new internal format(`struct pixel`) is introduced to deal with all
>  possible inputs. It consists of 16 bits fields that represent each of
>  the channels.
> 
>  The pixels blend is done using this internal format. And new handlers
>  are being added to convert a specific format to/from this internal 
>  format.
> 
>  So the blend operation depends on these handlers to convert to this 
>  common
>  format. The blended result, if necessary, is converted to the writeback
>  buffer format.
> 
>  This patch introduces three major differences to the blend function.
>  1 - All the planes are blended at once.
>  2 - The blend calculus is done as per line instead of per pixel.
>  3 - It is responsible to calculates the CRC and writing the writeback
>    buffer(if necessary).
> 
>  These changes allow us to allocate way less memory in the intermediate
>  buffer to compute these operations. Because now we don't need to
>  have the entire intermediate image lines at once, just one line is
>  enough.
> 
>  | Memory consumption (output dimensions) |
>  |:--:|
>  |   Current  | This patch|
>  |:--:|:-:|
>  |   Width * Heigth   | 2 * Width |
> 
>  Beyond memory, we also have a minor performance benefit from all
>  these changes. Results running the IGT tests `*kms_cursor_crc*`:
>   
> >>> First, thanks for this improvement.
> >>>
> >>> Some recent changes in kms_cursor_crc caused VKMS to fail in most test
> >>> cases (iirc, only size-change and alpha-opaque are passing currently).  
> >>
> >> I updated my igt and kernel(from drm_misc/drm-misc-next) to the latest
> >> commit[1][2] and I'm getting mixed results. Sometimes most of the test
> >> passes, sometimes almost nothing passes.  
> > hmm.. is it happening when running kms_cursor_crc? Is the results
> > variation random or is it possible to follow a set of steps to reproduce
> > it? When failing, what is the reason displayed by the log?  
> 
> I investigated it a little bit and discovered that the KMS
> cursor(".*kms_cursor_crc*" ) are failing after the execution of
> writeback tests(".*kms_writeback.*").
> 
> I don't know what is causing it, but they are failing while trying to 
> commit the KMS changes.
> 
> out.txt:
> IGT-Version: 1.26-NO-GIT (x86_64) (Linux: 5.17.0-rc2 x86_64)
> Stack trace:
>#0 ../lib/igt_core.c:1754 __igt_fail_assert()
>#1 ../lib/igt_kms.c:3795 do_display_commit()
>#2 ../lib/igt_kms.c:3901 igt_display_commit2()
>#3 ../tests/kms_cursor_crc.c:820 __igt_uniquereal_main814()
>#4 ../tests/kms_cursor_crc.c:814 main()
>#5 ../csu/libc-start.c:308 __libc_start_main()
>#6 [_start+0x2a]
> Subtest pipe-A-cursor-size-change: FAIL
> 
> err.txt:
> (kms_cursor_crc:1936) igt_kms-CRITICAL: Test assertion failure function 
> do_display_commit, file ../lib/igt_kms.c:3795:
> (kms_cursor_crc:1936) igt_kms-CRITICAL: Failed assertion: ret == 0
> (kms_cursor_crc:1936) igt_kms-CRITICAL: Last errno: 22, Invalid argument
> (kms_cursor_crc:1936) igt_kms-CRITICAL: error: -22 != 0
> 
> > 
> >  From my side, only the first two subtest of kms_cursor_crc is passing
> > before this patch. And after your changes here, all subtests are
> > successful again, except those related to 32x10 cursor size (that needs
> > futher investigation). I didn't check how the recent changes in
> > kms_cursor_crc affect VKMS performance on it, but I bet that clearing
> > the alpha channel is the reason to have the performance back.  
> 
> Yeah, I also don't understand why the 32x10 cursor tests are failing.
> 

Hi,

are the tests putting the cursor partially outside of the CRTC area?
Or partially outside of primary plane area (which IIRC you used when you
should have used the CRTC area?)?

Does the writeback test forget to unlink the writeback connector? Or
does VKMS not handle unlinking the writeback connector?

If both are a problem, the latter would be just an unrelated bug that
exposes the first bug in VKMS, because whether writeback is used or not
probably should not affect where the cursor plane is allowed to be.


Thanks,
pq


pgpsMKjczohKx.pgp
Description: OpenPGP digital signature


Re: [PATCH v10 1/4] MIPS: Loongson64: dts: update the display controller device node

2022-02-21 Thread Sergei Shtylyov
On 2/20/22 5:55 PM, Sui Jingfeng wrote:

> From: suijingfeng 
> 
> The display controller is a pci device, its PCI vendor id is 0x0014
> its PCI device id is 0x7a06.
> 
> 1) In order to let the driver to know which chip the DC is contained
>in, the compatible string of the display controller is updated
>according to the chip's name.
> 
> 2) Add display controller device node for ls2k1000 SoC
> 
> Reported-by: Krzysztof Kozlowski 
> Signed-off-by: suijingfeng 
> Signed-off-by: Sui Jingfeng <15330273...@189.cn>
> ---
>  arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 8 
>  arch/mips/boot/dts/loongson/ls7a-pch.dtsi  | 7 ++-
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi 
> b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
> index 768cf2abcea3..af9cda540f9e 100644
> --- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
> +++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
> @@ -209,6 +209,14 @@ gpu@5,0 {
>   interrupt-parent = <&liointc0>;
>   };
>  
> + lsdc: display-controller@6,0 {

   Shouldn't the node name just be "display", according to the section 2.2.2
of the DT spec?

[...]
> diff --git a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi 
> b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
> index 2f45fce2cdc4..ec35ea9b2fe8 100644
> --- a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
> +++ b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
> @@ -160,11 +160,8 @@ gpu@6,0 {
>   interrupt-parent = <&pic>;
>   };
>  
> - dc@6,1 {
> - compatible = "pci0014,7a06.0",
> -"pci0014,7a06",
> -"pciclass03",
> -"pciclass0300";
> + lsdc: display-controller@6,1 {

   Same here...

[...]

MBR, Sergey


Re: [PULL] drm-intel-gt-next

2022-02-21 Thread Jani Nikula
On Mon, 21 Feb 2022, Dave Airlie  wrote:
> On Thu, 17 Feb 2022 at 20:26, Joonas Lahtinen
>  wrote:
>>
>> Hi Dave & Daniel,
>>
>> Here is the first drm-intel-gt-next feature PR towards v5.18.
>
> Am I missing some previous drm-intel pull?
>
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_workarounds.c:
> In function ‘rcs_engine_wa_init’:
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_workarounds.c:2051:40:
> error: ‘XEHP_DIS_BBL_SYSPIPE’ undeclared (first use in this function)
>  2051 |   wa_masked_en(wal, GEN9_ROW_CHICKEN4, XEHP_DIS_BBL_SYSPIPE);
>   |^~~~
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_workarounds.c:2051:40:
> note: each undeclared identifier is reported only once for each
> function it appears in

There's apparently a silent conflict between changes in drm-intel-next
and drm-intel-gt-next. There's a fixup patch in drm-rerere:
fixups/drm-intel-gt-next.patch.

We opted to sync the branches via drm-next pulls and backmerges, but I'm
afraid that means you'd have to use the fixups when merging. I guess we
failed to communicate that. The alternative would've been cross-merges
within drm-intel.


BR,
Jani.


>
> Dave.
>>
>> For DG2 adds subplatform G12, missing workarounds and fixes GuC
>> loading on ARM64. C0/D0 stepping info added for RPL-S.
>>
>> For uAPI enables support for simple parallel submission with
>> execlists which was previously enabled only for GuC.
>>
>> Further fixes for PMU metrics when GuC is enabled, better error
>> reporting for GuC loading failures. Fix for PXP unbind splat.
>> Updates to GuC version 69.0.3 with improvements to GT reset
>> scenarios.
>>
>> The rest is mostly refactoring of the memory management code,
>> as highlights introduction of async unbinding/migration and
>> removal of short-term pinning in execbuf.
>>
>> Then a few selftest and coding style fixes.
>>
>> Regards, Joonas
>>
>> ***
>>
>> drm-intel-gt-next-2022-02-17:
>>
>> UAPI Changes:
>>
>> - Weak parallel submission support for execlists
>>
>>   Minimal implementation of the parallel submission support for
>>   execlists backend that was previously only implemented for GuC.
>>   Support one sibling non-virtual engine.
>>
>> Core Changes:
>>
>> - Two backmerges of drm/drm-next for header file renames/changes and
>>   i915_regs reorganization
>>
>> Driver Changes:
>>
>> - Add new DG2 subplatform: DG2-G12 (Matt R)
>> - Add new DG2 workarounds (Matt R, Ram, Bruce)
>> - Handle pre-programmed WOPCM registers for DG2+ (Daniele)
>> - Update guc shim control programming on XeHP SDV+ (Daniele)
>> - Add RPL-S C0/D0 stepping information (Anusha)
>> - Improve GuC ADS initialization to work on ARM64 on dGFX (Lucas)
>>
>> - Fix KMD and GuC race on accessing PMU busyness (Umesh)
>> - Use PM timestamp instead of RING TIMESTAMP for reference in PMU with GuC 
>> (Umesh)
>> - Report error on invalid reset notification from GuC (John)
>> - Avoid WARN splat by holding RPM wakelock during PXP unbind (Juston)
>> - Fixes to parallel submission implementation (Matt B.)
>> - Improve GuC loading status check/error reports (John)
>> - Tweak TTM LRU priority hint selection (Matt A.)
>> - Align the plane_vma to min_page_size of stolen mem (Ram)
>>
>> - Introduce vma resources and implement async unbinding (Thomas)
>> - Use struct vma_resource instead of struct vma_snapshot (Thomas)
>> - Return some TTM accel move errors instead of trying memcpy move (Thomas)
>> - Fix a race between vma / object destruction and unbinding (Thomas)
>> - Remove short-term pins from execbuf (Maarten)
>> - Update to GuC version 69.0.3 (John, Michal Wa.)
>> - Improvements to GT reset paths in GuC backend (Matt B.)
>> - Use shrinker_release_pages instead of writeback in shmem object hooks 
>> (Matt A., Tvrtko)
>> - Use trylock instead of blocking lock when freeing GEM objects (Maarten)
>> - Allocate intel_engine_coredump_alloc with ALLOW_FAIL (Matt B.)
>> - Fixes to object unmapping and purging (Matt A)
>> - Check for wedged device in GuC backend (John)
>> - Avoid lockdep splat by locking dpt_obj around set_cache_level (Maarten)
>> - Allow dead vm to unbind vma's without lock (Maarten)
>> - s/engine->i915/i915/ for DG2 engine workarounds (Matt R)
>>
>> - Use to_gt() helper for GGTT accesses (Michal Wi.)
>> - Selftest improvements (Matt B., Thomas, Ram)
>> - Coding style and compiler warning fixes (Matt B., Jasmine, Andi, Colin, 
>> Gustavo, Dan)
>>
>> The following changes since commit 53dbee4926d3706ca9e03f3928fa85b5ec3bc0cc:
>>
>>   Merge tag 'drm-misc-next-2022-01-27' of 
>> git://anongit.freedesktop.org/drm/drm-misc into drm-next (2022-02-01 
>> 19:02:41 +1000)
>>
>> are available in the Git repository at:
>>
>>   git://anongit.freedesktop.org/drm/drm-intel 
>> tags/drm-intel-gt-next-2022-02-17
>>
>> for you to fetch changes up to 154cfae6158141b18d65abb0db679bb51a8294e7:
>>
>>   drm/i915/dg2: Add Wa_22011100796 (2022-02-11 17:11:44 

Re: [PATCH v8 13/19] drm/mediatek: dpi: Add dpintf support

2022-02-21 Thread Maxime Ripard
Hi

(Now I remember your series ;)

On Fri, Feb 18, 2022 at 03:54:31PM +0100, Guillaume Ranquet wrote:
> dpintf is the displayport interface hardware unit. This unit is similar
> to dpi and can reuse most of the code.
> 
> This patch adds support for mt8195-dpintf to this dpi driver. Main
> differences are:
>  - Some features/functional components are not available for dpintf
>which are now excluded from code execution once is_dpintf is set
>  - dpintf can and needs to choose between different clockdividers based
>on the clockspeed. This is done by choosing a different clock parent.
>  - There are two additional clocks that need to be managed. These are
>only set for dpintf and will be set to NULL if not supplied. The
>clk_* calls handle these as normal clocks then.
>  - Some register contents differ slightly between the two components. To
>work around this I added register bits/masks with a DPINTF_ prefix
>and use them where different.
> 
> Based on a separate driver for dpintf created by
> Jason-JH.Lin .
> 
> Signed-off-by: Markus Schneider-Pargmann 
> Signed-off-by: Guillaume Ranquet 
> ---
>  drivers/gpu/drm/mediatek/mtk_dpi.c  | 164 +---
>  drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  38 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   8 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c  |   5 +-
>  include/linux/soc/mediatek/mtk-mmsys.h  |   2 +
>  6 files changed, 198 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index be99399faf1bb..c5639ada868f8 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -63,6 +63,14 @@ enum mtk_dpi_out_color_format {
>   MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL
>  };
>  
> +enum TVDPLL_CLK {
> + TVDPLL_PLL = 0,
> + TVDPLL_D2 = 2,
> + TVDPLL_D4 = 4,
> + TVDPLL_D8 = 8,
> + TVDPLL_D16 = 16,
> +};
> +
>  struct mtk_dpi {
>   struct drm_encoder encoder;
>   struct drm_bridge bridge;
> @@ -71,8 +79,10 @@ struct mtk_dpi {
>   void __iomem *regs;
>   struct device *dev;
>   struct clk *engine_clk;
> + struct clk *dpi_ck_cg;
>   struct clk *pixel_clk;
>   struct clk *tvd_clk;
> + struct clk *pclk_src[5];
>   int irq;
>   struct drm_display_mode mode;
>   const struct mtk_dpi_conf *conf;
> @@ -126,6 +136,7 @@ struct mtk_dpi_conf {
>   const u32 *output_fmts;
>   u32 num_output_fmts;
>   bool is_ck_de_pol;
> + bool is_dpintf;
>   bool swap_input_support;
>   // Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift)
>   u32 dimension_mask;
> @@ -384,6 +395,25 @@ static void mtk_dpi_config_disable_edge(struct mtk_dpi 
> *dpi)
>   mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN);
>  }
>  
> +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi, enum 
> mtk_dpi_out_color_format format)
> +{
> + u32 matrix_sel = 0;
> +
> + switch (format) {
> + case MTK_DPI_COLOR_FORMAT_YCBCR_422:
> + case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
> + case MTK_DPI_COLOR_FORMAT_YCBCR_444:
> + case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:
> + case MTK_DPI_COLOR_FORMAT_XV_YCC:
> + if (dpi->mode.hdisplay <= 720)
> + matrix_sel = 0x2;
> + break;
> + default:
> + break;
> + }
> + mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel, INT_MATRIX_SEL_MASK);
> +}
> +
>  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
>   enum mtk_dpi_out_color_format format)
>  {
> @@ -391,6 +421,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi 
> *dpi,
>   (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
>   mtk_dpi_config_yuv422_enable(dpi, false);
>   mtk_dpi_config_csc_enable(dpi, true);
> + mtk_dpi_matrix_sel(dpi, format);
>   if (dpi->conf->swap_input_support)
>   mtk_dpi_config_swap_input(dpi, false);
>   mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_BGR);
> @@ -398,6 +429,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi 
> *dpi,
>  (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
>   mtk_dpi_config_yuv422_enable(dpi, true);
>   mtk_dpi_config_csc_enable(dpi, true);
> + mtk_dpi_matrix_sel(dpi, format);
>   if (dpi->conf->swap_input_support)
>   mtk_dpi_config_swap_input(dpi, true);
>   mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_RGB);
> @@ -438,6 +470,8 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi)
>   mtk_dpi_disable(dpi);
>   clk_disable_unprepare(dpi->pixel_clk);
>   clk_disable_unprepare(dpi->engine_clk);
> + clk_disable_unprepare(dpi->dpi_ck_cg);
> + clk_disable_unprepa

Re: [PATCH v3] simplefb: Enable boot time VESA graphic mode selection.

2022-02-21 Thread Thomas Zimmermann

Hi

Am 18.02.22 um 17:04 schrieb Michal Suchanek:

Since switch to simplefb/simpledrm VESA graphic modes are no longer
available with legacy BIOS.

The x86 realmode boot code enables the VESA graphic modes when option
FB_BOOT_VESA_SUPPORT is enabled.

To enable use of VESA modes with simplefb in legacy BIOS boot mode drop
dependency of BOOT_VESA_SUPPORT on FB, also drop the FB_ prefix, and
select the option when simplefb enabled on x86.

The BOOT_VESA_SUPPORT is not specific to framebuffer but rather to x86
platform, move it from fbdev to x86 Kconfig.

Fixes: e3263ab389a7 ("x86: provide platform-devices for boot-framebuffers")
Signed-off-by: Michal Suchanek 


Acked-by: Thomas Zimmermann 

Thanks for the patch. I'll wait a bit for additional reviews before 
merging it.


Best regards
Thomas


---
v2: Select BOOT_VESA_SUPPORT from simplefb rather than simpledrm. The
simpledrm driver uses the firmware provided video modes only indirectly
through simplefb, and both can be enabled independently.
v3: Move BOOT_VESA_SUPPORT from fbdev to x86
---
  arch/x86/Kconfig|  6 ++
  arch/x86/boot/video-vesa.c  |  4 ++--
  drivers/firmware/Kconfig|  1 +
  drivers/video/fbdev/Kconfig | 13 +++--
  4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9f5bd41bf660..cceb1aab0486 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -942,6 +942,12 @@ config GART_IOMMU
  
  	  If unsure, say Y.
  
+config BOOT_VESA_SUPPORT

+   bool
+   help
+ If true, at least one selected framebuffer driver can take advantage
+ of VESA video modes set at an early boot stage via the vga= parameter.
+
  config MAXSMP
bool "Enable Maximum number of SMP Processors and NUMA Nodes"
depends on X86_64 && SMP && DEBUG_KERNEL
diff --git a/arch/x86/boot/video-vesa.c b/arch/x86/boot/video-vesa.c
index 7e185977a984..c2c6d35e3a43 100644
--- a/arch/x86/boot/video-vesa.c
+++ b/arch/x86/boot/video-vesa.c
@@ -83,7 +83,7 @@ static int vesa_probe(void)
   (vminfo.memory_layout == 4 ||
vminfo.memory_layout == 6) &&
   vminfo.memory_planes == 1) {
-#ifdef CONFIG_FB_BOOT_VESA_SUPPORT
+#ifdef CONFIG_BOOT_VESA_SUPPORT
/* Graphics mode, color, linear frame buffer
   supported.  Only register the mode if
   if framebuffer is configured, however,
@@ -121,7 +121,7 @@ static int vesa_set_mode(struct mode_info *mode)
if ((vminfo.mode_attr & 0x15) == 0x05) {
/* It's a supported text mode */
is_graphic = 0;
-#ifdef CONFIG_FB_BOOT_VESA_SUPPORT
+#ifdef CONFIG_BOOT_VESA_SUPPORT
} else if ((vminfo.mode_attr & 0x99) == 0x99) {
/* It's a graphics mode with linear frame buffer */
is_graphic = 1;
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 75cb91055c17..ad64f3a6f54f 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -224,6 +224,7 @@ config SYSFB
  config SYSFB_SIMPLEFB
bool "Mark VGA/VBE/EFI FB as generic system framebuffer"
depends on SYSFB
+   select BOOT_VESA_SUPPORT
help
  Firmwares often provide initial graphics framebuffers so the BIOS,
  bootloader or kernel can show basic video-output during boot for
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 6ed5e608dd04..5bdd303b5268 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -66,13 +66,6 @@ config FB_DDC
select I2C_ALGOBIT
select I2C
  
-config FB_BOOT_VESA_SUPPORT

-   bool
-   depends on FB
-   help
- If true, at least one selected framebuffer driver can take advantage
- of VESA video modes set at an early boot stage via the vga= parameter.
-
  config FB_CFB_FILLRECT
tristate
depends on FB
@@ -627,7 +620,7 @@ config FB_VESA
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
-   select FB_BOOT_VESA_SUPPORT
+   select BOOT_VESA_SUPPORT
help
  This is the frame buffer device driver for generic VESA 2.0
  compliant graphic cards. The older VESA 1.2 cards are not supported.
@@ -1051,7 +1044,7 @@ config FB_INTEL
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
-   select FB_BOOT_VESA_SUPPORT if FB_INTEL = y
+   select BOOT_VESA_SUPPORT if FB_INTEL = y
depends on !DRM_I915
help
  This driver supports the on-board graphics built in to the Intel
@@ -1378,7 +1371,7 @@ config FB_SIS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
-   select FB_BOOT_VESA_SUPPORT if FB_SIS = y
+   select BOOT_VESA_SUPPORT if FB_SIS = y
select FB_SIS_300 if !FB_SIS_315
help
  This

Re: [PATCH v6 17/23] arm64: dts: rockchip: rk356x: Add HDMI nodes

2022-02-21 Thread Sascha Hauer
On Mon, Feb 21, 2022 at 09:54:28AM +0100, Frank Wunderlich wrote:
> Hi
> 
> > Gesendet: Donnerstag, 17. Februar 2022 um 09:29 Uhr
> > Von: "Sascha Hauer" 
> 
> > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > @@ -10,7 +10,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> 
> why dropping this after adding in part 16?
> 
> >  #include 
> 
> it looks like you moved this to board includes...imho this should stay
> in the rk356x.dtsi, because compilation will fail if a board without
> the vop2 (and missing the include) is derived from rk356x.dtsi.

I dropped adding the include from Patch 16. The include is not needed by
rk356x.dtsi. When a board without vop2 support is added then it won't
need the include either.

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


[PATCH v1 7/8] ARM: dts: imx6dl-victgo: The TGO uses a lg, lb070wv8 compatible 7" display

2022-02-21 Thread Oleksij Rempel
From: Robin van der Gracht 

This series of devices is using lg,lb070wv8 instead of kyo,tcg121xglp.

Signed-off-by: Robin van der Gracht 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-victgo.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts 
b/arch/arm/boot/dts/imx6dl-victgo.dts
index 907682248aa7..7839021bc3eb 100644
--- a/arch/arm/boot/dts/imx6dl-victgo.dts
+++ b/arch/arm/boot/dts/imx6dl-victgo.dts
@@ -129,7 +129,7 @@ led-2 {
};
 
panel {
-   compatible = "kyo,tcg121xglp";
+   compatible = "lg,lb070wv8";
backlight = <&backlight_lcd>;
power-supply = <®_3v3>;
 
-- 
2.30.2



[PATCH v1 6/8] ARM: dts: imx6dl-victgo: Add interrupt-counter nodes

2022-02-21 Thread Oleksij Rempel
From: Robin van der Gracht 

Interrupt counter is mainlined, now we can add missing counter nodes.

Signed-off-by: Robin van der Gracht 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-victgo.dts | 41 -
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts 
b/arch/arm/boot/dts/imx6dl-victgo.dts
index 20c7f80e5ec9..907682248aa7 100644
--- a/arch/arm/boot/dts/imx6dl-victgo.dts
+++ b/arch/arm/boot/dts/imx6dl-victgo.dts
@@ -54,6 +54,27 @@ comp0_out: endpoint {
};
};
 
+   counter-0 {
+   compatible = "interrupt-counter";
+   pinctrl-names = "default";
+   pinctrl-0 = <&pinctrl_counter0>;
+   gpios = <&gpio2 0 GPIO_ACTIVE_LOW>;
+   };
+
+   counter-1 {
+   compatible = "interrupt-counter";
+   pinctrl-names = "default";
+   pinctrl-0 = <&pinctrl_counter1>;
+   gpios = <&gpio2 1 GPIO_ACTIVE_LOW>;
+   };
+
+   counter-2 {
+   compatible = "interrupt-counter";
+   pinctrl-names = "default";
+   pinctrl-0 = <&pinctrl_counter2>;
+   gpios = <&gpio2 2 GPIO_ACTIVE_LOW>;
+   };
+
gpio-keys {
compatible = "gpio-keys";
pinctrl-names = "default";
@@ -400,7 +421,7 @@ &gpio1 {
 
 &gpio2 {
gpio-line-names =
-   "", "", "", "", "", "", "", "",
+   "YACO_WHEEL", "YACO_RADAR", "YACO_PTO", "", "", "", "", "",
"", "LED_PWM", "", "", "",
"", "", "",
"", "", "", "", "", "", "ISB_IN1", "ON_SWITCH",
@@ -706,6 +727,24 @@ MX6QDL_PAD_KEY_ROW3__GPIO4_IO130x13008
>;
};
 
+   pinctrl_counter0: counter0grp {
+   fsl,pins = <
+   MX6QDL_PAD_NANDF_D0__GPIO2_IO00 0x1b000
+   >;
+   };
+
+   pinctrl_counter1: counter1grp {
+   fsl,pins = <
+   MX6QDL_PAD_NANDF_D1__GPIO2_IO01 0x1b000
+   >;
+   };
+
+   pinctrl_counter2: counter2grp {
+   fsl,pins = <
+   MX6QDL_PAD_NANDF_D2__GPIO2_IO02 0x1b000
+   >;
+   };
+
pinctrl_ecspi1: ecspi1grp {
fsl,pins = <
MX6QDL_PAD_EIM_D17__ECSPI1_MISO 0x100b1
-- 
2.30.2



[PATCH v1 1/8] ARM: dts: imx6qdl-vicut1/vicutgo: Set default backlight brightness to maximum

2022-02-21 Thread Oleksij Rempel
From: David Jander 

Recover default behavior of the device and set maximal brightness

Signed-off-by: David Jander 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-victgo.dts   | 2 +-
 arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts 
b/arch/arm/boot/dts/imx6dl-victgo.dts
index 227c952543d4..e6134efbfabd 100644
--- a/arch/arm/boot/dts/imx6dl-victgo.dts
+++ b/arch/arm/boot/dts/imx6dl-victgo.dts
@@ -28,7 +28,7 @@ backlight: backlight {
pwms = <&pwm1 0 500 0>;
brightness-levels = <0 16 64 255>;
num-interpolated-steps = <16>;
-   default-brightness-level = <1>;
+   default-brightness-level = <48>;
power-supply = <®_3v3>;
enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;
};
diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi 
b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
index 1ac7e13249d2..c1d06bc28c67 100644
--- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
@@ -23,7 +23,7 @@ backlight: backlight {
pwms = <&pwm1 0 500 0>;
brightness-levels = <0 16 64 255>;
num-interpolated-steps = <16>;
-   default-brightness-level = <1>;
+   default-brightness-level = <48>;
power-supply = <®_3v3>;
enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;
};
-- 
2.30.2



[PATCH v1 5/8] ARM: dts: imx6qdl-vicut1/vicutgo: The sgtl5000 uses i2s not ac97

2022-02-21 Thread Oleksij Rempel
From: Robin van der Gracht 

According to Documentation/devicetree/bindings/sound/fsl,ssi.txt
'fsl,mode' should be specified for AC97 mode only.

The 'fsl,ssi' documentation doesn't say anything about specifying
'sound-dai-cells' so we'll remove that as well.

Signed-off-by: Robin van der Gracht 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-victgo.dts   | 2 --
 arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 2 --
 2 files changed, 4 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts 
b/arch/arm/boot/dts/imx6dl-victgo.dts
index d542ddad4e32..20c7f80e5ec9 100644
--- a/arch/arm/boot/dts/imx6dl-victgo.dts
+++ b/arch/arm/boot/dts/imx6dl-victgo.dts
@@ -591,8 +591,6 @@ &pwm3 {
 };
 
 &ssi1 {
-   #sound-dai-cells = <0>;
-   fsl,mode = "ac97-slave";
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi 
b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
index ec39008c0950..97ef8264947a 100644
--- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
@@ -466,8 +466,6 @@ &pwm3 {
 };
 
 &ssi1 {
-   #sound-dai-cells = <0>;
-   fsl,mode = "ac97-slave";
status = "okay";
 };
 
-- 
2.30.2



[PATCH v1 4/8] ARM: dts: imx6qdl-vicut1: update gpio-line-names for some GPIOs

2022-02-21 Thread Oleksij Rempel
From: David Jander 

countedX lines have different board names (YACO_x). And REV_ and BOARD_ pins
have multiple functions. So, use names exposed to the OS.

Signed-off-by: David Jander 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi 
b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
index 2f6b263eea66..ec39008c0950 100644
--- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
@@ -277,9 +277,9 @@ &gpio1 {
 
 &gpio2 {
gpio-line-names =
-   "count0", "count1", "count2", "", "", "", "", "",
-   "REV_ID0", "REV_ID1", "REV_ID2", "REV_ID3", "REV_ID4",
-   "BOARD_ID0", "BOARD_ID1", "BOARD_ID2",
+   "YACO_WHEEL", "YACO_RADAR", "YACO_PTO", "", "", "", "", "",
+   "", "LED_PWM", "", "", "",
+   "", "", "",
"", "", "", "", "", "", "", "ON_SWITCH",
"POWER_LED", "", "ECSPI2_SS0", "", "", "", "", "";
 };
@@ -298,8 +298,10 @@ &gpio4 {
"", "", "", "", "", "", "UART4_TXD", "UART4_RXD",
"UART5_TXD", "UART5_RXD", "CAN1_TX", "CAN1_RX", "CAN1_SR",
"CAN2_SR", "CAN2_TX", "CAN2_RX",
-   "LED_DI0_DEBUG_0", "LED_DI0_DEBUG_1", "", "", "", "", "", "",
-   "", "", "", "", "BL_EN", "BL_PWM", "", "";
+   "LED_DI0_DEBUG_0", "LED_DI0_DEBUG_1", "", "", "", "ON1_CTRL",
+   "ON2_CTRL", "HITCH_IN_OUT",
+   "LIGHT_ON", "", "", "CONTACT_IN", "BL_EN", "BL_PWM", "",
+   "ISB_LED";
 };
 
 &gpio5 {
-- 
2.30.2



[PATCH v1 3/8] ARM: dts: imx6qdl-vicut1/vicutgo: Add backlight_led node

2022-02-21 Thread Oleksij Rempel
From: David Jander 

backlight_led is the dimmable backlight for the rubber border on the case. It
is also used to highlight the power- and some other buttons.

MX6QDL_PAD_SD4_DAT1__PWM3_OUT is also assigned as output for pwm3. Since
we need pwm3 for the backlight, we're forced to disable user space hardware
revision detection. The bootloader will have to supply this information
(i.e. through device tree).

Signed-off-by: David Jander 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-victgo.dts   | 24 +--
 arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 33 +--
 2 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts 
b/arch/arm/boot/dts/imx6dl-victgo.dts
index 833340c30537..d542ddad4e32 100644
--- a/arch/arm/boot/dts/imx6dl-victgo.dts
+++ b/arch/arm/boot/dts/imx6dl-victgo.dts
@@ -33,6 +33,15 @@ backlight_lcd: backlight {
enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;
};
 
+   backlight_led: backlight_led {
+   compatible = "pwm-backlight";
+   pwms = <&pwm3 0 500 0>;
+   brightness-levels = <0 16 64 255>;
+   num-interpolated-steps = <16>;
+   default-brightness-level = <48>;
+   power-supply = <®_3v3>;
+   };
+
connector {
compatible = "composite-video-connector";
label = "Composite0";
@@ -392,8 +401,8 @@ &gpio1 {
 &gpio2 {
gpio-line-names =
"", "", "", "", "", "", "", "",
-   "REV_ID0", "REV_ID1", "REV_ID2", "REV_ID3", "REV_ID4",
-   "BOARD_ID0", "BOARD_ID1", "BOARD_ID2",
+   "", "LED_PWM", "", "", "",
+   "", "", "",
"", "", "", "", "", "", "ISB_IN1", "ON_SWITCH",
"POWER_LED", "", "", "", "", "", "", "";
 };
@@ -778,17 +787,6 @@ MX6QDL_PAD_CSI0_VSYNC__GPIO5_IO21  0x1f0b0
/* ITU656_nPDN */
MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20 0x1b0b0
 
-   /* HW revision detect */
-   /* REV_ID0 */
-   MX6QDL_PAD_SD4_DAT0__GPIO2_IO08 0x1b0b0
-   /* REV_ID1 is shared with PWM3 */
-   /* REV_ID2 */
-   MX6QDL_PAD_SD4_DAT2__GPIO2_IO10 0x1b0b0
-   /* REV_ID3 */
-   MX6QDL_PAD_SD4_DAT3__GPIO2_IO11 0x1b0b0
-   /* REV_ID4 */
-   MX6QDL_PAD_SD4_DAT4__GPIO2_IO12 0x1b0b0
-
/* New in HW revision 1 */
/* ON1_FB */
MX6QDL_PAD_EIM_D20__GPIO3_IO20  0x100b0
diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi 
b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
index a1fbbc9c26b6..2f6b263eea66 100644
--- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
@@ -28,6 +28,15 @@ backlight_lcd: backlight {
enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;
};
 
+   backlight_led: backlight_led {
+   compatible = "pwm-backlight";
+   pwms = <&pwm3 0 500 0>;
+   brightness-levels = <0 16 64 255>;
+   num-interpolated-steps = <16>;
+   default-brightness-level = <48>;
+   power-supply = <®_3v3>;
+   };
+
connector {
compatible = "composite-video-connector";
label = "Composite0";
@@ -448,6 +457,12 @@ &pwm1 {
status = "okay";
 };
 
+&pwm3 {
+   pinctrl-names = "default";
+   pinctrl-0 = <&pinctrl_pwm3>;
+   status = "okay";
+};
+
 &ssi1 {
#sound-dai-cells = <0>;
fsl,mode = "ac97-slave";
@@ -669,18 +684,6 @@ MX6QDL_PAD_CSI0_VSYNC__GPIO5_IO21  0x1f0b0
/* ITU656_nPDN */
MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20 0x1b0b0
 
-   /* HW revision detect */
-   /* REV_ID0 */
-   MX6QDL_PAD_SD4_DAT0__GPIO2_IO08 0x1b0b0
-   /* REV_ID1 */
-   MX6QDL_PAD_SD4_DAT1__GPIO2_IO09 0x1b0b0
-   /* REV_ID2 */
-   MX6QDL_PAD_SD4_DAT2__GPIO2_IO10 0x1b0b0
-   /* REV_ID3 */
-   MX6QDL_PAD_SD4_DAT3__GPIO2_IO11 0x1b0b0
-   /* REV_ID4 */
-   MX6QDL_PAD_SD4_DAT4__GPIO2_IO12 0x1b0b0
-
/* New in HW revision 1 */
/* ON1_FB */
MX6QDL_PAD_EIM_D20__GPIO3_IO20  0x100b0
@@ -738,6 +741,12 @@ MX6QDL_PAD_DISP0_DAT8__PWM1_OUT0x1b0b0
>;
};
 
+   pinctr

[PATCH v1 8/8] ARM: dts: imx6qdl-victgo: add CAN termination support

2022-02-21 Thread Oleksij Rempel
From: David Jander 

The gpio1 0 pin is controlling CAN termination, not USB H1 VBUS. So,
remove wrong regulator and assign this gpio to new DT CAN termination
property.

Signed-off-by: David Jander 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-victgo.dts | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts 
b/arch/arm/boot/dts/imx6dl-victgo.dts
index 7839021bc3eb..d66da630e0af 100644
--- a/arch/arm/boot/dts/imx6dl-victgo.dts
+++ b/arch/arm/boot/dts/imx6dl-victgo.dts
@@ -160,15 +160,6 @@ reg_3v3: regulator-3v3 {
regulator-max-microvolt = <330>;
};
 
-   reg_h1_vbus: regulator-h1-vbus {
-   compatible = "regulator-fixed";
-   regulator-name = "h1-vbus";
-   regulator-min-microvolt = <500>;
-   regulator-max-microvolt = <500>;
-   gpio = <&gpio1 0 GPIO_ACTIVE_HIGH>;
-   enable-active-high;
-   };
-
reg_otg_vbus: regulator-otg-vbus {
compatible = "regulator-fixed";
regulator-name = "otg-vbus";
@@ -312,6 +303,8 @@ IMX_AUDMUX_V2_PTCR_SYN  
IMX_AUDMUX_V2_PDCR_RXDSEL(0)
 &can1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_can1>;
+   termination-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
+   termination-ohms = <150>;
status = "okay";
 };
 
@@ -646,7 +639,6 @@ &uart5 {
 };
 
 &usbh1 {
-   vbus-supply = <®_h1_vbus>;
pinctrl-names = "default";
phy_type = "utmi";
dr_mode = "host";
-- 
2.30.2



[PATCH v1 2/8] ARM: dts: imx6qdl-vicut1/vicutgo: Rename backlight to backlight_lcd

2022-02-21 Thread Oleksij Rempel
From: David Jander 

We have two backlight sources on this boards. Use more specific name for
the LCD backlight to see the difference.

Signed-off-by: David Jander 
Signed-off-by: Robin van der Gracht 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-victgo.dts   | 4 ++--
 arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts 
b/arch/arm/boot/dts/imx6dl-victgo.dts
index e6134efbfabd..833340c30537 100644
--- a/arch/arm/boot/dts/imx6dl-victgo.dts
+++ b/arch/arm/boot/dts/imx6dl-victgo.dts
@@ -21,7 +21,7 @@ chosen {
stdout-path = &uart4;
};
 
-   backlight: backlight {
+   backlight_lcd: backlight {
compatible = "pwm-backlight";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_backlight>;
@@ -100,7 +100,7 @@ led-2 {
 
panel {
compatible = "kyo,tcg121xglp";
-   backlight = <&backlight>;
+   backlight = <&backlight_lcd>;
power-supply = <®_3v3>;
 
port {
diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi 
b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
index c1d06bc28c67..a1fbbc9c26b6 100644
--- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
@@ -16,7 +16,7 @@ chosen {
stdout-path = &uart4;
};
 
-   backlight: backlight {
+   backlight_lcd: backlight {
compatible = "pwm-backlight";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_backlight>;
@@ -102,7 +102,7 @@ led-2 {
 
panel {
compatible = "kyo,tcg121xglp";
-   backlight = <&backlight>;
+   backlight = <&backlight_lcd>;
power-supply = <®_3v3>;
 
port {
-- 
2.30.2



Re: [PATCH v16 21/40] pwm: tegra: Add runtime PM and OPP support

2022-02-21 Thread Dmitry Osipenko
Hello Uwe,

21.02.2022 11:17, Uwe Kleine-König пишет:
>> @@ -344,7 +387,10 @@ static const struct of_device_id tegra_pwm_of_match[] = 
>> {
>>  MODULE_DEVICE_TABLE(of, tegra_pwm_of_match);
>>  
>>  static const struct dev_pm_ops tegra_pwm_pm_ops = {
>> -SET_SYSTEM_SLEEP_PM_OPS(tegra_pwm_suspend, tegra_pwm_resume)
>> +SET_RUNTIME_PM_OPS(tegra_pwm_runtime_suspend, tegra_pwm_runtime_resume,
>> +   NULL)
>> +SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> +pm_runtime_force_resume)
>>  };
>>  
>>  static struct platform_driver tegra_pwm_driver = {
> I admit to not completely understand the effects of this patch, but I
> don't see a problem either. So for me this patch is OK:
> 
> Acked-by: Uwe Kleine-König 
> 
> I spot a problem, it's not introduced by this patch however: If the
> consumer of the PWM didn't stop the hardware, the suspend should IMHO be
> prevented.

Why? The PWM driver itself will stop the h/w on suspend.

> I wonder if the patches in this series go in in one go via an ARM or
> Tegra tree, or each patch via its respective maintainer tree.

This series, including this patch, was already applied to 5.17 via the
tegra/soc tree. No action is needed anymore.


Re: [PATCH v4 0/10] clk: Improve clock range handling

2022-02-21 Thread Laurent Pinchart
Hi Stephen,

(CC'ing Jean-Michel)

On Fri, Feb 18, 2022 at 06:24:08PM -0800, Stephen Boyd wrote:
> Quoting Laurent Pinchart (2022-02-14 01:45:56)
> > Hi Maxime and Stephen,
> > 
> > We have recently posted a driver for the BCM2711 Unicam CSI-2 receiver
> > (see [1]) which is a perfect candidate for this API, as it needs a
> > minimum rate for the VPU clock. Any chance we can get this series merged
> > ? :-)
> 
> The rate range API already exists, but it's busted. I can see you like
> the patch series, can you provide any Reviewed-by or Tested-by tags?

Jean-Michel, as you're working on upstreaming the RPi Unicam driver
which is our use case for this API, could you test this patch series ?

-- 
Regards,

Laurent Pinchart


[PATCH] drm/sched: Add device pointer to drm_gpu_scheduler

2022-02-21 Thread Jiawei Gu
Add device pointer so scheduler's printing can use
DRM_DEV_ERROR() instead, which makes life easier under multiple GPU
scenario.

v2: amend all calls of drm_sched_init()
v3: fill dev pointer for all drm_sched_init() calls

Signed-off-by: Jiawei Gu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_sched.c   |  2 +-
 drivers/gpu/drm/lima/lima_sched.c |  2 +-
 drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
 drivers/gpu/drm/scheduler/sched_main.c|  9 +
 drivers/gpu/drm/v3d/v3d_sched.c   | 10 +-
 include/drm/gpu_scheduler.h   |  3 ++-
 7 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 45977a72b5dd..cd2d594d4ffc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -502,7 +502,7 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring,
 
r = drm_sched_init(&ring->sched, &amdgpu_sched_ops,
   num_hw_submission, amdgpu_job_hang_limit,
-  timeout, NULL, sched_score, ring->name);
+  timeout, NULL, sched_score, ring->name, adev->dev);
if (r) {
DRM_ERROR("Failed to create scheduler on ring %s.\n",
  ring->name);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c 
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 58f593b278c1..35e5ef7dbdcc 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -195,7 +195,7 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
ret = drm_sched_init(&gpu->sched, &etnaviv_sched_ops,
 etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
 msecs_to_jiffies(500), NULL, NULL,
-dev_name(gpu->dev));
+dev_name(gpu->dev), gpu->dev);
if (ret)
return ret;
 
diff --git a/drivers/gpu/drm/lima/lima_sched.c 
b/drivers/gpu/drm/lima/lima_sched.c
index 5612d73f238f..8d517c8880e3 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -490,7 +490,7 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, 
const char *name)
return drm_sched_init(&pipe->base, &lima_sched_ops, 1,
  lima_job_hang_limit,
  msecs_to_jiffies(timeout), NULL,
- NULL, name);
+ NULL, name, pipe->ldev->dev);
 }
 
 void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 908d79520853..a6925dbb6224 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -812,7 +812,7 @@ int panfrost_job_init(struct panfrost_device *pfdev)
 nentries, 0,
 msecs_to_jiffies(JOB_TIMEOUT_MS),
 pfdev->reset.wq,
-NULL, "pan_js");
+NULL, "pan_js", pfdev->dev);
if (ret) {
dev_err(pfdev->dev, "Failed to create scheduler: %d.", 
ret);
goto err_sched;
diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
b/drivers/gpu/drm/scheduler/sched_main.c
index f91fb31ab7a7..b81fceb0b8a2 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -491,7 +491,7 @@ void drm_sched_start(struct drm_gpu_scheduler *sched, bool 
full_recovery)
if (r == -ENOENT)
drm_sched_job_done(s_job);
else if (r)
-   DRM_ERROR("fence add callback failed (%d)\n",
+   DRM_DEV_ERROR(sched->dev, "fence add callback 
failed (%d)\n",
  r);
} else
drm_sched_job_done(s_job);
@@ -957,7 +957,7 @@ static int drm_sched_main(void *param)
if (r == -ENOENT)
drm_sched_job_done(sched_job);
else if (r)
-   DRM_ERROR("fence add callback failed (%d)\n",
+   DRM_DEV_ERROR(sched->dev, "fence add callback 
failed (%d)\n",
  r);
dma_fence_put(fence);
} else {
@@ -991,7 +991,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
   const struct drm_sched_backend_ops *ops,
   unsigned hw_submission, unsigned hang_limit,
   long timeout, struct workqueue_struct *timeout_wq,
-  atomic_t *score, const char *nam

[PATCH v2 00/22] drm: Fill in default values for plane properties

2022-02-21 Thread Maxime Ripard
Hi,

We have a bunch of functions that create planes properties that will take a
default value, but it isn't actually enforced in the plane state.

This leads to drivers having multiple strategies to work around that issue,
most of them being a variation of forcing a value at plane state reset time.
Others work fine by luck, or have entirely ignored the issue.

This series aims at making sure the default value set by the call to the
function isn't ignored, and then making sure all drivers behave consistently.

Let me know what you think,
Maxime

Changes from v1:
  - Collected tags
  - Squashed some patches

Dave Stevenson (3):
  drm/object: Add drm_object_property_get_default_value() function
  drm/object: Add default zpos value at reset
  drm/object: Add default color encoding and range value at reset

Maxime Ripard (19):
  drm/komeda: plane: switch to plane reset helper
  drm/tegra: plane: switch to plane reset helper
  drm/tegra: hub: Fix zpos initial value mismatch
  drm/omap: plane: Fix zpos initial value mismatch
  drm/amd/display: Fix color encoding mismatch
  drm/tegra: plane: Remove redundant zpos initialisation
  drm/komeda: plane: Remove redundant zpos initialisation
  drm/exynos: plane: Remove redundant zpos initialisation
  drm/imx: ipuv3-plane: Remove redundant zpos initialisation
  drm/msm/mdp5: Remove redundant zpos initialisation
  drm/nouveau/kms: Remove redundant zpos initialisation
  drm/omap: plane: Remove redundant zpos initialisation
  drm/rcar: plane: Remove redundant zpos initialisation
  drm/sti: plane: Remove redundant zpos initialisation
  drm/sun4i: layer: Remove redundant zpos initialisation
  drm/komeda: plane: Remove redundant color encoding and range
initialisation
  drm/armada: overlay: Remove redundant color encoding and range
initialisation
  drm/imx: ipuv3-plane: Remove redundant color encoding and range
initialisation
  drm/omap: plane: Remove redundant color encoding and range
initialisation

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
 .../gpu/drm/arm/display/komeda/komeda_plane.c | 13 +
 drivers/gpu/drm/armada/armada_overlay.c   |  2 -
 drivers/gpu/drm/drm_atomic_state_helper.c | 25 +
 drivers/gpu/drm/drm_mode_object.c | 53 +++
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  5 +-
 drivers/gpu/drm/imx/ipuv3-plane.c |  8 +--
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 16 +++---
 drivers/gpu/drm/nouveau/dispnv50/wndw.c   |  2 -
 drivers/gpu/drm/omapdrm/omap_plane.c  | 22 
 drivers/gpu/drm/rcar-du/rcar_du_plane.c   |  1 -
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c |  1 -
 drivers/gpu/drm/sti/sti_cursor.c  |  2 +-
 drivers/gpu/drm/sti/sti_gdp.c |  2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c   |  2 +-
 drivers/gpu/drm/sti/sti_plane.c   |  6 ---
 drivers/gpu/drm/sti/sti_plane.h   |  1 -
 drivers/gpu/drm/sun4i/sun4i_layer.c   | 16 +++---
 drivers/gpu/drm/tegra/hub.c   |  2 +-
 drivers/gpu/drm/tegra/plane.c |  6 +--
 include/drm/drm_mode_object.h |  7 +++
 21 files changed, 111 insertions(+), 83 deletions(-)

-- 
2.35.1



[PATCH v2 01/22] drm/komeda: plane: switch to plane reset helper

2022-02-21 Thread Maxime Ripard
komeda_plane_reset() does the state initialisation by copying a lot of
the code found in the __drm_atomic_helper_plane_reset(). Let's switch to
that helper and reduce the boilerplate.

Cc: Brian Starkey 
Cc: "James (Qian) Wang" 
Cc: Liviu Dudau 
Cc: Mihail Atanassov 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index d63d83800a8a..1778f6e1ea56 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -145,14 +145,10 @@ static void komeda_plane_reset(struct drm_plane *plane)
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (state) {
-   state->base.rotation = DRM_MODE_ROTATE_0;
-   state->base.pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
-   state->base.alpha = DRM_BLEND_ALPHA_OPAQUE;
+   __drm_atomic_helper_plane_reset(plane, &state->base);
state->base.zpos = kplane->layer->base.id;
state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
-   plane->state = &state->base;
-   plane->state->plane = plane;
}
 }
 
-- 
2.35.1



[PATCH v2 02/22] drm/tegra: plane: switch to plane reset helper

2022-02-21 Thread Maxime Ripard
tegra_plane_reset() does the state initialisation by copying a lot of
the code found in the __drm_atomic_helper_plane_reset(). Let's switch to
that helper and reduce the boilerplate.

Cc: linux-te...@vger.kernel.org
Cc: Jonathan Hunter 
Cc: Thierry Reding 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/tegra/plane.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 321cb1f13da6..ec0822c86926 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -37,8 +37,7 @@ static void tegra_plane_reset(struct drm_plane *plane)
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (state) {
-   plane->state = &state->base;
-   plane->state->plane = plane;
+   __drm_atomic_helper_plane_reset(plane, &state->base);
plane->state->zpos = p->index;
plane->state->normalized_zpos = p->index;
 
-- 
2.35.1



[PATCH v2 05/22] drm/amd/display: Fix color encoding mismatch

2022-02-21 Thread Maxime Ripard
The amdgpu KMS driver calls drm_plane_create_color_properties() with a
default encoding set to BT709.

However, the core will ignore it and the driver doesn't force it in its
plane state reset hook, so the initial value will be 0, which represents
BT601.

Fix the mismatch by using an initial value of BT601 in
drm_plane_create_color_properties().

Cc: amd-...@lists.freedesktop.org
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: Harry Wentland 
Cc: Leo Li 
Cc: "Pan, Xinhui" 
Cc: Rodrigo Siqueira 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index feccf2b555d2..86b27a355e90 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct 
amdgpu_display_manager *dm,
BIT(DRM_COLOR_YCBCR_BT2020),
BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
BIT(DRM_COLOR_YCBCR_FULL_RANGE),
-   DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
+   DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE);
}
 
supported_rotations =
-- 
2.35.1



[PATCH v2 03/22] drm/tegra: hub: Fix zpos initial value mismatch

2022-02-21 Thread Maxime Ripard
While the tegra_shared_plane_create() function calls
drm_plane_create_zpos_property() with an initial value of 0,
tegra_plane_reset() will force it to the plane index.

Fix the discrepancy by setting the initial zpos value to the plane index
in the function call.

Cc: linux-te...@vger.kernel.org
Cc: Jonathan Hunter 
Cc: Thierry Reding 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/tegra/hub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index b910155f80c4..7f68a142d922 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -788,7 +788,7 @@ struct drm_plane *tegra_shared_plane_create(struct 
drm_device *drm,
}
 
drm_plane_helper_add(p, &tegra_shared_plane_helper_funcs);
-   drm_plane_create_zpos_property(p, 0, 0, 255);
+   drm_plane_create_zpos_property(p, index, 0, 255);
 
return p;
 }
-- 
2.35.1



[PATCH v2 10/22] drm/exynos: plane: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The exynos KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in exynos_drm_plane_reset(). However, the helpers have
been adjusted to set it properly at reset, so this is not needed
anymore.

Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: Alim Akhtar 
Cc: Inki Dae 
Cc: Joonyoung Shim 
Cc: Krzysztof Kozlowski 
Cc: Kyungmin Park 
Cc: Seung-Woo Kim 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c 
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index df76bdee7dca..3615cf329e32 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -122,7 +122,6 @@ static void exynos_plane_mode_set(struct 
exynos_drm_plane_state *exynos_state)
 
 static void exynos_drm_plane_reset(struct drm_plane *plane)
 {
-   struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
struct exynos_drm_plane_state *exynos_state;
 
if (plane->state) {
@@ -133,10 +132,8 @@ static void exynos_drm_plane_reset(struct drm_plane *plane)
}
 
exynos_state = kzalloc(sizeof(*exynos_state), GFP_KERNEL);
-   if (exynos_state) {
+   if (exynos_state)
__drm_atomic_helper_plane_reset(plane, &exynos_state->base);
-   plane->state->zpos = exynos_plane->config->zpos;
-   }
 }
 
 static struct drm_plane_state *
-- 
2.35.1



[PATCH v2 08/22] drm/tegra: plane: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The tegra KMS driver will call drm_plane_create_zpos_property() with an
init value of the plane index.

Since the initial value wasn't carried over in the state, the driver had
to set it again in tegra_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: linux-te...@vger.kernel.org
Cc: Jonathan Hunter 
Cc: Thierry Reding 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/tegra/plane.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index ec0822c86926..a00913d064d3 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -25,7 +25,6 @@ static void tegra_plane_destroy(struct drm_plane *plane)
 
 static void tegra_plane_reset(struct drm_plane *plane)
 {
-   struct tegra_plane *p = to_tegra_plane(plane);
struct tegra_plane_state *state;
unsigned int i;
 
@@ -38,8 +37,6 @@ static void tegra_plane_reset(struct drm_plane *plane)
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (state) {
__drm_atomic_helper_plane_reset(plane, &state->base);
-   plane->state->zpos = p->index;
-   plane->state->normalized_zpos = p->index;
 
for (i = 0; i < 3; i++)
state->iova[i] = DMA_MAPPING_ERROR;
-- 
2.35.1



[PATCH v2 04/22] drm/omap: plane: Fix zpos initial value mismatch

2022-02-21 Thread Maxime Ripard
While the omap_plane_init() function calls
drm_plane_create_zpos_property() with an initial value of 0,
omap_plane_reset() will force it to another value depending on the plane
type.

Fix the discrepancy by setting the initial zpos value to the same value
in the drm_plane_create_zpos_property() call.

Reviewed-by: Tomi Valkeinen 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c 
b/drivers/gpu/drm/omapdrm/omap_plane.c
index b35205c4e979..e67baf9a942c 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -533,6 +533,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
unsigned int num_planes = dispc_get_num_ovls(priv->dispc);
struct drm_plane *plane;
struct omap_plane *omap_plane;
+   unsigned int zpos;
int ret;
u32 nformats;
const u32 *formats;
@@ -564,7 +565,16 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
drm_plane_helper_add(plane, &omap_plane_helper_funcs);
 
omap_plane_install_properties(plane, &plane->base);
-   drm_plane_create_zpos_property(plane, 0, 0, num_planes - 1);
+
+   /*
+* Set the zpos default depending on whether we are a primary or overlay
+* plane.
+*/
+   if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+   zpos = 0;
+   else
+   zpos = omap_plane->id;
+   drm_plane_create_zpos_property(plane, zpos, 0, num_planes - 1);
drm_plane_create_alpha_property(plane);
drm_plane_create_blend_mode_property(plane, 
BIT(DRM_MODE_BLEND_PREMULTI) |
 BIT(DRM_MODE_BLEND_COVERAGE));
-- 
2.35.1



[PATCH v2 12/22] drm/msm/mdp5: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The mdp KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in mdp5_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: freedr...@lists.freedesktop.org
Cc: linux-arm-...@vger.kernel.org
Cc: Abhinav Kumar 
Cc: Rob Clark 
Cc: Sean Paul 
Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index c6b69afcbac8..5d8ac84c510b 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -48,6 +48,8 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
 static void mdp5_plane_install_properties(struct drm_plane *plane,
struct drm_mode_object *obj)
 {
+   unsigned int zpos;
+
drm_plane_create_rotation_property(plane,
   DRM_MODE_ROTATE_0,
   DRM_MODE_ROTATE_0 |
@@ -59,7 +61,12 @@ static void mdp5_plane_install_properties(struct drm_plane 
*plane,
BIT(DRM_MODE_BLEND_PIXEL_NONE) |
BIT(DRM_MODE_BLEND_PREMULTI) |
BIT(DRM_MODE_BLEND_COVERAGE));
-   drm_plane_create_zpos_property(plane, 1, 1, 255);
+
+   if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+   zpos = STAGE_BASE;
+   else
+   zpos = STAGE0 + drm_plane_index(plane);
+   drm_plane_create_zpos_property(plane, zpos, 1, 255);
 }
 
 static void
@@ -91,13 +98,6 @@ static void mdp5_plane_reset(struct drm_plane *plane)
 
kfree(to_mdp5_plane_state(plane->state));
mdp5_state = kzalloc(sizeof(*mdp5_state), GFP_KERNEL);
-
-   if (plane->type == DRM_PLANE_TYPE_PRIMARY)
-   mdp5_state->base.zpos = STAGE_BASE;
-   else
-   mdp5_state->base.zpos = STAGE0 + drm_plane_index(plane);
-   mdp5_state->base.normalized_zpos = mdp5_state->base.zpos;
-
__drm_atomic_helper_plane_reset(plane, &mdp5_state->base);
 }
 
-- 
2.35.1



[PATCH v2 07/22] drm/object: Add default zpos value at reset

2022-02-21 Thread Maxime Ripard
From: Dave Stevenson 

The drm_plane_create_zpos_property() function asks for an initial value,
and will set the associated plane state variable with that value if a
state is present.

However, that function is usually called at a time where there's no
state yet. Since the drm_plane_state reset helper doesn't take care of
reading that value when it's called, it means that in most cases the
initial value will be 0, or the drivers will have to work around it.

Let's add some code in __drm_atomic_helper_plane_state_reset() to get
the initial zpos value if the property has been attached in order to fix
this.

Reviewed-by: Harry Wentland 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Dave Stevenson 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index ddcf5c2c8e6a..1412cee404ca 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -243,11 +243,22 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
 void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
   struct drm_plane *plane)
 {
+   u64 val;
+
plane_state->plane = plane;
plane_state->rotation = DRM_MODE_ROTATE_0;
 
plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+
+   if (plane->zpos_property) {
+   if (!drm_object_property_get_default_value(&plane->base,
+  plane->zpos_property,
+  &val)) {
+   plane_state->zpos = val;
+   plane_state->normalized_zpos = val;
+   }
+   }
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
 
-- 
2.35.1



[PATCH v2 15/22] drm/rcar: plane: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The rcar-du KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in rcar_du_plane_reset() and rcar_du_vsp_plane_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Cc: linux-renesas-...@vger.kernel.org
Reviewed-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 -
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c 
b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 862197be1e01..9dda5e06457d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -696,7 +696,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
state->hwindex = -1;
state->source = RCAR_DU_PLANE_MEMORY;
state->colorkey = RCAR_DU_COLORKEY_NONE;
-   state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 }
 
 static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index b7fc5b069cbc..719c60034952 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -362,7 +362,6 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
return;
 
__drm_atomic_helper_plane_reset(plane, &state->state);
-   state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 }
 
 static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
-- 
2.35.1



[PATCH v2 14/22] drm/omap: plane: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The omap KMS driver will call drm_plane_create_zpos_property() with an
init value of the plane index and the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in omap_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Reviewed-by: Tomi Valkeinen 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c 
b/drivers/gpu/drm/omapdrm/omap_plane.c
index e67baf9a942c..d96bc929072a 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -414,13 +414,6 @@ static void omap_plane_reset(struct drm_plane *plane)
return;
 
__drm_atomic_helper_plane_reset(plane, &omap_state->base);
-
-   /*
-* Set the zpos default depending on whether we are a primary or overlay
-* plane.
-*/
-   plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
-  ? 0 : omap_plane->id;
plane->state->color_encoding = DRM_COLOR_YCBCR_BT601;
plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE;
 }
-- 
2.35.1



[PATCH v2 09/22] drm/komeda: plane: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The komeda KMS driver will call drm_plane_create_zpos_property() with an
init value of the plane index.

Since the initial value wasn't carried over in the state, the driver had
to set it again in komeda_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: Brian Starkey 
Cc: "James (Qian) Wang" 
Cc: Liviu Dudau 
Cc: Mihail Atanassov 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index 1778f6e1ea56..383bb2039bd4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -135,7 +135,6 @@ static void komeda_plane_destroy(struct drm_plane *plane)
 static void komeda_plane_reset(struct drm_plane *plane)
 {
struct komeda_plane_state *state;
-   struct komeda_plane *kplane = to_kplane(plane);
 
if (plane->state)
__drm_atomic_helper_plane_destroy_state(plane->state);
@@ -146,7 +145,6 @@ static void komeda_plane_reset(struct drm_plane *plane)
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (state) {
__drm_atomic_helper_plane_reset(plane, &state->base);
-   state->base.zpos = kplane->layer->base.id;
state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
}
-- 
2.35.1



[PATCH v2 17/22] drm/sun4i: layer: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The sun4i KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in sun4i_backend_layer_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-su...@lists.linux.dev
Cc: Chen-Yu Tsai 
Reviewed-by: Jernej Skrabec 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun4i_layer.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c 
b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 929e95f86b5b..6d43080791a0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -18,7 +18,6 @@
 
 static void sun4i_backend_layer_reset(struct drm_plane *plane)
 {
-   struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
struct sun4i_layer_state *state;
 
if (plane->state) {
@@ -31,10 +30,8 @@ static void sun4i_backend_layer_reset(struct drm_plane 
*plane)
}
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
-   if (state) {
+   if (state)
__drm_atomic_helper_plane_reset(plane, &state->state);
-   plane->state->zpos = layer->id;
-   }
 }
 
 static struct drm_plane_state *
@@ -192,7 +189,8 @@ static const uint64_t sun4i_layer_modifiers[] = {
 
 static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
struct sun4i_backend *backend,
-   enum drm_plane_type type)
+   enum drm_plane_type type,
+   unsigned int id)
 {
const uint64_t *modifiers = sun4i_layer_modifiers;
const uint32_t *formats = sun4i_layer_formats;
@@ -204,6 +202,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct 
drm_device *drm,
if (!layer)
return ERR_PTR(-ENOMEM);
 
+   layer->id = id;
layer->backend = backend;
 
if (IS_ERR_OR_NULL(backend->frontend)) {
@@ -226,8 +225,8 @@ static struct sun4i_layer *sun4i_layer_init_one(struct 
drm_device *drm,
 &sun4i_backend_layer_helper_funcs);
 
drm_plane_create_alpha_property(&layer->plane);
-   drm_plane_create_zpos_property(&layer->plane, 0, 0,
-  SUN4I_BACKEND_NUM_LAYERS - 1);
+   drm_plane_create_zpos_property(&layer->plane, layer->id,
+  0, SUN4I_BACKEND_NUM_LAYERS - 1);
 
return layer;
 }
@@ -249,14 +248,13 @@ struct drm_plane **sun4i_layers_init(struct drm_device 
*drm,
enum drm_plane_type type = i ? DRM_PLANE_TYPE_OVERLAY : 
DRM_PLANE_TYPE_PRIMARY;
struct sun4i_layer *layer;
 
-   layer = sun4i_layer_init_one(drm, backend, type);
+   layer = sun4i_layer_init_one(drm, backend, type, i);
if (IS_ERR(layer)) {
dev_err(drm->dev, "Couldn't initialize %s plane\n",
i ? "overlay" : "primary");
return ERR_CAST(layer);
}
 
-   layer->id = i;
planes[i] = &layer->plane;
}
 
-- 
2.35.1



[PATCH v2 16/22] drm/sti: plane: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The sti KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in sti_plane_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Reviewed-by: Alain Volmat 
Reviewed-by: Philippe Cornu 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sti/sti_cursor.c | 2 +-
 drivers/gpu/drm/sti/sti_gdp.c| 2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c  | 2 +-
 drivers/gpu/drm/sti/sti_plane.c  | 6 --
 drivers/gpu/drm/sti/sti_plane.h  | 1 -
 5 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index 1d6051b4f6fe..414c9973aa6d 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -351,7 +351,7 @@ static const struct drm_plane_funcs 
sti_cursor_plane_helpers_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = drm_plane_cleanup,
-   .reset = sti_plane_reset,
+   .reset = drm_atomic_helper_plane_reset,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
.late_register = sti_cursor_late_register,
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index d1a35d97bc45..3db3768a3241 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -905,7 +905,7 @@ static const struct drm_plane_funcs 
sti_gdp_plane_helpers_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = drm_plane_cleanup,
-   .reset = sti_plane_reset,
+   .reset = drm_atomic_helper_plane_reset,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
.late_register = sti_gdp_late_register,
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index 3c61ba8b43e0..2201a50353eb 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1283,7 +1283,7 @@ static const struct drm_plane_funcs 
sti_hqvdp_plane_helpers_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = drm_plane_cleanup,
-   .reset = sti_plane_reset,
+   .reset = drm_atomic_helper_plane_reset,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
.late_register = sti_hqvdp_late_register,
diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index 3da4a46df2f2..173409cdb99e 100644
--- a/drivers/gpu/drm/sti/sti_plane.c
+++ b/drivers/gpu/drm/sti/sti_plane.c
@@ -112,12 +112,6 @@ static int sti_plane_get_default_zpos(enum drm_plane_type 
type)
return 0;
 }
 
-void sti_plane_reset(struct drm_plane *plane)
-{
-   drm_atomic_helper_plane_reset(plane);
-   plane->state->zpos = sti_plane_get_default_zpos(plane->type);
-}
-
 static void sti_plane_attach_zorder_property(struct drm_plane *drm_plane,
 enum drm_plane_type type)
 {
diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
index 065bfb4a..8e33e629d9b0 100644
--- a/drivers/gpu/drm/sti/sti_plane.h
+++ b/drivers/gpu/drm/sti/sti_plane.h
@@ -81,5 +81,4 @@ void sti_plane_update_fps(struct sti_plane *plane,
 
 void sti_plane_init_property(struct sti_plane *plane,
 enum drm_plane_type type);
-void sti_plane_reset(struct drm_plane *plane);
 #endif
-- 
2.35.1



[PATCH v2 06/22] drm/object: Add drm_object_property_get_default_value() function

2022-02-21 Thread Maxime Ripard
From: Dave Stevenson 

Some functions to create properties (drm_plane_create_zpos_property or
drm_plane_create_color_properties for example) will ask for a range of
acceptable value and an initial one.

This initial value is then stored in the values array for that property.

Let's provide an helper to access this property.

Reviewed-by: Laurent Pinchart 
Signed-off-by: Dave Stevenson 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_mode_object.c | 53 +--
 include/drm/drm_mode_object.h |  7 
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_mode_object.c 
b/drivers/gpu/drm/drm_mode_object.c
index 86d9e907c0b2..ba1608effc0f 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -297,11 +297,26 @@ int drm_object_property_set_value(struct drm_mode_object 
*obj,
 }
 EXPORT_SYMBOL(drm_object_property_set_value);
 
+static int __drm_object_property_get_prop_value(struct drm_mode_object *obj,
+   struct drm_property *property,
+   uint64_t *val)
+{
+   int i;
+
+   for (i = 0; i < obj->properties->count; i++) {
+   if (obj->properties->properties[i] == property) {
+   *val = obj->properties->values[i];
+   return 0;
+   }
+   }
+
+   return -EINVAL;
+}
+
 static int __drm_object_property_get_value(struct drm_mode_object *obj,
   struct drm_property *property,
   uint64_t *val)
 {
-   int i;
 
/* read-only properties bypass atomic mechanism and still store
 * their value in obj->properties->values[].. mostly to avoid
@@ -311,15 +326,7 @@ static int __drm_object_property_get_value(struct 
drm_mode_object *obj,
!(property->flags & DRM_MODE_PROP_IMMUTABLE))
return drm_atomic_get_property(obj, property, val);
 
-   for (i = 0; i < obj->properties->count; i++) {
-   if (obj->properties->properties[i] == property) {
-   *val = obj->properties->values[i];
-   return 0;
-   }
-
-   }
-
-   return -EINVAL;
+   return __drm_object_property_get_prop_value(obj, property, val);
 }
 
 /**
@@ -348,6 +355,32 @@ int drm_object_property_get_value(struct drm_mode_object 
*obj,
 }
 EXPORT_SYMBOL(drm_object_property_get_value);
 
+/**
+ * drm_object_property_get_default_value - retrieve the default value of a
+ * property when in atomic mode.
+ * @obj: drm mode object to get property value from
+ * @property: property to retrieve
+ * @val: storage for the property value
+ *
+ * This function retrieves the default state of the given property as passed in
+ * to drm_object_attach_property
+ *
+ * Only atomic drivers should call this function directly, as for non-atomic
+ * drivers it will return the current value.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_object_property_get_default_value(struct drm_mode_object *obj,
+ struct drm_property *property,
+ uint64_t *val)
+{
+   WARN_ON(!drm_drv_uses_atomic_modeset(property->dev));
+
+   return __drm_object_property_get_prop_value(obj, property, val);
+}
+EXPORT_SYMBOL(drm_object_property_get_default_value);
+
 /* helper for getconnector and getproperties ioctls */
 int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic,
   uint32_t __user *prop_ptr,
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index c34a3e8030e1..912f1e415685 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -98,6 +98,10 @@ struct drm_object_properties {
 * Hence atomic drivers should not use drm_object_property_set_value()
 * and drm_object_property_get_value() on mutable objects, i.e. those
 * without the DRM_MODE_PROP_IMMUTABLE flag set.
+*
+* For atomic drivers the default value of properties is stored in this
+* array, so drm_object_property_get_default_value can be used to
+* retrieve it.
 */
uint64_t values[DRM_OBJECT_MAX_PROPERTY];
 };
@@ -126,6 +130,9 @@ int drm_object_property_set_value(struct drm_mode_object 
*obj,
 int drm_object_property_get_value(struct drm_mode_object *obj,
  struct drm_property *property,
  uint64_t *value);
+int drm_object_property_get_default_value(struct drm_mode_object *obj,
+ struct drm_property *property,
+ uint64_t *val);
 
 void drm_object_attach_property(struct drm_mode_object *obj,
struct drm_property *property,
-- 

[PATCH v2 20/22] drm/armada: overlay: Remove redundant color encoding and range initialisation

2022-02-21 Thread Maxime Ripard
The armada KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Limited Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in armada_overlay_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: Russell King 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/armada/armada_overlay.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_overlay.c 
b/drivers/gpu/drm/armada/armada_overlay.c
index 424250535fed..a25539039efd 100644
--- a/drivers/gpu/drm/armada/armada_overlay.c
+++ b/drivers/gpu/drm/armada/armada_overlay.c
@@ -325,8 +325,6 @@ static void armada_overlay_reset(struct drm_plane *plane)
state->contrast = DEFAULT_CONTRAST;
state->saturation = DEFAULT_SATURATION;
__drm_atomic_helper_plane_reset(plane, &state->base.base);
-   state->base.base.color_encoding = DEFAULT_ENCODING;
-   state->base.base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
}
 }
 
-- 
2.35.1



[PATCH v2 18/22] drm/object: Add default color encoding and range value at reset

2022-02-21 Thread Maxime Ripard
From: Dave Stevenson 

The drm_plane_create_color_properties() function asks for an initial
value for the color encoding and range, and will set the associated
plane state variable with that value if a state is present.

However, that function is usually called at a time where there's no
state yet. Since the drm_plane_state reset helper doesn't take care of
reading that value when it's called, it means that in most cases the
initial value will be 0 (so DRM_COLOR_YCBCR_BT601 and
DRM_COLOR_YCBCR_LIMITED_RANGE, respectively), or the drivers will have
to work around it.

Let's add some code in __drm_atomic_helper_plane_state_reset() to get
the initial encoding and range value if the property has been attached
in order to fix this.

Reviewed-by: Harry Wentland 
Signed-off-by: Dave Stevenson 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 1412cee404ca..3b6d3bdbd099 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -251,6 +251,20 @@ void __drm_atomic_helper_plane_state_reset(struct 
drm_plane_state *plane_state,
plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
 
+   if (plane->color_encoding_property) {
+   if (!drm_object_property_get_default_value(&plane->base,
+  
plane->color_encoding_property,
+  &val))
+   plane_state->color_encoding = val;
+   }
+
+   if (plane->color_range_property) {
+   if (!drm_object_property_get_default_value(&plane->base,
+  
plane->color_range_property,
+  &val))
+   plane_state->color_range = val;
+   }
+
if (plane->zpos_property) {
if (!drm_object_property_get_default_value(&plane->base,
   plane->zpos_property,
-- 
2.35.1



[PATCH v2 13/22] drm/nouveau/kms: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The nouveau KMS driver will call drm_plane_create_zpos_property() with
an init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in nv50_wndw_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: nouv...@lists.freedesktop.org
Cc: Ben Skeggs 
Cc: Karol Herbst 
Cc: Lyude Paul 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c 
b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 133c8736426a..0c1a2ea0ed04 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -635,8 +635,6 @@ nv50_wndw_reset(struct drm_plane *plane)
plane->funcs->atomic_destroy_state(plane, plane->state);
 
__drm_atomic_helper_plane_reset(plane, &asyw->state);
-   plane->state->zpos = nv50_wndw_zpos_default(plane);
-   plane->state->normalized_zpos = nv50_wndw_zpos_default(plane);
 }
 
 static void
-- 
2.35.1



[PATCH v2 11/22] drm/imx: ipuv3-plane: Remove redundant zpos initialisation

2022-02-21 Thread Maxime Ripard
The imx KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in ipu_plane_state_reset(). However, the helpers have
been adjusted to set it properly at reset, so this is not needed
anymore.

Cc: linux-arm-ker...@lists.infradead.org
Cc: NXP Linux Team 
Cc: Fabio Estevam 
Cc: Pengutronix Kernel Team 
Cc: Philipp Zabel 
Cc: Sascha Hauer 
Cc: Shawn Guo 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
b/drivers/gpu/drm/imx/ipuv3-plane.c
index 846c1aae69c8..414bdf08d0b0 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -297,7 +297,6 @@ void ipu_plane_disable_deferred(struct drm_plane *plane)
 
 static void ipu_plane_state_reset(struct drm_plane *plane)
 {
-   unsigned int zpos = (plane->type == DRM_PLANE_TYPE_PRIMARY) ? 0 : 1;
struct ipu_plane_state *ipu_state;
 
if (plane->state) {
@@ -311,8 +310,6 @@ static void ipu_plane_state_reset(struct drm_plane *plane)
 
if (ipu_state) {
__drm_atomic_helper_plane_reset(plane, &ipu_state->base);
-   ipu_state->base.zpos = zpos;
-   ipu_state->base.normalized_zpos = zpos;
ipu_state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
ipu_state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
}
-- 
2.35.1



[PATCH v2 19/22] drm/komeda: plane: Remove redundant color encoding and range initialisation

2022-02-21 Thread Maxime Ripard
The komeda KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Limited Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in komeda_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: Brian Starkey 
Cc: "James (Qian) Wang" 
Cc: Liviu Dudau 
Cc: Mihail Atanassov 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index 383bb2039bd4..541949f2d44a 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -143,11 +143,8 @@ static void komeda_plane_reset(struct drm_plane *plane)
plane->state = NULL;
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
-   if (state) {
+   if (state)
__drm_atomic_helper_plane_reset(plane, &state->base);
-   state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
-   state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
-   }
 }
 
 static struct drm_plane_state *
-- 
2.35.1



[PATCH v2 22/22] drm/omap: plane: Remove redundant color encoding and range initialisation

2022-02-21 Thread Maxime Ripard
The omap KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Full Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in omap_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Reviewed-by: Tomi Valkeinen 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c 
b/drivers/gpu/drm/omapdrm/omap_plane.c
index d96bc929072a..b83d91ec030a 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -403,7 +403,6 @@ void omap_plane_install_properties(struct drm_plane *plane,
 
 static void omap_plane_reset(struct drm_plane *plane)
 {
-   struct omap_plane *omap_plane = to_omap_plane(plane);
struct omap_plane_state *omap_state;
 
if (plane->state)
@@ -414,8 +413,6 @@ static void omap_plane_reset(struct drm_plane *plane)
return;
 
__drm_atomic_helper_plane_reset(plane, &omap_state->base);
-   plane->state->color_encoding = DRM_COLOR_YCBCR_BT601;
-   plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE;
 }
 
 static struct drm_plane_state *
-- 
2.35.1



[PATCH v2 21/22] drm/imx: ipuv3-plane: Remove redundant color encoding and range initialisation

2022-02-21 Thread Maxime Ripard
The imx KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Limited Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in ipu_plane_state_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: linux-arm-ker...@lists.infradead.org
Cc: NXP Linux Team 
Cc: Fabio Estevam 
Cc: Pengutronix Kernel Team 
Cc: Philipp Zabel 
Cc: Sascha Hauer 
Cc: Shawn Guo 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
b/drivers/gpu/drm/imx/ipuv3-plane.c
index 414bdf08d0b0..36b32e8806e3 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -308,11 +308,8 @@ static void ipu_plane_state_reset(struct drm_plane *plane)
 
ipu_state = kzalloc(sizeof(*ipu_state), GFP_KERNEL);
 
-   if (ipu_state) {
+   if (ipu_state)
__drm_atomic_helper_plane_reset(plane, &ipu_state->base);
-   ipu_state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
-   ipu_state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
-   }
 }
 
 static struct drm_plane_state *
-- 
2.35.1



Re: [PATCH v10 1/4] MIPS: Loongson64: dts: update the display controller device node

2022-02-21 Thread Krzysztof Kozlowski
On 21/02/2022 10:19, Sergei Shtylyov wrote:
> On 2/20/22 5:55 PM, Sui Jingfeng wrote:
> 
>> From: suijingfeng 
>>
>> The display controller is a pci device, its PCI vendor id is 0x0014
>> its PCI device id is 0x7a06.
>>
>> 1) In order to let the driver to know which chip the DC is contained
>>in, the compatible string of the display controller is updated
>>according to the chip's name.
>>
>> 2) Add display controller device node for ls2k1000 SoC
>>
>> Reported-by: Krzysztof Kozlowski 
>> Signed-off-by: suijingfeng 
>> Signed-off-by: Sui Jingfeng <15330273...@189.cn>
>> ---
>>  arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 8 
>>  arch/mips/boot/dts/loongson/ls7a-pch.dtsi  | 7 ++-
>>  2 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi 
>> b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
>> index 768cf2abcea3..af9cda540f9e 100644
>> --- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
>> +++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
>> @@ -209,6 +209,14 @@ gpu@5,0 {
>>  interrupt-parent = <&liointc0>;
>>  };
>>  
>> +lsdc: display-controller@6,0 {
> 
>Shouldn't the node name just be "display", according to the section 2.2.2
> of the DT spec?

lcd-controller, led-controller. As I understood from the bindings, this
is not physical device displaying something (like a panel) but rather a
device controlling such panel. Therefore display-controller feels
appropriate.


Best regards,
Krzysztof


[PATCH] drm/amdgpu: check vm ready by evicting

2022-02-21 Thread Qiang Yu
Workstation application ANSA/META get this error dmesg:
[drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16)

This is caused by:
1. create a 256MB buffer in invisible VRAM
2. CPU map the buffer and access it causes vm_fault and try to move
   it to visible VRAM
3. force visible VRAM space and traverse all VRAM bos to check if
   evicting this bo is valuable
4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable()
   will set amdgpu_vm->evicting, but latter due to not in visible
   VRAM, won't really evict it so not add it to amdgpu_vm->evicted
5. before next CS to clear the amdgpu_vm->evicting, user VM ops
   ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted)
   but fail in amdgpu_vm_bo_update_mapping() (check
   amdgpu_vm->evicting) and get this error log

This error won't affect functionality as next CS will finish the
waiting VM ops. But we'd better clear the error log by check the
evicting flag which really stop VM ops latter.

Signed-off-by: Qiang Yu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 37acd8911168..2cd9f1a2e5fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
  * Check if all VM PDs/PTs are ready for updates
  *
  * Returns:
- * True if eviction list is empty.
+ * True if VM is not evicting.
  */
 bool amdgpu_vm_ready(struct amdgpu_vm *vm)
 {
-   return list_empty(&vm->evicted);
+   bool ret;
+
+   amdgpu_vm_eviction_lock(vm);
+   ret = !vm->evicting;
+   amdgpu_vm_eviction_unlock(vm);
+   return ret;
 }
 
 /**
-- 
2.25.1



Re: (subset) [Intel-gfx] [PATCH 17/22] drm/vc4: Use drm_mode_copy()

2022-02-21 Thread Maxime Ripard
On Fri, 18 Feb 2022 12:03:58 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> struct drm_display_mode embeds a list head, so overwriting
> the full struct with another one will corrupt the list
> (if the destination mode is on a list). Use drm_mode_copy()
> instead which explicitly preserves the list head of
> the destination mode.
> 
> [...]

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime


Re: [PATCH] drm/sched: Add device pointer to drm_gpu_scheduler

2022-02-21 Thread Christian König

Am 21.02.22 um 10:57 schrieb Jiawei Gu:

Add device pointer so scheduler's printing can use
DRM_DEV_ERROR() instead, which makes life easier under multiple GPU
scenario.

v2: amend all calls of drm_sched_init()
v3: fill dev pointer for all drm_sched_init() calls

Signed-off-by: Jiawei Gu 


Reviewed-by: Christian König 

When Andrey is fine with that as well I think the best approach is to 
push this upstream through drm-misc-next since it touches multiple drivers.


We can merge it into the DKMS branch as well if necessary.

Regards,
Christian


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c |  2 +-
  drivers/gpu/drm/etnaviv/etnaviv_sched.c   |  2 +-
  drivers/gpu/drm/lima/lima_sched.c |  2 +-
  drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
  drivers/gpu/drm/scheduler/sched_main.c|  9 +
  drivers/gpu/drm/v3d/v3d_sched.c   | 10 +-
  include/drm/gpu_scheduler.h   |  3 ++-
  7 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 45977a72b5dd..cd2d594d4ffc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -502,7 +502,7 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring,
  
  	r = drm_sched_init(&ring->sched, &amdgpu_sched_ops,

   num_hw_submission, amdgpu_job_hang_limit,
-  timeout, NULL, sched_score, ring->name);
+  timeout, NULL, sched_score, ring->name, adev->dev);
if (r) {
DRM_ERROR("Failed to create scheduler on ring %s.\n",
  ring->name);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c 
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 58f593b278c1..35e5ef7dbdcc 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -195,7 +195,7 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
ret = drm_sched_init(&gpu->sched, &etnaviv_sched_ops,
 etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
 msecs_to_jiffies(500), NULL, NULL,
-dev_name(gpu->dev));
+dev_name(gpu->dev), gpu->dev);
if (ret)
return ret;
  
diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c

index 5612d73f238f..8d517c8880e3 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -490,7 +490,7 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, 
const char *name)
return drm_sched_init(&pipe->base, &lima_sched_ops, 1,
  lima_job_hang_limit,
  msecs_to_jiffies(timeout), NULL,
- NULL, name);
+ NULL, name, pipe->ldev->dev);
  }
  
  void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 908d79520853..a6925dbb6224 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -812,7 +812,7 @@ int panfrost_job_init(struct panfrost_device *pfdev)
 nentries, 0,
 msecs_to_jiffies(JOB_TIMEOUT_MS),
 pfdev->reset.wq,
-NULL, "pan_js");
+NULL, "pan_js", pfdev->dev);
if (ret) {
dev_err(pfdev->dev, "Failed to create scheduler: %d.", 
ret);
goto err_sched;
diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
b/drivers/gpu/drm/scheduler/sched_main.c
index f91fb31ab7a7..b81fceb0b8a2 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -491,7 +491,7 @@ void drm_sched_start(struct drm_gpu_scheduler *sched, bool 
full_recovery)
if (r == -ENOENT)
drm_sched_job_done(s_job);
else if (r)
-   DRM_ERROR("fence add callback failed (%d)\n",
+   DRM_DEV_ERROR(sched->dev, "fence add callback failed 
(%d)\n",
  r);
} else
drm_sched_job_done(s_job);
@@ -957,7 +957,7 @@ static int drm_sched_main(void *param)
if (r == -ENOENT)
drm_sched_job_done(sched_job);
else if (r)
-   DRM_ERROR("fence add callback failed (%d)\n",
+   DRM_DEV_ERROR(sched->dev, "fence add callback failed 
(%d)\n",
  r);
dma_fence_put(fence);
} else {
@@

Re: [PATCH] drm/amdgpu: check vm ready by evicting

2022-02-21 Thread Christian König




Am 21.02.22 um 11:12 schrieb Qiang Yu:

Workstation application ANSA/META get this error dmesg:
[drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16)

This is caused by:
1. create a 256MB buffer in invisible VRAM
2. CPU map the buffer and access it causes vm_fault and try to move
it to visible VRAM
3. force visible VRAM space and traverse all VRAM bos to check if
evicting this bo is valuable
4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable()
will set amdgpu_vm->evicting, but latter due to not in visible
VRAM, won't really evict it so not add it to amdgpu_vm->evicted
5. before next CS to clear the amdgpu_vm->evicting, user VM ops
ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted)
but fail in amdgpu_vm_bo_update_mapping() (check
amdgpu_vm->evicting) and get this error log

This error won't affect functionality as next CS will finish the
waiting VM ops. But we'd better clear the error log by check the
evicting flag which really stop VM ops latter.

Signed-off-by: Qiang Yu 


Reviewed-by: Christian König 

Good work.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 37acd8911168..2cd9f1a2e5fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
   * Check if all VM PDs/PTs are ready for updates
   *
   * Returns:
- * True if eviction list is empty.
+ * True if VM is not evicting.
   */
  bool amdgpu_vm_ready(struct amdgpu_vm *vm)
  {
-   return list_empty(&vm->evicted);
+   bool ret;
+
+   amdgpu_vm_eviction_lock(vm);
+   ret = !vm->evicting;
+   amdgpu_vm_eviction_unlock(vm);
+   return ret;
  }
  
  /**




Re: [PATCH] drm: Only select I2C_ALGOBIT for drivers that actually need it

2022-02-21 Thread Uwe Kleine-König
Hello,

On Fri, May 14, 2021 at 12:01:42PM +0200, Uwe Kleine-König wrote:
> While working on a drm driver that doesn't need the i2c algobit stuff I
> noticed that DRM selects this code even tough only 8 drivers actually use
> it. While also only some drivers use i2c, keep the select for I2C for the
> next cleanup patch. Still prepare this already by also selecting I2C for
> the individual drivers.
> 
> Signed-off-by: Uwe Kleine-König 

This patch is already old but the issue is still valid and the patch
even still applies to today's Linus' master branch.

I didn't receive any human feedback. If you consider this patch
worthwile I can recheck if it's still correct as is or needs adaption.

Best regards
Uwe

> ---
>  drivers/gpu/drm/Kconfig | 5 -
>  drivers/gpu/drm/ast/Kconfig | 2 ++
>  drivers/gpu/drm/gma500/Kconfig  | 2 ++
>  drivers/gpu/drm/hisilicon/hibmc/Kconfig | 2 ++
>  drivers/gpu/drm/i915/Kconfig| 2 ++
>  drivers/gpu/drm/mgag200/Kconfig | 2 ++
>  drivers/gpu/drm/nouveau/Kconfig | 2 ++
>  7 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 3c16bd1afd87..351ea617c498 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -12,7 +12,6 @@ menuconfig DRM
>   select HDMI
>   select FB_CMDLINE
>   select I2C
> - select I2C_ALGOBIT
>   select DMA_SHARED_BUFFER
>   select SYNC_FILE
>  # gallium uses SYS_kcmp for os_same_file_description() to de-duplicate
> @@ -233,6 +232,8 @@ config DRM_RADEON
>  select DRM_KMS_HELPER
>  select DRM_TTM
>   select DRM_TTM_HELPER
> + select I2C
> + select I2C_ALGOBIT
>   select POWER_SUPPLY
>   select HWMON
>   select BACKLIGHT_CLASS_DEVICE
> @@ -254,6 +255,8 @@ config DRM_AMDGPU
>   select DRM_SCHED
>   select DRM_TTM
>   select DRM_TTM_HELPER
> + select I2C
> + select I2C_ALGOBIT
>   select POWER_SUPPLY
>   select HWMON
>   select BACKLIGHT_CLASS_DEVICE
> diff --git a/drivers/gpu/drm/ast/Kconfig b/drivers/gpu/drm/ast/Kconfig
> index fbcf2f45cef5..bcc25decd485 100644
> --- a/drivers/gpu/drm/ast/Kconfig
> +++ b/drivers/gpu/drm/ast/Kconfig
> @@ -6,6 +6,8 @@ config DRM_AST
>   select DRM_VRAM_HELPER
>   select DRM_TTM
>   select DRM_TTM_HELPER
> + select I2C
> + select I2C_ALGOBIT
>   help
>Say yes for experimental AST GPU driver. Do not enable
>this driver without having a working -modesetting,
> diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
> index 0cff20265f97..e26c3a24955d 100644
> --- a/drivers/gpu/drm/gma500/Kconfig
> +++ b/drivers/gpu/drm/gma500/Kconfig
> @@ -3,6 +3,8 @@ config DRM_GMA500
>   tristate "Intel GMA500/600/3600/3650 KMS Framebuffer"
>   depends on DRM && PCI && X86 && MMU
>   select DRM_KMS_HELPER
> + select I2C
> + select I2C_ALGOBIT
>   # GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915
>   select ACPI_VIDEO if ACPI
>   select BACKLIGHT_CLASS_DEVICE if ACPI
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig 
> b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> index 43943e980203..ac8c42dc79f6 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> @@ -6,6 +6,8 @@ config DRM_HISI_HIBMC
>   select DRM_VRAM_HELPER
>   select DRM_TTM
>   select DRM_TTM_HELPER
> + select I2C
> + select I2C_ALGOBIT
>   help
> Choose this option if you have a Hisilicon Hibmc soc chipset.
> If M is selected the module will be called hibmc-drm.
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 69f57ca9c68d..b3bb6f7cfbbc 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -13,6 +13,8 @@ config DRM_I915
>   select DRM_PANEL
>   select DRM_MIPI_DSI
>   select RELAY
> + select I2C
> + select I2C_ALGOBIT
>   select IRQ_WORK
>   # i915 depends on ACPI_VIDEO when ACPI is enabled
>   # but for select to work, need to select ACPI_VIDEO's dependencies, ick
> diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig
> index eec59658a938..b28c5e4828f4 100644
> --- a/drivers/gpu/drm/mgag200/Kconfig
> +++ b/drivers/gpu/drm/mgag200/Kconfig
> @@ -4,6 +4,8 @@ config DRM_MGAG200
>   depends on DRM && PCI && MMU
>   select DRM_GEM_SHMEM_HELPER
>   select DRM_KMS_HELPER
> + select I2C
> + select I2C_ALGOBIT
>   help
>This is a KMS driver for Matrox G200 chips. It supports the original
>MGA G200 desktop chips and the server variants. It requires 0.3.0
> diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
> index 9436310d0854..8823f0b24c73 100644
> --- a/drivers/gpu/drm/nouveau/Kconfig
> +++ b/drivers/gpu/drm/nouveau/Kconfig
> @@ -7,6 +7,8 @@ config DRM_NOUVEAU
>

Re: [PATCH] drm/amdgpu: check vm ready by evicting

2022-02-21 Thread Paul Menzel

Dear Qiang Yu,


Am 21.02.22 um 11:12 schrieb Qiang Yu:


Thank you for your patch. Reading the commit message summary, I have no 
idea what “check vm ready by evicting” means. Can you please rephrase it?



Workstation application ANSA/META get this error dmesg:


What version, and how can this be reproduced exactly? Just by starting 
the application?



[drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16)

This is caused by:
1. create a 256MB buffer in invisible VRAM
2. CPU map the buffer and access it causes vm_fault and try to move
it to visible VRAM
3. force visible VRAM space and traverse all VRAM bos to check if
evicting this bo is valuable
4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable()
will set amdgpu_vm->evicting, but latter due to not in visible
VRAM, won't really evict it so not add it to amdgpu_vm->evicted
5. before next CS to clear the amdgpu_vm->evicting, user VM ops
ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted)
but fail in amdgpu_vm_bo_update_mapping() (check
amdgpu_vm->evicting) and get this error log

This error won't affect functionality as next CS will finish the
waiting VM ops. But we'd better clear the error log by check the


s/check/checking/


evicting flag which really stop VM ops latter.


stop*s*?

Can you please elaborate. Christian’s and your discussions was quite 
long, so adding a summary, why this approach works and what possible 
regressions there are going to be might be warranted.



Kind regards,

Paul



Signed-off-by: Qiang Yu 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 37acd8911168..2cd9f1a2e5fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
   * Check if all VM PDs/PTs are ready for updates
   *
   * Returns:
- * True if eviction list is empty.
+ * True if VM is not evicting.
   */
  bool amdgpu_vm_ready(struct amdgpu_vm *vm)
  {
-   return list_empty(&vm->evicted);
+   bool ret;
+
+   amdgpu_vm_eviction_lock(vm);
+   ret = !vm->evicting;
+   amdgpu_vm_eviction_unlock(vm);
+   return ret;
  }
  
  /**


Re: [Intel-gfx] [PATCH 6/7] drm: Document fdinfo format specification

2022-02-21 Thread Tvrtko Ursulin



Hi Rob,

On 25/01/2022 10:24, Tvrtko Ursulin wrote:


On 21/01/2022 11:50, Tvrtko Ursulin wrote:

On 20/01/2022 16:44, Rob Clark wrote:


[snip]


If there is a tool somewhere that displays this info, that would be
useful for testing my implementation.


I have a patch to Intel specific intel_gpu_top (see 
https://patchwork.freedesktop.org/patch/468491/?series=98555&rev=1). 
I'll have a look to see how much work would it be to extract common 
bits into a library and write a quick agnostic tool using it.


I factored out some code from intel_gpu_top in a quick and dirty attempt 
to make it generic and made a very rudimentary tools/gputop:


https://cgit.freedesktop.org/~tursulin/intel-gpu-tools/log/?h=gputop


Have you managed to spend any time playing with this yet?

The only remaining open was Daniel's mild concern if vendor agnostic 
userspace is possible using the proposed spec. If you managed to wire up 
the compliant exports and gputop tool works I think that concern would 
be settled.


Regards,

Tvrtko


Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

2022-02-21 Thread Noralf Trønnes



Den 18.02.2022 16.11, skrev Noralf Trønnes:
> Add binding for MIPI DBI compatible SPI panels.
> 
> v4:
> - There should only be two compatible (Maxime)
> - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible
> 
> v3:
> - Move properties to Device Tree (Maxime)
> - Use contains for compatible (Maxime)
> - Add backlight property to example
> - Flesh out description
> 
> v2:
> - Fix path for panel-common.yaml
> - Use unevaluatedProperties
> - Drop properties which are in the allOf section
> - Drop model property (Rob)
> 
> Acked-by: Maxime Ripard 
> Signed-off-by: Noralf Trønnes 
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++
>  1 file changed, 125 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml 
> b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> new file mode 100644
> index ..748c09113168
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml

> +allOf:
> +  - $ref: panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +items:
> +  - {} # Panel Specific Compatible
> +  - const: panel-mipi-dbi-spi
> +

Rob's bot uses a -m flag I didn't use, and with that the compatible fails:

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
  CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
  DTEX
Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dts
  DTC
Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml
  CHECK
Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml
Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml:0:0:
/example-0/spi/display@0: failed to match any schema with compatible:
['sainsmart18', 'panel-mipi-dbi-spi']

How can I write the compatible property to make the checker happy?

Noralf.

> +  write-only:
> +type: boolean
> +description:
> +  Controller is not readable (ie. MISO is not wired up).
> +
> +  dc-gpios:
> +maxItems: 1
> +description: |
> +  Controller data/command selection (D/CX) in 4-line SPI mode.
> +  If not set, the controller is in 3-line SPI mode.
> +
> +required:
> +  - compatible
> +  - reg
> +  - panel-timing
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +#include 
> +
> +spi {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +display@0{
> +compatible = "sainsmart18", "panel-mipi-dbi-spi";
> +reg = <0>;
> +spi-max-frequency = <4000>;
> +
> +dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> +reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
> +write-only;
> +
> +backlight = <&backlight>;
> +
> +width-mm = <35>;
> +height-mm = <28>;
> +
> +panel-timing {
> +hactive = <160>;
> +vactive = <128>;
> +hback-porch = <0>;
> +vback-porch = <0>;
> +
> +clock-frequency = <0>;
> +hfront-porch = <0>;
> +hsync-len = <0>;
> +vfront-porch = <0>;
> +vsync-len = <0>;
> +};
> +};
> +};
> +
> +...


[PATCH] drm/amd/display: move FPU-related code from dcn20 to dml folder

2022-02-21 Thread Melissa Wen
Move parts of dcn20 code that uses FPU to dml folder. It aims to isolate
FPU operations as described by series:

drm/amd/display: Introduce FPU directory inside DC
https://patchwork.freedesktop.org/series/93042/

This patch moves the following functions from dcn20_resource to
dml/dcn20_fpu and calls of public functions in dcn20_resource are
wrapped by DC_FP_START/END():

- void dcn20_populate_dml_writeback_from_context
- static bool is_dtbclk_required()
- static enum dcn_zstate_support_state()
- void dcn20_calculate_dlg_params()
- static void swizzle_to_dml_params()
- int dcn20_populate_dml_pipes_from_context()
- void dcn20_calculate_wm()
- void dcn20_cap_soc_clocks()
- void dcn20_update_bounding_box()
- void dcn20_patch_bounding_box()
- bool dcn20_validate_bandwidth_fp()

This movement also affects dcn30/31, as dcn20_calculate_dlg_params() is
used by dcn30 and dcn31. For this reason, I included dcn20_fpu headers
in dcn20_resource headers to make dcn20_calculate_dlg_params() visible
to dcn30/31.

Three new functions are created to isolate well-delimited FPU
operations:

- void dcn20_fpu_set_wb_arb_params(): set cli_watermark,
  pstate_watermark and time_per_pixel from wb_arb_params (struct
mcif_arb_params), since those uses FPU operations on double types:
WritebackUrgentWatermark, WritebackDRAMClockChangeWatermark, '16.0'.
- void dcn20_fpu_set_wm_ranges(): set min_fill_clk_mhz and
  max_fill_clk_mhz involves FPU calcs on dram_speed_mts (double type);
- void dcn20_fpu_adjust_dppclk(): adjust operation on RequiredDPPCLK
  that is a double.

Signed-off-by: Melissa Wen 
---
 drivers/gpu/drm/amd/display/dc/dcn20/Makefile |   25 -
 .../drm/amd/display/dc/dcn20/dcn20_resource.c | 1370 +---
 .../drm/amd/display/dc/dcn20/dcn20_resource.h |   30 +-
 .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c  | 1385 +
 .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.h  |   42 +-
 5 files changed, 1451 insertions(+), 1401 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/Makefile 
b/drivers/gpu/drm/amd/display/dc/dcn20/Makefile
index 5fcaf78334ff..abaed2121feb 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/Makefile
@@ -9,31 +9,6 @@ DCN20 = dcn20_resource.o dcn20_init.o dcn20_hwseq.o 
dcn20_dpp.o dcn20_dpp_cm.o d
 
 DCN20 += dcn20_dsc.o
 
-ifdef CONFIG_X86
-CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o := -mhard-float -msse
-endif
-
-ifdef CONFIG_PPC64
-CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o := -mhard-float -maltivec
-endif
-
-ifdef CONFIG_CC_IS_GCC
-ifeq ($(call cc-ifversion, -lt, 0701, y), y)
-IS_OLD_GCC = 1
-endif
-endif
-
-ifdef CONFIG_X86
-ifdef IS_OLD_GCC
-# Stack alignment mismatch, proceed with caution.
-# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3
-# (8B stack alignment).
-CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o += -mpreferred-stack-boundary=4
-else
-CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o += -msse2
-endif
-endif
-
 AMD_DAL_DCN20 = $(addprefix $(AMDDALPATH)/dc/dcn20/,$(DCN20))
 
 AMD_DISPLAY_FILES += $(AMD_DAL_DCN20)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index dfe2e1c25a26..63c50bee0144 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -63,7 +63,6 @@
 #include "dcn20_dccg.h"
 #include "dcn20_vmid.h"
 #include "dc_link_ddc.h"
-#include "dc_link_dp.h"
 #include "dce/dce_panel_cntl.h"
 
 #include "navi10_ip_offset.h"
@@ -93,367 +92,6 @@
 
 #define DC_LOGGER_INIT(logger)
 
-struct _vcs_dpi_ip_params_st dcn2_0_ip = {
-   .odm_capable = 1,
-   .gpuvm_enable = 0,
-   .hostvm_enable = 0,
-   .gpuvm_max_page_table_levels = 4,
-   .hostvm_max_page_table_levels = 4,
-   .hostvm_cached_page_table_levels = 0,
-   .pte_group_size_bytes = 2048,
-   .num_dsc = 6,
-   .rob_buffer_size_kbytes = 168,
-   .det_buffer_size_kbytes = 164,
-   .dpte_buffer_size_in_pte_reqs_luma = 84,
-   .pde_proc_buffer_size_64k_reqs = 48,
-   .dpp_output_buffer_pixels = 2560,
-   .opp_output_buffer_lines = 1,
-   .pixel_chunk_size_kbytes = 8,
-   .pte_chunk_size_kbytes = 2,
-   .meta_chunk_size_kbytes = 2,
-   .writeback_chunk_size_kbytes = 2,
-   .line_buffer_size_bits = 789504,
-   .is_line_buffer_bpp_fixed = 0,
-   .line_buffer_fixed_bpp = 0,
-   .dcc_supported = true,
-   .max_line_buffer_lines = 12,
-   .writeback_luma_buffer_size_kbytes = 12,
-   .writeback_chroma_buffer_size_kbytes = 8,
-   .writeback_chroma_line_buffer_width_pixels = 4,
-   .writeback_max_hscl_ratio = 1,
-   .writeback_max_vscl_ratio = 1,
-   .writeback_min_hscl_ratio = 1,
-   .writeback_min_vscl_ratio = 1,
-   .writeback_max_hscl_taps = 12,
-   .writeback_max_vscl_taps = 12,
-   .writeback_line_buffer_luma_buffer_size = 0,
-   .writeback_line_

Re: [PATCH v3 3/9] gpu: host1x: Add context device management code

2022-02-21 Thread Mikko Perttunen

On 2/19/22 19:48, Dmitry Osipenko wrote:

18.02.2022 14:39, Mikko Perttunen пишет:
...

+/*
+ * Due to an issue with T194 NVENC, only 38 bits can be used.
+ * Anyway, 256GiB of IOVA ought to be enough for anyone.
+ */
+static dma_addr_t context_device_dma_mask = DMA_BIT_MASK(38);


s/dma_addr_t/u64/ ? Apparently you should get compilation warning on ARM32.

https://elixir.bootlin.com/linux/v5.17-rc4/source/include/linux/device.h#L524 >

+int host1x_context_list_init(struct host1x *host1x)
+{
+   struct host1x_context_list *cdl = &host1x->context_list;
+   struct host1x_context *ctx;
+   struct device_node *node;
+   int index;


Nitpick: unsigned int

...

+del_devices:
+   while (--index >= 0)


Nitpick: while (index--)

... >> +void host1x_context_list_free(struct host1x_context_list *cdl)

+{
+   int i;


Nitpick: unsigned int



Thanks, fixed all.

Mikko


Re: [PATCH v3 3/9] gpu: host1x: Add context device management code

2022-02-21 Thread Mikko Perttunen

On 2/19/22 19:52, Dmitry Osipenko wrote:

18.02.2022 14:39, Mikko Perttunen пишет:

+   for (index = 0; index < cdl->len; index++) {
+   struct iommu_fwspec *fwspec;
+
+   ctx = &cdl->devs[index];
+
+   ctx->host = host1x;
+
+   device_initialize(&ctx->dev);
+
+   ctx->dev.dma_mask = &context_device_dma_mask;
+   ctx->dev.coherent_dma_mask = context_device_dma_mask;
+   dev_set_name(&ctx->dev, "host1x-ctx.%d", index);
+   ctx->dev.bus = &host1x_context_device_bus_type;


host1x_context_device_bus_type will be an undefined symbol if
CONFIG_TEGRA_HOST1X_CONTEXT_BUS=n? Please compile and test all combinations.


But this file is only built if CONFIG_HOST1X, which selects 
CONFIG_TEGRA_HOST1X_CONTEXT_BUS?


Mikko


Re: [PATCH v3 8/9] drm/tegra: vic: Implement get_streamid_offset

2022-02-21 Thread Mikko Perttunen

On 2/19/22 20:54, Dmitry Osipenko wrote:

19.02.2022 21:49, Dmitry Osipenko пишет:

18.02.2022 14:39, Mikko Perttunen пишет:

+static int vic_get_streamid_offset(struct tegra_drm_client *client)
+{
+   struct vic *vic = to_vic(client);
+   int err;
+
+   err = vic_load_firmware(vic);


You can't invoke vic_load_firmware() while RPM is suspended. Either
replace this with RPM get/put or do something else.


Why not, I'm not seeing any HW accesses in vic_load_firmware? Although 
it looks like it might race with the vic_load_firmware call in 
vic_runtime_resume which probably needs to be fixed.





+   if (err < 0)
+   return err;
+
+   if (vic->can_use_context)
+   return 0x30;
+   else
+   return -ENOTSUPP;


If (!vic->can_use_context)
return -ENOTSUPP;

return 0x30;


and s/ENOTSUPP/EOPNOTSUPP/


Ok.

Mikko


Re: [PATCH v6 21/23] drm: rockchip: Add VOP2 driver

2022-02-21 Thread Andy Yan

Hi Sascha:

On 2/17/22 16:29, Sascha Hauer wrote:

From: Andy Yan 

The VOP2 unit is found on Rockchip SoCs beginning with rk3566/rk3568.
It replaces the VOP unit found in the older Rockchip SoCs.

This driver has been derived from the downstream Rockchip Kernel and
heavily modified:

- All nonstandard DRM properties have been removed
- dropped struct vop2_plane_state and pass around less data between
   functions
- Dropped all DRM_FORMAT_* not known on upstream
- rework register access to get rid of excessively used macros
- Drop all waiting for framesyncs


All the waiting sync in the downstream divers are try to fix special 
problems,


and some of them inherited from upstream vop driver, for example: the 
fb_unref_work,


It submitted by Tomas Figa to upstream vop driver to make the wait for 
flip and asynchronously cursor work


right.

VOP2 share the same hardware design in vblank, so I don't think these 
code are useless.


[0] 
https://patchwork.kernel.org/project/linux-rockchip/patch/1473857701-9250-5-git-send-email-tf...@chromium.org/


[1] 
https://patchwork.kernel.org/project/linux-rockchip/patch/1473857701-9250-6-git-send-email-tf...@chromium.org/


[2] 
https://patchwork.kernel.org/project/linux-rockchip/patch/1473857701-9250-4-git-send-email-tf...@chromium.org/





The driver is tested with HDMI and MIPI-DSI display on a RK3568-EVB
board. Overlay support is tested with the modetest utility. AFBC support
on the cluster windows is tested with weston-simple-dmabuf-egl on
weston using the (yet to be upstreamed) panfrost driver support.

Signed-off-by: Andy Yan 
Signed-off-by: Sascha Hauer 
---

Notes:
 Changes since v5:
 - consistently use u8/u16/u32 rather than uint8_t/uint16_t/uint32_t
 - Use spin_lock rather than spin_lock_irqsave
 - replace printk with drm_dbg
 - break some overlong lines
 
 Changes since v4:

 - Avoid stack frame overflow by not allocating big array on the stack
 
 Changes since v3:

 - Sort includes
 - fix typos
 - Drop spinlock
 - Use regmap_set_bits()/regmap_clear_bits()
 - simplify vop2_scale_factor()
 - simplify vop2_afbc_transform_offset()
 
 Changes since v4:

 - Sort nodes alphabetically
 
 Changes since v3:

 - Fix HDMI connector type

  drivers/gpu/drm/rockchip/Kconfig |6 +
  drivers/gpu/drm/rockchip/Makefile|1 +
  drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |1 +
  drivers/gpu/drm/rockchip/rockchip_drm_drv.h  |6 +-
  drivers/gpu/drm/rockchip/rockchip_drm_fb.c   |2 +
  drivers/gpu/drm/rockchip/rockchip_drm_vop.h  |   15 +
  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2708 ++
  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  477 +++
  drivers/gpu/drm/rockchip/rockchip_vop2_reg.c |  281 ++
  9 files changed, 3496 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index b9b156308460a..4ff0043f0ee70 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -28,6 +28,12 @@ config ROCKCHIP_VOP
  This selects support for the VOP driver. You should enable it
  on all older SoCs up to RK3399.
  
+config ROCKCHIP_VOP2

+   bool "Rockchip VOP2 driver"
+   help
+ This selects support for the VOP2 driver. You should enable it
+ on all newer SoCs beginning form RK3568.
+
  config ROCKCHIP_ANALOGIX_DP
bool "Rockchip specific extensions for Analogix DP driver"
depends on ROCKCHIP_VOP
diff --git a/drivers/gpu/drm/rockchip/Makefile 
b/drivers/gpu/drm/rockchip/Makefile
index dfc5512fdb9f1..3ff7b21c04149 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -6,6 +6,7 @@
  rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
rockchip_drm_gem.o
  
+rockchipdrm-$(CONFIG_ROCKCHIP_VOP2) += rockchip_drm_vop2.o rockchip_vop2_reg.o

  rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.o
  rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
  rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 82c8faf1fb6b8..95f6c5985fdd7 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -459,6 +459,7 @@ static int __init rockchip_drm_init(void)
  
  	num_rockchip_sub_drivers = 0;

ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_ROCKCHIP_VOP);
+   ADD_ROCKCHIP_SUB_DRIVER(vop2_platform_driver, CONFIG_ROCKCHIP_VOP2);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_lvds_driver,
CONFIG_ROCKCHIP_LVDS);

Re: [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()

2022-02-21 Thread Max Krummenacher
Am Montag, den 21.02.2022, 09:29 +0100 schrieb Boris Brezillon:
> Hello Christoph,
> 
> On Sat, 19 Feb 2022 09:28:44 +
> Christoph Niedermaier  wrote:
> 
> > From: Max Krummenacher [mailto:max.oss...@gmail.com]
> > Sent: Wednesday, February 9, 2022 10:38 AM
> > > > If display timings were read from the devicetree using
> > > > of_get_display_timing() and pixelclk-active is defined
> > > > there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is
> > > > automatically generated. Through the function
> > > > drm_bus_flags_from_videomode() e.g. called in the
> > > > panel-simple driver this flag got into the bus flags,
> > > > but then in imx_pd_bridge_atomic_check() the bus flag
> > > > check failed and will not initialize the display. The
> > > > original commit fe141cedc433 does not explain why this
> > > > check was introduced. So remove the bus flags check,
> > > > because it stops the initialization of the display with
> > > > valid bus flags.
> > > > 
> > > > Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by the 
> > > > bridge when
> > > > available")
> > > > Signed-off-by: Christoph Niedermaier 
> > > > Cc: Marek Vasut 
> > > > Cc: Boris Brezillon 
> > > > Cc: Philipp Zabel 
> > > > Cc: David Airlie 
> > > > Cc: Daniel Vetter 
> > > > Cc: Shawn Guo 
> > > > Cc: Sascha Hauer 
> > > > Cc: Pengutronix Kernel Team 
> > > > Cc: Fabio Estevam 
> > > > Cc: NXP Linux Team 
> > > > Cc: linux-arm-ker...@lists.infradead.org
> > > > To: dri-devel@lists.freedesktop.org
> > > > ---
> > > > V2: - Add Boris to the Cc list
> > > > ---
> > > >  drivers/gpu/drm/imx/parallel-display.c | 8 
> > > >  1 file changed, 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/imx/parallel-display.c 
> > > > b/drivers/gpu/drm/imx/parallel-display.c
> > > > index a8aba0141ce7..06cb1a59b9bc 100644
> > > > --- a/drivers/gpu/drm/imx/parallel-display.c
> > > > +++ b/drivers/gpu/drm/imx/parallel-display.c
> > > > @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct 
> > > > drm_bridge *bridge,
> > > >   if (!imx_pd_format_supported(bus_fmt))
> > > >   return -EINVAL;
> > > > 
> > > > - if (bus_flags &
> > > > - ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH |
> > > > -   DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> > > > -   DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) {
> > > > - dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", 
> > > > bus_flags);
> > > > - return -EINVAL;
> > > > - }
> > > > -
> > > >   bridge_state->output_bus_cfg.flags = bus_flags;
> > > >   bridge_state->input_bus_cfg.flags = bus_flags;
> > > >   imx_crtc_state->bus_flags = bus_flags;  
> > > 
> > > Tested on a Colibri iMX6DL with a panel-dpi based panel.
> > > 
> > > Tested-by: Max Krummenacher   
> > 
> > I still ask myself why this bus flag check is in the code.
> > Is there a reason not to remove that?
> 
> The reasoning was that DE_{LOW,HIGH} and
> FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE were the only bus_flags taken into
> account by the crtc logic, so anything else is simply ignored. This was
> definitely wrong since the driver supports at least one of the VSYNC
> polarity (perhaps both if there's a way to configure it that's not
> hooked-up yet).
> 
> So I guess figuring out the default VSYNC polarity and accepting the
> according DISPLAY_FLAGS_SYNC_XXXEDGE flag is what makes most sense here.

Note that {HV}SYNC polarities are taken from the timings '.flag' field
The bus_flags do not carry
that information.
(drivers/gpu/ipu-v3/ipu-di.c:611:if (sig->mode.flags &
DISPLAY_FLAGS_HSYNC_HIGH))

The new flags DRM_BUS_FLAG_SYNC_DRIVE_{POS,NEG}EDGE are siblings to
DRM_BUS_FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE and would allow to specify
on which pixelclock edge the sync signals are to be driven.
Before that addition it was implicitly assumed that the sync signals
and data signals would be driven on the same clock edge.
The way I read the IPU driver it is not
foreseen that the data lines
and sync lines are driven by a different clock edge.

I personally would just drop the sanity test on
bus_flags. This is what
this patch proposes.

Regards,
Max
> 
> Regards,
> 
> Boris



Re: [PATCH v3 9/9] drm/tegra: Support context isolation

2022-02-21 Thread Mikko Perttunen

On 2/19/22 20:35, Dmitry Osipenko wrote:

18.02.2022 14:39, Mikko Perttunen пишет:

+   if (context->memory_context && 
context->client->ops->get_streamid_offset) {

 ^^^

+   int offset = 
context->client->ops->get_streamid_offset(context->client);
+
+   if (offset >= 0) {
+   job->context = context->memory_context;
+   job->engine_streamid_offset = offset;
+   host1x_context_get(job->context);
+   }


You should bump refcount unconditionally or you'll get refcnt underflow
on put, when offset < 0.


This refcount is intended to be dropped from 'release_job', where it's 
dropped if job->context is set, which it is from this path.





+   }
+
/*
 * job_data is now part of job reference counting, so don't release
 * it from here.
diff --git a/drivers/gpu/drm/tegra/uapi.c b/drivers/gpu/drm/tegra/uapi.c
index 9ab9179d2026..be33da54d12c 100644
--- a/drivers/gpu/drm/tegra/uapi.c
+++ b/drivers/gpu/drm/tegra/uapi.c
@@ -33,6 +33,9 @@ static void tegra_drm_channel_context_close(struct 
tegra_drm_context *context)
struct tegra_drm_mapping *mapping;
unsigned long id;
  
+	if (context->memory_context)

+   host1x_context_put(context->memory_context);


The "if (context->memory_context &&
context->client->ops->get_streamid_offset)" above doesn't match the "if
(context->memory_context)". You'll get refcount underflow.


And this drop is for the refcount implicitly added when allocating the 
memory context through host1x_context_alloc; so these two places should 
be independent.


Please elaborate if I missed something.

Thanks,
Mikko


[PATCH v2] drm/i915/ttm: fixup the mock_bo

2022-02-21 Thread Matthew Auld
When running the mock selftests we currently blow up with:

<6> [299.836278] i915: Running 
i915_gem_huge_page_mock_selftests/igt_mock_memory_region_huge_pages
<1> [299.836356] BUG: kernel NULL pointer dereference, address: 00c8
<1> [299.836361] #PF: supervisor read access in kernel mode
<1> [299.836364] #PF: error_code(0x) - not-present page
<6> [299.836367] PGD 0 P4D 0
<4> [299.836369] Oops:  [#1] PREEMPT SMP NOPTI
<4> [299.836372] CPU: 1 PID: 1429 Comm: i915_selftest Tainted: G U  
  5.17.0-rc4-CI-CI_DRM_11227+ #1
<4> [299.836376] Hardware name: Intel(R) Client Systems NUC11TNHi5/NUC11TNBi5, 
BIOS TNTGL357.0042.2020.1221.1743 12/21/2020
<4> [299.836380] RIP: 0010:ttm_resource_init+0x57/0x90 [ttm]
<4> [299.836392] RSP: 0018:c90001e4f680 EFLAGS: 00010203
<4> [299.836395] RAX:  RBX: c90001e4f708 RCX: 

<4> [299.836398] RDX: 888116172528 RSI: c90001e4f6f8 RDI: 

<4> [299.836401] RBP: c90001e4f6f8 R08: 01b0 R09: 
888116172528
<4> [299.836403] R10: 0001 R11: a4cb2e51 R12: 
c90001e4fa90
<4> [299.836406] R13: 888116172528 R14: 888130d7f4b0 R15: 
888130d7f400
<4> [299.836409] FS:  7ff241684500() GS:88849fe8() 
knlGS:
<4> [299.836412] CS:  0010 DS:  ES:  CR0: 80050033
<4> [299.836416] CR2: 00c8 CR3: 000107b80001 CR4: 
00770ee0
<4> [299.836418] PKRU: 5554
<4> [299.836420] Call Trace:
<4> [299.836422]  
<4> [299.836423]  i915_ttm_buddy_man_alloc+0x68/0x240 [i915]

ttm_resource_init() now needs to access the bo->bdev, and also wants to
store the bo reference. Try to keep both working. The mock_bo is a hack
so we can interface directly with the ttm managers alloc() and free() hooks for
our mock testing, without invoking other TTM features like eviction,
moves, etc.

v2: make sure we only touch res->bo if the alloc() returns successfully

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5123
Fixes: 0e05fc49c358 ("drm/ttm: add common accounting to the resource mgr v3")
Signed-off-by: Matthew Auld 
Cc: Christian König 
Cc: Thomas Hellström 
Acked-by: Christian König 
---
 drivers/gpu/drm/i915/intel_region_ttm.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c 
b/drivers/gpu/drm/i915/intel_region_ttm.c
index f2b888c16958..7dea07c579aa 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -200,11 +200,14 @@ intel_region_ttm_resource_alloc(struct 
intel_memory_region *mem,
int ret;
 
mock_bo.base.size = size;
+   mock_bo.bdev = &mem->i915->bdev;
place.flags = flags;
 
ret = man->func->alloc(man, &mock_bo, &place, &res);
if (ret == -ENOSPC)
ret = -ENXIO;
+   if (!ret)
+   res->bo = NULL; /* Rather blow up, then some uaf */
return ret ? ERR_PTR(ret) : res;
 }
 
@@ -219,6 +222,11 @@ void intel_region_ttm_resource_free(struct 
intel_memory_region *mem,
struct ttm_resource *res)
 {
struct ttm_resource_manager *man = mem->region_private;
+   struct ttm_buffer_object mock_bo = {};
+
+   mock_bo.base.size = res->num_pages << PAGE_SHIFT;
+   mock_bo.bdev = &mem->i915->bdev;
+   res->bo = &mock_bo;
 
man->func->free(man, res);
 }
-- 
2.34.1



Re: [PATCH] drm: rcar-du: switch to devm_drm_of_get_bridge

2022-02-21 Thread Laurent Pinchart
On Mon, Feb 21, 2022 at 09:56:19AM +0100, Maxime Ripard wrote:
> On Mon, Feb 21, 2022 at 08:37:57AM +0100, José Expósito wrote:
> > The function "drm_of_find_panel_or_bridge" has been deprecated in
> > favor of "devm_drm_of_get_bridge".
> > 
> > Switch to the new function and reduce boilerplate.
> > 
> > Signed-off-by: José Expósito 
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 16 +---
> >  1 file changed, 5 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> > b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 72a272cfc11e..99b0febc56d1 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -712,18 +712,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  {
> > int ret;
> >  
> > -   ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0,
> > - &lvds->panel, &lvds->next_bridge);
> 
> I guess lvds->panel can be removed from the rcar_lvds struct as well?

It's used in rcar_lvds_get_lvds_mode() though, so this patch introduces
a regression.

-- 
Regards,

Laurent Pinchart


Re: [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()

2022-02-21 Thread Boris Brezillon
On Mon, 21 Feb 2022 12:56:43 +0100
Max Krummenacher  wrote:

> Am Montag, den 21.02.2022, 09:29 +0100 schrieb Boris Brezillon:
> > Hello Christoph,
> > 
> > On Sat, 19 Feb 2022 09:28:44 +
> > Christoph Niedermaier  wrote:
> >   
> > > From: Max Krummenacher [mailto:max.oss...@gmail.com]
> > > Sent: Wednesday, February 9, 2022 10:38 AM  
> > > > > If display timings were read from the devicetree using
> > > > > of_get_display_timing() and pixelclk-active is defined
> > > > > there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is
> > > > > automatically generated. Through the function
> > > > > drm_bus_flags_from_videomode() e.g. called in the
> > > > > panel-simple driver this flag got into the bus flags,
> > > > > but then in imx_pd_bridge_atomic_check() the bus flag
> > > > > check failed and will not initialize the display. The
> > > > > original commit fe141cedc433 does not explain why this
> > > > > check was introduced. So remove the bus flags check,
> > > > > because it stops the initialization of the display with
> > > > > valid bus flags.
> > > > > 
> > > > > Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by 
> > > > > the bridge when
> > > > > available")
> > > > > Signed-off-by: Christoph Niedermaier 
> > > > > Cc: Marek Vasut 
> > > > > Cc: Boris Brezillon 
> > > > > Cc: Philipp Zabel 
> > > > > Cc: David Airlie 
> > > > > Cc: Daniel Vetter 
> > > > > Cc: Shawn Guo 
> > > > > Cc: Sascha Hauer 
> > > > > Cc: Pengutronix Kernel Team 
> > > > > Cc: Fabio Estevam 
> > > > > Cc: NXP Linux Team 
> > > > > Cc: linux-arm-ker...@lists.infradead.org
> > > > > To: dri-devel@lists.freedesktop.org
> > > > > ---
> > > > > V2: - Add Boris to the Cc list
> > > > > ---
> > > > >  drivers/gpu/drm/imx/parallel-display.c | 8 
> > > > >  1 file changed, 8 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/imx/parallel-display.c 
> > > > > b/drivers/gpu/drm/imx/parallel-display.c
> > > > > index a8aba0141ce7..06cb1a59b9bc 100644
> > > > > --- a/drivers/gpu/drm/imx/parallel-display.c
> > > > > +++ b/drivers/gpu/drm/imx/parallel-display.c
> > > > > @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct 
> > > > > drm_bridge *bridge,
> > > > >   if (!imx_pd_format_supported(bus_fmt))
> > > > >   return -EINVAL;
> > > > > 
> > > > > - if (bus_flags &
> > > > > - ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH |
> > > > > -   DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> > > > > -   DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) {
> > > > > - dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", 
> > > > > bus_flags);
> > > > > - return -EINVAL;
> > > > > - }
> > > > > -
> > > > >   bridge_state->output_bus_cfg.flags = bus_flags;
> > > > >   bridge_state->input_bus_cfg.flags = bus_flags;
> > > > >   imx_crtc_state->bus_flags = bus_flags;
> > > > 
> > > > Tested on a Colibri iMX6DL with a panel-dpi based panel.
> > > > 
> > > > Tested-by: Max Krummenacher 
> > > 
> > > I still ask myself why this bus flag check is in the code.
> > > Is there a reason not to remove that?  
> > 
> > The reasoning was that DE_{LOW,HIGH} and
> > FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE were the only bus_flags taken into
> > account by the crtc logic, so anything else is simply ignored. This was
> > definitely wrong since the driver supports at least one of the VSYNC
> > polarity (perhaps both if there's a way to configure it that's not
> > hooked-up yet).
> > 
> > So I guess figuring out the default VSYNC polarity and accepting the
> > according DISPLAY_FLAGS_SYNC_XXXEDGE flag is what makes most sense here.  
> 
> Note that {HV}SYNC polarities are taken from the timings '.flag' field
> The bus_flags do not carry
> that information.
> (drivers/gpu/ipu-v3/ipu-di.c:611:if (sig->mode.flags &
> DISPLAY_FLAGS_HSYNC_HIGH))
> 
> The new flags DRM_BUS_FLAG_SYNC_DRIVE_{POS,NEG}EDGE are siblings to
> DRM_BUS_FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE and would allow to specify
> on which pixelclock edge the sync signals are to be driven.
> Before that addition it was implicitly assumed that the sync signals
> and data signals would be driven on the same clock edge.

Oh, ok.

> The way I read the IPU driver it is not
> foreseen that the data lines
> and sync lines are driven by a different clock edge.
> 
> I personally would just drop the sanity test on
> bus_flags. This is what
> this patch proposes.

Sounds fine.

Acked-by: Boris Brezillon 


Re: [REPOST PATCH v4 02/13] drm/msm/dsi: Pass DSC params to drm_panel

2022-02-21 Thread Dmitry Baryshkov

On 10/02/2022 13:34, Vinod Koul wrote:

When DSC is enabled, we need to pass the DSC parameters to panel driver
as well, so add a dsc parameter in panel and set it when DSC is enabled

Also, fetch and pass DSC configuration for DSI panels to DPU encoder,
which will enable and configure DSC hardware blocks accordingly.

Signed-off-by: Dmitry Baryshkov 
Signed-off-by: Vinod Koul 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  2 ++
  drivers/gpu/drm/msm/dsi/dsi.c   |  5 +
  drivers/gpu/drm/msm/dsi/dsi.h   |  1 +
  drivers/gpu/drm/msm/dsi/dsi_host.c  | 22 ++
  drivers/gpu/drm/msm/msm_drv.h   |  8 
  include/drm/drm_panel.h |  7 +++
  6 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 47fe11a84a77..ef6ddac22767 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -578,6 +578,8 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
MSM_DISPLAY_CAP_CMD_MODE :
MSM_DISPLAY_CAP_VID_MODE;
  
+		info.dsc = msm_dsi_get_dsc_config(priv->dsi[i]);

+
if (msm_dsi_is_bonded_dsi(priv->dsi[i]) && priv->dsi[other]) {
rc = msm_dsi_modeset_init(priv->dsi[other], dev, 
encoder);
if (rc) {
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 052548883d27..3aeac15e7421 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -20,6 +20,11 @@ bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
return !(host_flags & MIPI_DSI_MODE_VIDEO);
  }
  
+struct msm_display_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi)

+{
+   return msm_dsi_host_get_dsc_config(msm_dsi->host);
+}
+
  static int dsi_get_phy(struct msm_dsi *msm_dsi)
  {
struct platform_device *pdev = msm_dsi->pdev;
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index c8dedc95428c..16cd9b2fce86 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -152,6 +152,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi);
  int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_bonded_dsi);
  void msm_dsi_host_snapshot(struct msm_disp_state *disp_state, struct 
mipi_dsi_host *host);
  void msm_dsi_host_test_pattern_en(struct mipi_dsi_host *host);
+struct msm_display_dsc_config *msm_dsi_host_get_dsc_config(struct 
mipi_dsi_host *host);
  
  /* dsi phy */

  struct msm_dsi_phy;
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 27553194f9fa..7e9913eff724 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2059,9 +2059,24 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
  {
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
+   struct drm_panel *panel;
int ret;
  
  	msm_host->dev = dev;

+   panel = msm_dsi_host_get_panel(&msm_host->base);
+
+   if (panel && panel->dsc) {
+   struct msm_display_dsc_config *dsc = msm_host->dsc;
+
+   if (!dsc) {
+   dsc = devm_kzalloc(&msm_host->pdev->dev, sizeof(*dsc), 
GFP_KERNEL);
+   if (!dsc)
+   return -ENOMEM;
+   dsc->drm = panel->dsc;
+   msm_host->dsc = dsc;
+   }
+   }
+
ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
if (ret) {
pr_err("%s: alloc tx gem obj failed, %d\n", __func__, ret);
@@ -2626,3 +2641,10 @@ void msm_dsi_host_test_pattern_en(struct mipi_dsi_host 
*host)
dsi_write(msm_host, 
REG_DSI_TEST_PATTERN_GEN_CMD_STREAM0_TRIGGER,

DSI_TEST_PATTERN_GEN_CMD_STREAM0_TRIGGER_SW_TRIGGER);
  }
+
+struct msm_display_dsc_config *msm_dsi_host_get_dsc_config(struct 
mipi_dsi_host *host)
+{
+   struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
+
+   return msm_host->dsc;
+}
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 384f9bad4760..e7a312edfe67 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -119,6 +119,7 @@ struct msm_display_topology {
   *  based on num_of_h_tiles
   * @is_te_using_watchdog_timer:  Boolean to indicate watchdog TE is
   * used instead of panel TE in cmd mode panels
+ * @dsc:   DSC configuration data for DSC-enabled displays
   */
  struct msm_display_info {
int intf_type;
@@ -126,6 +127,7 @@ struct msm_display_info {
uint32_t num_of_h_tiles;
uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
bool is_te_using_watchdog_timer;
+   stru

Re: [PATCH v2 1/3] drm/mm: Ensure that the entry is not NULL before extracting rb_node

2022-02-21 Thread Tvrtko Ursulin



On 18/02/2022 03:47, Kasireddy, Vivek wrote:

Hi Tvrtko,



On 17/02/2022 07:50, Vivek Kasireddy wrote:

While looking for next holes suitable for an allocation, although,
it is highly unlikely, make sure that the DECLARE_NEXT_HOLE_ADDR
macro is using a valid node before it extracts the rb_node from it.


Was the need for this just a consequence of insufficient locking in the
i915 patch?

[Kasireddy, Vivek] Partly, yes; but I figured since we are anyway doing
if (!entry || ..), it makes sense to dereference entry and extract the rb_node
after this check.


Unless I am blind I don't see that it makes a difference. 
"&entry->rb_hole_addr" is taking an address of, which works "fine" is 
entry is NULL. And does not get past the !entry check for the actual 
de-reference via RB_EMPTY_NODE. With your patch you move that after the 
!entry check but still have it in the RB_EMPTY_NODE macro. Again, unless 
I am blind, I think just drop this patch.


Regards,

Tvrtko



Thanks,
Vivek



Regards,

Tvrtko



Cc: Tvrtko Ursulin 
Cc: Christian König 
Signed-off-by: Vivek Kasireddy 
---
   drivers/gpu/drm/drm_mm.c | 5 +++--
   1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 8257f9d4f619..499d8874e4ed 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -389,11 +389,12 @@ first_hole(struct drm_mm *mm,
   #define DECLARE_NEXT_HOLE_ADDR(name, first, last)\
   static struct drm_mm_node *name(struct drm_mm_node *entry, u64 size) \
   {\
-   struct rb_node *parent, *node = &entry->rb_hole_addr;\
+   struct rb_node *parent, *node;  \
\
-   if (!entry || RB_EMPTY_NODE(node))  \
+   if (!entry || RB_EMPTY_NODE(&entry->rb_hole_addr))   \
return NULL;\
\
+   node = &entry->rb_hole_addr; \
if (usable_hole_addr(node->first, size)) {   \
node = node->first;  \
while (usable_hole_addr(node->last, size))   \


Re: [PATCH 1/2] drm/mm: Add an iterator to optimally walk over holes for an allocation (v3)

2022-02-21 Thread Tvrtko Ursulin



On 16/02/2022 10:36, Christian König wrote:

Am 15.02.22 um 23:23 schrieb Vivek Kasireddy:

This iterator relies on drm_mm_first_hole() and drm_mm_next_hole()
functions to identify suitable holes for an allocation of a given
size by efficiently traversing the rbtree associated with the given
allocator.

It replaces the for loop in drm_mm_insert_node_in_range() and can
also be used by drm drivers to quickly identify holes of a certain
size within a given range.

v2: (Tvrtko)
- Prepend a double underscore for the newly exported first/next_hole
- s/each_best_hole/each_suitable_hole/g
- Mask out DRM_MM_INSERT_ONCE from the mode before calling
   first/next_hole and elsewhere.

v3: (Tvrtko)
- Reduce the number of hunks by retaining the "mode" variable name

Cc: Christian König 
Reviewed-by: Tvrtko Ursulin 
Suggested-by: Tvrtko Ursulin 
Signed-off-by: Vivek Kasireddy 


Of hand it looks like it does what the description says without any 
functional change, but I don't know the internals of drm_mm so well either.


Feel free to add an Acked-by: Christian König .


Thanks!

Can we merge this via the Intel tree as one series (one drm core plus 
one i915 patch)? Daniel?


Regards,

Tvrtko


[v1] arm64/dts/qcom/sc7280: update mdp clk to max supported value to support higher refresh rates

2022-02-21 Thread Vinod Polimera
Panels with higher refresh rate will need mdp clk above 300Mhz.
Select max frequency for mdp clock during bootup, dpu driver will
scale down the clock as per usecase when first update from the framework is 
received.

Signed-off-by: Vinod Polimera 
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index baf1653..7af96fc 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2895,7 +2895,7 @@
assigned-clocks = <&dispcc 
DISP_CC_MDSS_MDP_CLK>,
<&dispcc 
DISP_CC_MDSS_VSYNC_CLK>,
<&dispcc DISP_CC_MDSS_AHB_CLK>;
-   assigned-clock-rates = <3>,
+   assigned-clock-rates = <50667>,
<1920>,
<1920>;
operating-points-v2 = <&mdp_opp_table>;
-- 
2.7.4



Re: [PATCH v2] drm/i915/ttm: fixup the mock_bo

2022-02-21 Thread Thomas Hellström
On Mon, 2022-02-21 at 12:11 +, Matthew Auld wrote:
> When running the mock selftests we currently blow up with:
> 
> <6> [299.836278] i915: Running
> i915_gem_huge_page_mock_selftests/igt_mock_memory_region_huge_pages
> <1> [299.836356] BUG: kernel NULL pointer dereference, address:
> 00c8
> <1> [299.836361] #PF: supervisor read access in kernel mode
> <1> [299.836364] #PF: error_code(0x) - not-present page
> <6> [299.836367] PGD 0 P4D 0
> <4> [299.836369] Oops:  [#1] PREEMPT SMP NOPTI
> <4> [299.836372] CPU: 1 PID: 1429 Comm: i915_selftest Tainted: G
> U    5.17.0-rc4-CI-CI_DRM_11227+ #1
> <4> [299.836376] Hardware name: Intel(R) Client Systems
> NUC11TNHi5/NUC11TNBi5, BIOS TNTGL357.0042.2020.1221.1743 12/21/2020
> <4> [299.836380] RIP: 0010:ttm_resource_init+0x57/0x90 [ttm]
> <4> [299.836392] RSP: 0018:c90001e4f680 EFLAGS: 00010203
> <4> [299.836395] RAX:  RBX: c90001e4f708 RCX:
> 
> <4> [299.836398] RDX: 888116172528 RSI: c90001e4f6f8 RDI:
> 
> <4> [299.836401] RBP: c90001e4f6f8 R08: 01b0 R09:
> 888116172528
> <4> [299.836403] R10: 0001 R11: a4cb2e51 R12:
> c90001e4fa90
> <4> [299.836406] R13: 888116172528 R14: 888130d7f4b0 R15:
> 888130d7f400
> <4> [299.836409] FS:  7ff241684500()
> GS:88849fe8() knlGS:
> <4> [299.836412] CS:  0010 DS:  ES:  CR0: 80050033
> <4> [299.836416] CR2: 00c8 CR3: 000107b80001 CR4:
> 00770ee0
> <4> [299.836418] PKRU: 5554
> <4> [299.836420] Call Trace:
> <4> [299.836422]  
> <4> [299.836423]  i915_ttm_buddy_man_alloc+0x68/0x240 [i915]
> 
> ttm_resource_init() now needs to access the bo->bdev, and also wants
> to
> store the bo reference. Try to keep both working. The mock_bo is a
> hack
> so we can interface directly with the ttm managers alloc() and free()
> hooks for
> our mock testing, without invoking other TTM features like eviction,
> moves, etc.
> 
> v2: make sure we only touch res->bo if the alloc() returns
> successfully
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5123
> Fixes: 0e05fc49c358 ("drm/ttm: add common accounting to the resource
> mgr v3")
> Signed-off-by: Matthew Auld 
> Cc: Christian König 
> Cc: Thomas Hellström 
> Acked-by: Christian König 


Reviewed-by: Thomas Hellström 

> ---
>  drivers/gpu/drm/i915/intel_region_ttm.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
> b/drivers/gpu/drm/i915/intel_region_ttm.c
> index f2b888c16958..7dea07c579aa 100644
> --- a/drivers/gpu/drm/i915/intel_region_ttm.c
> +++ b/drivers/gpu/drm/i915/intel_region_ttm.c
> @@ -200,11 +200,14 @@ intel_region_ttm_resource_alloc(struct
> intel_memory_region *mem,
> int ret;
>  
> mock_bo.base.size = size;
> +   mock_bo.bdev = &mem->i915->bdev;
> place.flags = flags;
>  
> ret = man->func->alloc(man, &mock_bo, &place, &res);
> if (ret == -ENOSPC)
> ret = -ENXIO;
> +   if (!ret)
> +   res->bo = NULL; /* Rather blow up, then some uaf */
> return ret ? ERR_PTR(ret) : res;
>  }
>  
> @@ -219,6 +222,11 @@ void intel_region_ttm_resource_free(struct
> intel_memory_region *mem,
>     struct ttm_resource *res)
>  {
> struct ttm_resource_manager *man = mem->region_private;
> +   struct ttm_buffer_object mock_bo = {};
> +
> +   mock_bo.base.size = res->num_pages << PAGE_SHIFT;
> +   mock_bo.bdev = &mem->i915->bdev;
> +   res->bo = &mock_bo;
>  
> man->func->free(man, res);
>  }




Re: [PATCH v16 21/40] pwm: tegra: Add runtime PM and OPP support

2022-02-21 Thread Uwe Kleine-König
Hello,

On Mon, Feb 21, 2022 at 12:53:58PM +0300, Dmitry Osipenko wrote:
> 21.02.2022 11:17, Uwe Kleine-König пишет:
> >> @@ -344,7 +387,10 @@ static const struct of_device_id tegra_pwm_of_match[] 
> >> = {
> >>  MODULE_DEVICE_TABLE(of, tegra_pwm_of_match);
> >>  
> >>  static const struct dev_pm_ops tegra_pwm_pm_ops = {
> >> -  SET_SYSTEM_SLEEP_PM_OPS(tegra_pwm_suspend, tegra_pwm_resume)
> >> +  SET_RUNTIME_PM_OPS(tegra_pwm_runtime_suspend, tegra_pwm_runtime_resume,
> >> + NULL)
> >> +  SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> >> +  pm_runtime_force_resume)
> >>  };
> >>  
> >>  static struct platform_driver tegra_pwm_driver = {
> > I admit to not completely understand the effects of this patch, but I
> > don't see a problem either. So for me this patch is OK:
> > 
> > Acked-by: Uwe Kleine-König 
> > 
> > I spot a problem, it's not introduced by this patch however: If the
> > consumer of the PWM didn't stop the hardware, the suspend should IMHO be
> > prevented.
> 
> Why? The PWM driver itself will stop the h/w on suspend.

Stopping the PWM might be bad. Only the consumer can know if it's ok to
stop the PWM on suspend. If so the consumer should stop the PWM in their
suspend callback and the PWM should prevent suspend if it wasn't
stopped.

> > I wonder if the patches in this series go in in one go via an ARM or
> > Tegra tree, or each patch via its respective maintainer tree.
> 
> This series, including this patch, was already applied to 5.17 via the
> tegra/soc tree. No action is needed anymore.

Ah, I missed that, thanks.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |


signature.asc
Description: PGP signature


[PATCH 0/8] drm/vc4: Fix frame corruption when moving the cursor

2022-02-21 Thread Maxime Ripard
Hi,

This series fixes a long standing issue with the VC4 driver when one was
moving the cursor on X11 along the edges of the monitor, if we had
overscan margins enabled.

The details are in the commit log of the last patch, but the main reason
was that moving along the edges with the overscan margins enabled
triggers a full blown commit, as opposed to an async commit. Since that
commit is on the cursor plane, it's treated as a legacy cursor update,
and won't wait for vblank, so it's possible to queue multiple commit
between vblank.

Now, the composition happens in the HVS, and the HVS has a series of
descriptors stored in an internal RAM, one for each plane. Allocations
in that RAM are tied to the CRTC state, and freed when that state is
destroyed. That internal RAM is also used by the HVS to store some
internal context while it's generating a frame.

If we get multiple commits between vblanks, chances are that the RAM
entries used by one of the first commit is going to be freed and reused
by a later commit, which will then overwrite the content of the earlier
entries, erasing the context in the process and corrupting the frame.

We've tested multiple solutions, but the one that seem to work without
any cons is to defer the deallocation of RAM entries to the next vblank
after the CRTC state has been freed.

Let me know what you think,
Maxime

Maxime Ripard (8):
  drm/vc4: kms: Take old state core clock rate into account
  drm/vc4: hvs: Fix frame count register readout
  drm/vc4: hvs: Store channel in variable
  drm/vc4: hvs: Remove dlist setup duplication
  drm/vc4: hvs: Move the dlist setup to its own function
  drm/vc4: hvs: Ignore atomic_flush if we're disabled
  drm/vc4: hvs: Use pointer to HVS in HVS_READ and HVS_WRITE macros
  drm/vc4: hvs: Defer dlist slots deallocation

 drivers/gpu/drm/vc4/vc4_crtc.c |  24 +--
 drivers/gpu/drm/vc4/vc4_drv.h  |  30 +++-
 drivers/gpu/drm/vc4/vc4_hvs.c  | 309 ++---
 drivers/gpu/drm/vc4/vc4_kms.c  |  10 +-
 drivers/gpu/drm/vc4/vc4_regs.h |  13 +-
 5 files changed, 299 insertions(+), 87 deletions(-)

-- 
2.35.1



[PATCH 1/8] drm/vc4: kms: Take old state core clock rate into account

2022-02-21 Thread Maxime Ripard
During a commit, the core clock, which feeds the HVS, needs to run at
a minimum of 500MHz.

While doing that commit, we can also change the mode to one that
requires a higher core clock, so we take the core clock rate associated
to that new state into account for that boost.

However, the old state also needs to be taken into account if it
requires a core clock higher that the new one and our 500MHz limit,
since it's still live in hardware at the beginning of our commit.

Fixes: 16e101051f32 ("drm/vc4: Increase the core clock based on HVS load")
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_kms.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 24de29bc1cda..992d6a240002 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -385,9 +385,10 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state 
*state)
}
 
if (vc4->hvs->hvs5) {
+   unsigned long state_rate = max(old_hvs_state->core_clock_rate,
+  new_hvs_state->core_clock_rate);
unsigned long core_rate = max_t(unsigned long,
-   5,
-   new_hvs_state->core_clock_rate);
+   5, state_rate);
 
clk_set_min_rate(hvs->core_clk, core_rate);
}
-- 
2.35.1



[PATCH 4/8] drm/vc4: hvs: Remove dlist setup duplication

2022-02-21 Thread Maxime Ripard
Setting the DISPLISTx register needs to occur in every case, and we
don't need to protect the register using the event_lock, so we can just
move it after the if branches and simplify a bit the function.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hvs.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index d225eea6e640..71aa5081eaa3 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -402,15 +402,12 @@ static void vc4_hvs_update_dlist(struct drm_crtc *crtc)
crtc->state->event = NULL;
}
 
-   HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel),
- vc4_state->mm.start);
-
spin_unlock_irqrestore(&dev->event_lock, flags);
-   } else {
-   HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel),
- vc4_state->mm.start);
}
 
+   HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel),
+ vc4_state->mm.start);
+
spin_lock_irqsave(&vc4_crtc->irq_lock, flags);
vc4_crtc->current_dlist = vc4_state->mm.start;
spin_unlock_irqrestore(&vc4_crtc->irq_lock, flags);
-- 
2.35.1



[PATCH 3/8] drm/vc4: hvs: Store channel in variable

2022-02-21 Thread Maxime Ripard
The assigned_channel field of our vc4_crtc_state structure is accessed
multiple times in vc4_hvs_atomic_flush, so let's move it to a variable
that can be used in all those places.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hvs.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index c8cae10500b9..d225eea6e640 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -460,6 +460,7 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
+   unsigned int channel = vc4_state->assigned_channel;
struct drm_plane *plane;
struct vc4_plane_state *vc4_plane_state;
bool debug_dump_regs = false;
@@ -500,8 +501,8 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
/* This sets a black background color fill, as is the case
 * with other DRM drivers.
 */
-   HVS_WRITE(SCALER_DISPBKGNDX(vc4_state->assigned_channel),
- 
HVS_READ(SCALER_DISPBKGNDX(vc4_state->assigned_channel)) |
+   HVS_WRITE(SCALER_DISPBKGNDX(channel),
+ HVS_READ(SCALER_DISPBKGNDX(channel)) |
  SCALER_DISPBKGND_FILL);
 
/* Only update DISPLIST if the CRTC was already running and is not
@@ -515,7 +516,7 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
vc4_hvs_update_dlist(crtc);
 
if (crtc->state->color_mgmt_changed) {
-   u32 dispbkgndx = 
HVS_READ(SCALER_DISPBKGNDX(vc4_state->assigned_channel));
+   u32 dispbkgndx = HVS_READ(SCALER_DISPBKGNDX(channel));
 
if (crtc->state->gamma_lut) {
vc4_hvs_update_gamma_lut(crtc);
@@ -527,7 +528,7 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
 */
dispbkgndx &= ~SCALER_DISPBKGND_GAMMA;
}
-   HVS_WRITE(SCALER_DISPBKGNDX(vc4_state->assigned_channel), 
dispbkgndx);
+   HVS_WRITE(SCALER_DISPBKGNDX(channel), dispbkgndx);
}
 
if (debug_dump_regs) {
-- 
2.35.1



[PATCH 2/8] drm/vc4: hvs: Fix frame count register readout

2022-02-21 Thread Maxime Ripard
In order to get the field currently being output, the driver has been
using the display FIFO frame count in the HVS, reading a 6-bit field at
the offset 12 in the DISPSTATx register.

While that field is indeed at that location for the FIFO 1 and 2, the
one for the FIFO0 is actually in the DISPSTAT1 register, at the offset
18.

Fixes: e538092cb15c ("drm/vc4: Enable precise vblank timestamping for 
interlaced modes.")
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_crtc.c |  2 +-
 drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
 drivers/gpu/drm/vc4/vc4_hvs.c  | 23 +++
 drivers/gpu/drm/vc4/vc4_regs.h | 12 ++--
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index e6cc47470e03..72fadce38d32 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -123,7 +123,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc 
*crtc,
*vpos /= 2;
 
/* Use hpos to correct for field offset in interlaced mode. */
-   if (VC4_GET_FIELD(val, SCALER_DISPSTATX_FRAME_COUNT) % 2)
+   if (vc4_hvs_get_fifo_frame_count(dev, 
vc4_crtc_state->assigned_channel) % 2)
*hpos += mode->crtc_htotal / 2;
}
 
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 4329e09d357c..801da3e8ebdb 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -935,6 +935,7 @@ void vc4_irq_reset(struct drm_device *dev);
 extern struct platform_driver vc4_hvs_driver;
 void vc4_hvs_stop_channel(struct drm_device *dev, unsigned int output);
 int vc4_hvs_get_fifo_from_output(struct drm_device *dev, unsigned int output);
+u8 vc4_hvs_get_fifo_frame_count(struct drm_device *dev, unsigned int fifo);
 int vc4_hvs_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state 
*state);
 void vc4_hvs_atomic_begin(struct drm_crtc *crtc, struct drm_atomic_state 
*state);
 void vc4_hvs_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state 
*state);
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 604933e20e6a..c8cae10500b9 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -197,6 +197,29 @@ static void vc4_hvs_update_gamma_lut(struct drm_crtc *crtc)
vc4_hvs_lut_load(crtc);
 }
 
+u8 vc4_hvs_get_fifo_frame_count(struct drm_device *dev, unsigned int fifo)
+{
+   struct vc4_dev *vc4 = to_vc4_dev(dev);
+   u8 field = 0;
+
+   switch (fifo) {
+   case 0:
+   field = VC4_GET_FIELD(HVS_READ(SCALER_DISPSTAT1),
+ SCALER_DISPSTAT1_FRCNT0);
+   break;
+   case 1:
+   field = VC4_GET_FIELD(HVS_READ(SCALER_DISPSTAT1),
+ SCALER_DISPSTAT1_FRCNT1);
+   break;
+   case 2:
+   field = VC4_GET_FIELD(HVS_READ(SCALER_DISPSTAT2),
+ SCALER_DISPSTAT2_FRCNT2);
+   break;
+   }
+
+   return field;
+}
+
 int vc4_hvs_get_fifo_from_output(struct drm_device *dev, unsigned int output)
 {
struct vc4_dev *vc4 = to_vc4_dev(dev);
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index 33410718089e..bae8c9cd6f7c 100644
--- a/drivers/gpu/drm/vc4/vc4_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_regs.h
@@ -379,8 +379,6 @@
 # define SCALER_DISPSTATX_MODE_EOF 3
 # define SCALER_DISPSTATX_FULL BIT(29)
 # define SCALER_DISPSTATX_EMPTYBIT(28)
-# define SCALER_DISPSTATX_FRAME_COUNT_MASK VC4_MASK(17, 12)
-# define SCALER_DISPSTATX_FRAME_COUNT_SHIFT12
 # define SCALER_DISPSTATX_LINE_MASKVC4_MASK(11, 0)
 # define SCALER_DISPSTATX_LINE_SHIFT   0
 
@@ -403,9 +401,15 @@
 (x) * (SCALER_DISPBKGND1 - \
SCALER_DISPBKGND0))
 #define SCALER_DISPSTAT10x0058
+# define SCALER_DISPSTAT1_FRCNT0_MASK  VC4_MASK(23, 18)
+# define SCALER_DISPSTAT1_FRCNT0_SHIFT 18
+# define SCALER_DISPSTAT1_FRCNT1_MASK  VC4_MASK(17, 12)
+# define SCALER_DISPSTAT1_FRCNT1_SHIFT 12
+
 #define SCALER_DISPSTATX(x)(SCALER_DISPSTAT0 +\
 (x) * (SCALER_DISPSTAT1 - \
SCALER_DISPSTAT0))
+
 #define SCALER_DISPBASE10x005c
 #define SCALER_DISPBASEX(x)(SCALER_DISPBASE0 +\
 (x) * (SCALER_DISPBASE1 - \
@@ -415,7 +419,11 @@
 (x) * (SCALER_DISPCTRL1 - \
SCALER_DISPCTRL0))
 #define SCALER_DISPBKGND2

[PATCH 5/8] drm/vc4: hvs: Move the dlist setup to its own function

2022-02-21 Thread Maxime Ripard
The vc4_hvs_update_dlist function mostly deals with setting up the
vblank events and setting up the dlist entry pointer to our current
active one.

We'll want to do the former separately from the vblank handling in later
patches, so let's move it to a function of its own.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hvs.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 71aa5081eaa3..2d540fc11357 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -382,10 +382,19 @@ int vc4_hvs_atomic_check(struct drm_crtc *crtc, struct 
drm_atomic_state *state)
return 0;
 }
 
+static void vc4_hvs_install_dlist(struct drm_crtc *crtc)
+{
+   struct drm_device *dev = crtc->dev;
+   struct vc4_dev *vc4 = to_vc4_dev(dev);
+   struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
+
+   HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel),
+ vc4_state->mm.start);
+}
+
 static void vc4_hvs_update_dlist(struct drm_crtc *crtc)
 {
struct drm_device *dev = crtc->dev;
-   struct vc4_dev *vc4 = to_vc4_dev(dev);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
unsigned long flags;
@@ -405,9 +414,6 @@ static void vc4_hvs_update_dlist(struct drm_crtc *crtc)
spin_unlock_irqrestore(&dev->event_lock, flags);
}
 
-   HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel),
- vc4_state->mm.start);
-
spin_lock_irqsave(&vc4_crtc->irq_lock, flags);
vc4_crtc->current_dlist = vc4_state->mm.start;
spin_unlock_irqrestore(&vc4_crtc->irq_lock, flags);
@@ -434,6 +440,7 @@ void vc4_hvs_atomic_enable(struct drm_crtc *crtc,
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
bool oneshot = vc4_crtc->feeds_txp;
 
+   vc4_hvs_install_dlist(crtc);
vc4_hvs_update_dlist(crtc);
vc4_hvs_init_channel(vc4, crtc, mode, oneshot);
 }
@@ -509,8 +516,10 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
 * If the CRTC is being disabled, there's no point in updating this
 * information.
 */
-   if (crtc->state->active && old_state->active)
+   if (crtc->state->active && old_state->active) {
+   vc4_hvs_install_dlist(crtc);
vc4_hvs_update_dlist(crtc);
+   }
 
if (crtc->state->color_mgmt_changed) {
u32 dispbkgndx = HVS_READ(SCALER_DISPBKGNDX(channel));
-- 
2.35.1



[PATCH 6/8] drm/vc4: hvs: Ignore atomic_flush if we're disabled

2022-02-21 Thread Maxime Ripard
atomic_flush will be called for each CRTC even if they aren't enabled.

The whole code we have there will thus run without a properly affected
channel, which can then result in all sorts of weird behaviour.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hvs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 2d540fc11357..5db125dc2358 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -472,6 +472,9 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
u32 __iomem *dlist_start = vc4->hvs->dlist + vc4_state->mm.start;
u32 __iomem *dlist_next = dlist_start;
 
+   if (vc4_state->assigned_channel == VC4_HVS_CHANNEL_DISABLED)
+   return;
+
if (debug_dump_regs) {
DRM_INFO("CRTC %d HVS before:\n", drm_crtc_index(crtc));
vc4_hvs_dump_state(dev);
-- 
2.35.1



[PATCH 7/8] drm/vc4: hvs: Use pointer to HVS in HVS_READ and HVS_WRITE macros

2022-02-21 Thread Maxime Ripard
Those macros are really about the HVS itself, and thus its associated
structure vc4_hvs, rather than the entire (virtual) vc4 device.

Let's change those macros to use the hvs pointer directly, and change
the calling sites accordingly.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 14 +--
 drivers/gpu/drm/vc4/vc4_drv.h  | 16 +++
 drivers/gpu/drm/vc4/vc4_hvs.c  | 77 +-
 drivers/gpu/drm/vc4/vc4_kms.c  |  5 ++-
 4 files changed, 60 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 72fadce38d32..5bb4027e479e 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -70,6 +70,7 @@ static const struct debugfs_reg32 crtc_regs[] = {
 static unsigned int
 vc4_crtc_get_cob_allocation(struct vc4_dev *vc4, unsigned int channel)
 {
+   struct vc4_hvs *hvs = vc4->hvs;
u32 dispbase = HVS_READ(SCALER_DISPBASEX(channel));
/* Top/base are supposed to be 4-pixel aligned, but the
 * Raspberry Pi firmware fills the low bits (which are
@@ -89,6 +90,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc 
*crtc,
 {
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
+   struct vc4_hvs *hvs = vc4->hvs;
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
struct vc4_crtc_state *vc4_crtc_state = to_vc4_crtc_state(crtc->state);
unsigned int cob_size;
@@ -123,7 +125,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc 
*crtc,
*vpos /= 2;
 
/* Use hpos to correct for field offset in interlaced mode. */
-   if (vc4_hvs_get_fifo_frame_count(dev, 
vc4_crtc_state->assigned_channel) % 2)
+   if (vc4_hvs_get_fifo_frame_count(hvs, 
vc4_crtc_state->assigned_channel) % 2)
*hpos += mode->crtc_htotal / 2;
}
 
@@ -413,6 +415,7 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, 
struct drm_encoder *encode
 static void require_hvs_enabled(struct drm_device *dev)
 {
struct vc4_dev *vc4 = to_vc4_dev(dev);
+   struct vc4_hvs *hvs = vc4->hvs;
 
WARN_ON_ONCE((HVS_READ(SCALER_DISPCTRL) & SCALER_DISPCTRL_ENABLE) !=
 SCALER_DISPCTRL_ENABLE);
@@ -426,6 +429,7 @@ static int vc4_crtc_disable(struct drm_crtc *crtc,
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
struct drm_device *dev = crtc->dev;
+   struct vc4_dev *vc4 = to_vc4_dev(dev);
int ret;
 
CRTC_WRITE(PV_V_CONTROL,
@@ -455,7 +459,7 @@ static int vc4_crtc_disable(struct drm_crtc *crtc,
vc4_encoder->post_crtc_disable(encoder, state);
 
vc4_crtc_pixelvalve_reset(crtc);
-   vc4_hvs_stop_channel(dev, channel);
+   vc4_hvs_stop_channel(vc4->hvs, channel);
 
if (vc4_encoder && vc4_encoder->post_crtc_powerdown)
vc4_encoder->post_crtc_powerdown(encoder, state);
@@ -481,6 +485,7 @@ static struct drm_encoder 
*vc4_crtc_get_encoder_by_type(struct drm_crtc *crtc,
 int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
 {
struct drm_device *drm = crtc->dev;
+   struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
enum vc4_encoder_type encoder_type;
const struct vc4_pv_data *pv_data;
@@ -502,7 +507,7 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
if (!(CRTC_READ(PV_V_CONTROL) & PV_VCONTROL_VIDEN))
return 0;
 
-   channel = vc4_hvs_get_fifo_from_output(drm, vc4_crtc->data->hvs_output);
+   channel = vc4_hvs_get_fifo_from_output(vc4->hvs, 
vc4_crtc->data->hvs_output);
if (channel < 0)
return 0;
 
@@ -715,6 +720,7 @@ static void vc4_crtc_handle_page_flip(struct vc4_crtc 
*vc4_crtc)
struct drm_crtc *crtc = &vc4_crtc->base;
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
+   struct vc4_hvs *hvs = vc4->hvs;
u32 chan = vc4_crtc->current_hvs_channel;
unsigned long flags;
 
@@ -733,7 +739,7 @@ static void vc4_crtc_handle_page_flip(struct vc4_crtc 
*vc4_crtc)
 * the CRTC and encoder already reconfigured, leading to
 * underruns. This can be seen when reconfiguring the CRTC.
 */
-   vc4_hvs_unmask_underrun(dev, chan);
+   vc4_hvs_unmask_underrun(hvs, chan);
}
spin_unlock(&vc4_crtc->irq_lock);
spin_unlock_irqrestore(&dev->event_lock, flags);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 801da3e8ebdb..15e0c2ac3940 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -574,8 +574,8 @@ to_vc4_crtc_state(struct drm_crtc_state *crtc_state)
 
 #define V3D_READ(offset) readl(vc4->v3d->regs + offset)
 #define V3D_WRITE(offset, val) writel(va

[PATCH 8/8] drm/vc4: hvs: Defer dlist slots deallocation

2022-02-21 Thread Maxime Ripard
During normal operations, the cursor position update is done through an
asynchronous plane update, which on the vc4 driver basically just
modifies the right dlist word to move the plane to the new coordinates.

However, when we have the overscan margins setup, we fall back to a
regular commit when we are next to the edges. And since that commit
happens to be on a cursor plane, it's considered a legacy cursor update
by KMS.

The main difference it makes is that it won't wait for its completion
(ie, next vblank) before returning. This means if we have multiple
commits happening in rapid succession, we can have several of them
happening before the next vblank.

In parallel, our dlist allocation is tied to a CRTC state, and each time
we do a commit we end up with a new CRTC state, with the previous one
being freed. This means that we free our previous dlist entry (but don't
clear it though) every time a new one is being committed.

Now, if we were to have two commits happening before the next vblank, we
could end up freeing reusing the same dlist entries before the next
vblank.

Indeed, we would start from an initial state taking, for example, the
dlist entries 10 to 20, then start a commit taking the entries 20 to 30
and setting the dlist pointer to 20, and freeing the dlist entries 10 to
20. However, since we haven't reach vblank yet, the HVS is still using
the entries 10 to 20.

If we were to make a new commit now, chances are the allocator are going
to give the 10 to 20 entries back, and we would change their content to
match the new state. If vblank hasn't happened yet, we just corrupted
the active dlist entries.

A first attempt to solve this was made by creating an intermediate dlist
buffer to store the current (ie, as of the last commit) dlist content,
that we would update each time the HVS is done with a frame. However, if
the interrupt handler missed the vblank window, we would end up copying
our intermediate dlist to the hardware one during the composition,
essentially creating the same issue.

Since making sure that our interrupt handler runs within a fixed,
constrained, time window would require to make Linux a real-time kernel,
this seems a bit out of scope.

Instead, we can work around our original issue by keeping the dlist
slots allocation longer. That way, we won't reuse a dlist slot while
it's still in flight. In order to achieve this, instead of freeing the
dlist slot when its associated CRTC state is destroyed, we'll queue it
in a list.

A naive implementation would free the buffers in that queue when we get
our end of frame interrupt. However, there's still a race since, just
like in the shadow dlist case, we don't control when the handler for
that interrupt is going to run. Thus, we can end up with a commit adding
an old dlist allocation to our queue during the window between our
actual interrupt and when our handler will run. And since that buffer is
still being used for the composition of the current frame, we can't free
it right away, exposing us to the original bug.

Fortunately for us, the hardware provides a frame counter that is
increased each time the first line of a frame is being generated.
Associating the frame counter the image is supposed to go away to the
allocation, and then only deallocate buffers that have a counter below
or equal to the one we see when the deallocation code should prevent the
above race from occuring.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_crtc.c |  10 +-
 drivers/gpu/drm/vc4/vc4_drv.h  |  15 ++-
 drivers/gpu/drm/vc4/vc4_hvs.c  | 181 ++---
 drivers/gpu/drm/vc4/vc4_regs.h |   1 +
 4 files changed, 184 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 5bb4027e479e..ed5fc400b66d 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -920,14 +920,8 @@ void vc4_crtc_destroy_state(struct drm_crtc *crtc,
struct vc4_dev *vc4 = to_vc4_dev(crtc->dev);
struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
 
-   if (drm_mm_node_allocated(&vc4_state->mm)) {
-   unsigned long flags;
-
-   spin_lock_irqsave(&vc4->hvs->mm_lock, flags);
-   drm_mm_remove_node(&vc4_state->mm);
-   spin_unlock_irqrestore(&vc4->hvs->mm_lock, flags);
-
-   }
+   vc4_hvs_mark_dlist_entry_stale(vc4->hvs, vc4_state->mm);
+   vc4_state->mm = NULL;
 
drm_atomic_helper_crtc_destroy_state(crtc, state);
 }
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 15e0c2ac3940..4d155af0262d 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -330,6 +330,9 @@ struct vc4_hvs {
struct drm_mm lbm_mm;
spinlock_t mm_lock;
 
+   struct list_head stale_dlist_entries;
+   struct work_struct free_dlist_work;
+
struct drm_mm_node mitchell_netravali_filter;
 
struct debugfs_regset32 reg

Re: [PATCH v10 1/4] MIPS: Loongson64: dts: update the display controller device node

2022-02-21 Thread Sui Jingfeng



On 2022/2/21 17:19, Sergei Shtylyov wrote:

On 2/20/22 5:55 PM, Sui Jingfeng wrote:


From: suijingfeng 

The display controller is a pci device, its PCI vendor id is 0x0014
its PCI device id is 0x7a06.

1) In order to let the driver to know which chip the DC is contained
in, the compatible string of the display controller is updated
according to the chip's name.

2) Add display controller device node for ls2k1000 SoC

Reported-by: Krzysztof Kozlowski 
Signed-off-by: suijingfeng 
Signed-off-by: Sui Jingfeng <15330273...@189.cn>
---
  arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 8 
  arch/mips/boot/dts/loongson/ls7a-pch.dtsi  | 7 ++-
  2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi 
b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
index 768cf2abcea3..af9cda540f9e 100644
--- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
@@ -209,6 +209,14 @@ gpu@5,0 {
interrupt-parent = <&liointc0>;
};
  
+			lsdc: display-controller@6,0 {

Shouldn't the node name just be "display", according to the section 2.2.2
of the DT spec?

[...]

diff --git a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi 
b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
index 2f45fce2cdc4..ec35ea9b2fe8 100644
--- a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
+++ b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
@@ -160,11 +160,8 @@ gpu@6,0 {
interrupt-parent = <&pic>;
};
  
-			dc@6,1 {

-   compatible = "pci0014,7a06.0",
-  "pci0014,7a06",
-  "pciclass03",
-  "pciclass0300";
+   lsdc: display-controller@6,1 {

Same here...

[...]

MBR, Sergey


Display sounds like a panel or monitor, while we are the device that driven the 
display device.

Running find . -name "*.dtsi" -type f | xargs grep "display-controller"
at drm-tip/arch/ directory show that there are a number of vendors using
display controller as theirnode name, for example Atmel and STM32.


./arm/boot/dts/pxa3xx.dtsi: gcu: display-controller@5400 {
./arm/boot/dts/at91sam9n12.dtsi:
hlcdc-display-controller {
./arm/boot/dts/at91sam9n12.dtsi:compatible = 
"atmel,hlcdc-display-controller";
./arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi: hlcdc-display-controller {
./arm/boot/dts/stm32h743.dtsi:  ltdc: display-controller@50001000 {
./arm/boot/dts/stm32mp151.dtsi: ltdc: display-controller@5a001000 {
./arm/boot/dts/at91sam9x5dm.dtsi:   hlcdc-display-controller {
./arm/boot/dts/gemini.dtsi: display-controller@6a00 {
./arm/boot/dts/stm32f429.dtsi:  ltdc: display-controller@40016800 {
./arm/boot/dts/at91sam9x5_lcd.dtsi: 
hlcdc-display-controller {
./arm/boot/dts/at91sam9x5_lcd.dtsi: compatible = 
"atmel,hlcdc-display-controller";
./arm/boot/dts/sama5d2.dtsi:
hlcdc-display-controller {
./arm/boot/dts/sama5d2.dtsi:compatible = 
"atmel,hlcdc-display-controller";
./arm/boot/dts/sama5d4.dtsi:
hlcdc-display-controller {
./arm/boot/dts/sama5d4.dtsi:compatible = 
"atmel,hlcdc-display-controller";
./arm/boot/dts/sama5d3_lcd.dtsi:
hlcdc-display-controller {
./arm/boot/dts/sama5d3_lcd.dtsi:compatible = 
"atmel,hlcdc-display-controller";
./arm/boot/dts/sam9x60.dtsi:
hlcdc-display-controller {
./arm/boot/dts/sam9x60.dtsi:compatible = 
"atmel,hlcdc-display-controller";

./arm/boot/dts/at91-sama5d4_ma5d4evk.dts:   
hlcdc-display-controller {
./arm/boot/dts/at91sam9n12ek.dts:   
hlcdc-display-controller {
./arm/boot/dts/am335x-pdu001.dts:   display-controller@0 {
./arm/boot/dts/at91-nattis-2-natte-2.dts:   hlcdc-display-controller {
./arm/boot/dts/gemini-dlink-dir-685.dts:
display-controller@6a00 {



Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs

2022-02-21 Thread Uwe Kleine-König
Hi Rob,

On Thu, Feb 10, 2022 at 06:54:13PM +0100, Lucas Stach wrote:
> Am Dienstag, dem 01.02.2022 um 11:35 -0600 schrieb Rob Herring:
> > On Fri, Jan 28, 2022 at 06:58:29PM +0100, Uwe Kleine-König wrote:
> > > On Fri, Jan 28, 2022 at 07:04:10AM -0600, Rob Herring wrote:
> > > > On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König
> > > >  wrote:
> > > > > 
> > > > > From: Marian Cichy 
> > > > > 
> > > > > This files documents the device tree for the new imx21-lcdc DRM 
> > > > > driver.
> > > > 
> > > > No, bindings document h/w and the h/w has not changed. We already have
> > > > a binding for the LCDC.
> > > 
> > > Just to be sure we're talking about the same thing: You're refering to
> > > Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt, right?
> > 
> > Looks right...
> > 
> > > I'm unsure what to do now. Should the two different bindings just be
> > > described in the same file? Should I stick to fsl,imx21-fb even for the
> > > new binding? (The hardware unit is named LCDC, so the name chosen here
> > > is the better one.) Please advise.
> > 
> > Yes, the name is unfortunate, but it should be 1 binding, 1 file, and 
> > unchanged (unless you want to add new optional properties). 
>
> The old FB driver binding is pretty insane. Except the reg and
> interrupt properties it is very confused about things. It exposes
> internal implementation details (like specifying verbatim register
> settings in the DT) and other properties are just misplaced, like the
> lcd-supply property that controls the panel power supply.
> 
> I really don't think that trying to stay backwards compatible here is a
> win for anyone. Anyone willing to switch their systems running on a 15
> year old SoC to the new DRM driver probably doesn't mind if they have
> to modify the DTS to make it work. Can we please let the FB driver die
> in peace and have a clean slate driver/binding for the DRM driver?

Does this feedback change anything on your side? My expectation is that
the graphics people will be happy about every fb driver being replaced
by a DRM driver and there will be hardly any incentive to add a layer
over the DRM driver to make it honor the old fb binding.

Assume I convert the old binding to yaml and then add the newly
supported binding to that using a big oneOf, would that be an acceptable
compromise?

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |


signature.asc
Description: PGP signature


Re: [PATCH v10 1/4] MIPS: Loongson64: dts: update the display controller device node

2022-02-21 Thread Sui Jingfeng



On 2022/2/21 18:01, Krzysztof Kozlowski wrote:

On 21/02/2022 10:19, Sergei Shtylyov wrote:

On 2/20/22 5:55 PM, Sui Jingfeng wrote:


From: suijingfeng 

The display controller is a pci device, its PCI vendor id is 0x0014
its PCI device id is 0x7a06.

1) In order to let the driver to know which chip the DC is contained
in, the compatible string of the display controller is updated
according to the chip's name.

2) Add display controller device node for ls2k1000 SoC

Reported-by: Krzysztof Kozlowski 
Signed-off-by: suijingfeng 
Signed-off-by: Sui Jingfeng <15330273...@189.cn>
---
  arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 8 
  arch/mips/boot/dts/loongson/ls7a-pch.dtsi  | 7 ++-
  2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi 
b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
index 768cf2abcea3..af9cda540f9e 100644
--- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
@@ -209,6 +209,14 @@ gpu@5,0 {
interrupt-parent = <&liointc0>;
};
  
+			lsdc: display-controller@6,0 {

Shouldn't the node name just be "display", according to the section 2.2.2
of the DT spec?

lcd-controller, led-controller. As I understood from the bindings, this
is not physical device displaying something (like a panel) but rather a
device controlling such panel. Therefore display-controller feels
appropriate.


Best regards,
Krzysztof

Extremely correct.


Re: Regression from 3c196f056666 ("drm/amdgpu: always reset the asic in suspend (v2)") on suspend?

2022-02-21 Thread Alex Deucher
On Mon, Feb 21, 2022 at 3:29 AM Eric Valette  wrote:
>
> On 20/02/2022 16:48, Dominique Dumont wrote:
> > On Monday, 14 February 2022 22:52:27 CET Alex Deucher wrote:
> >> Does the system actually suspend?
> >
> > Not really. The screens looks like it's going to suspend, but it does come
> > back after 10s or so. The light mounted in the middle of the power button 
> > does
> > not switch off.
>
>
> As I have a very similar problem and also commented on the original
> debian bug report
> (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005005), I will add
> some information here on another amd only laptop (renoir AMD Ryzen 7
> 4800H with Radeon Graphics + Radeon RX 5500/5500M / Pro 5500M).
>
> For me the suspend works once, but after the first resume (I do know
> know if it is in the suspend path or the resume path I see a RIP in the
> dmesg (see aditional info in debian bug))  and later suspend do not
> work: It only go to the kde login screen.
>
> I was unable due to network connectivity to do a full bisect but tested
> with the patch I had on my laptop:
>
> 5.10.101 works, 5.10 from debian works
> 5.11 works
> 5.12 works
> 5.13 suspend works but when resuming the PC is dead I have to reboot
> 5.14 seems to work but looking at dmesg it is full of RIP messages at
> various places.
> 5.15.24 is a described 5.15 from debian is behaving identically
> 5.16 from debian is behaving identically.
>
> >> Is this system S0i3 or regular S3?
>
> For me it is real S3.
>
> The proposed patch is intended for INTEl + intel gpu + amdgpu but I have
> dual amd GPU.

It doesn't really matter what the platform is, it could still
potentially help on your system, it depends on the bios implementation
for your platform and how it handles suspend. You can try the patch,
but I don't think you are hitting the same issue.  I bisect would be
helpful in your case.

Alex


Re: [PATCH v8 04/19] video/hdmi: Add audio_infoframe packing for DP

2022-02-21 Thread AngeloGioacchino Del Regno

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

From: Markus Schneider-Pargmann 

Similar to HDMI, DP uses audio infoframes as well which are structured
very similar to the HDMI ones.

This patch adds a helper function to pack the HDMI audio infoframe for
DP, called hdmi_audio_infoframe_pack_for_dp().
hdmi_audio_infoframe_pack_only() is split into two parts. One of them
packs the payload only and can be used for HDMI and DP.

Signed-off-by: Markus Schneider-Pargmann 
Signed-off-by: Guillaume Ranquet 
---
  drivers/video/hdmi.c| 83 -
  include/drm/drm_dp_helper.h |  2 +
  include/linux/hdmi.h|  7 +++-
  3 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 947be761dfa40..63e74d9fd210e 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -21,6 +21,7 @@
   * DEALINGS IN THE SOFTWARE.
   */
  
+#include 

  #include 
  #include 
  #include 
@@ -381,12 +382,34 @@ static int hdmi_audio_infoframe_check_only(const struct 
hdmi_audio_infoframe *fr
   *
   * Returns 0 on success or a negative error code on failure.
   */
-int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame)
+int hdmi_audio_infoframe_check(const struct hdmi_audio_infoframe *frame)
  {
return hdmi_audio_infoframe_check_only(frame);
  }
  EXPORT_SYMBOL(hdmi_audio_infoframe_check);
  
+static void

+hdmi_audio_infoframe_pack_payload(const struct hdmi_audio_infoframe *frame,
+ u8 *buffer)
+{
+   u8 channels;
+
+   if (frame->channels >= 2)
+   channels = frame->channels - 1;
+   else
+   channels = 0;
+
+   buffer[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
+   buffer[1] = ((frame->sample_frequency & 0x7) << 2) |
+(frame->sample_size & 0x3);
+   buffer[2] = frame->coding_type_ext & 0x1f;
+   buffer[3] = frame->channel_allocation;
+   buffer[4] = (frame->level_shift_value & 0xf) << 3;
+
+   if (frame->downmix_inhibit)
+   buffer[4] |= BIT(7);
+}
+
  /**
   * hdmi_audio_infoframe_pack_only() - write HDMI audio infoframe to binary 
buffer
   * @frame: HDMI audio infoframe
@@ -404,7 +427,6 @@ EXPORT_SYMBOL(hdmi_audio_infoframe_check);
  ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe 
*frame,
   void *buffer, size_t size)
  {
-   unsigned char channels;
u8 *ptr = buffer;
size_t length;
int ret;
@@ -420,28 +442,13 @@ ssize_t hdmi_audio_infoframe_pack_only(const struct 
hdmi_audio_infoframe *frame,
  
  	memset(buffer, 0, size);
  
-	if (frame->channels >= 2)

-   channels = frame->channels - 1;
-   else
-   channels = 0;
-
ptr[0] = frame->type;
ptr[1] = frame->version;
ptr[2] = frame->length;
ptr[3] = 0; /* checksum */
  
-	/* start infoframe payload */

-   ptr += HDMI_INFOFRAME_HEADER_SIZE;
-
-   ptr[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
-   ptr[1] = ((frame->sample_frequency & 0x7) << 2) |
-(frame->sample_size & 0x3);
-   ptr[2] = frame->coding_type_ext & 0x1f;
-   ptr[3] = frame->channel_allocation;
-   ptr[4] = (frame->level_shift_value & 0xf) << 3;
-
-   if (frame->downmix_inhibit)
-   ptr[4] |= BIT(7);
+   hdmi_audio_infoframe_pack_payload(frame,
+ ptr + HDMI_INFOFRAME_HEADER_SIZE);
  
  	hdmi_infoframe_set_checksum(buffer, length);
  
@@ -479,6 +486,44 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,

  }
  EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
  
+/**

+ * hdmi_audio_infoframe_pack_for_dp - Pack a HDMI Audio infoframe for
+ *displayport


This fits in one line (82 cols is ok)... but, anyway, please capitalize D and P
in the DisplayPort word.


+ *
+ * @frame HDMI Audio infoframe


You're almost there! You missed a colon after each description.
Also, I personally like seeing indented descriptions as, in my opinion, this
enhances human readability, as it's a bit more pleasing to the eye... but
it's not a requirement, so you be the judge.

* @frame:  HDMI Audio infoframe
* @sdp:Secondary data packet..
* @dp_version: DisplayPort version..

Please fix.


+ * @sdp secondary data packet for display port. This is filled with the
+ * appropriate data
+ * @dp_version Display Port version to be encoded in the header
+ *
+ * Packs a HDMI Audio Infoframe to be sent over Display Port. This function
+ * fills the secondary data packet to be used for Display Port.
+ *
+ * Return: Number of total written bytes or a negative errno on failure.
+ */
+ssize_t
+hdmi_audio_infoframe_pack_for_dp(const struct hdmi_audio_infoframe *frame,
+struct dp_sdp *sdp, u8 dp_version)
+{
+   int ret;
+
+   ret = hdmi_audi

Re: [PATCH v8 03/19] drm/edid: Add cea_sad helpers for freq/length

2022-02-21 Thread AngeloGioacchino Del Regno

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

From: Markus Schneider-Pargmann 

This patch adds two helper functions that extract the frequency and word
length from a struct cea_sad.

For these helper functions new defines are added that help translate the
'freq' and 'byte2' fields into real numbers.

Signed-off-by: Markus Schneider-Pargmann 
Signed-off-by: Guillaume Ranquet 
---
  drivers/gpu/drm/drm_edid.c | 74 ++
  include/drm/drm_edid.h | 18 --
  2 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 12893e7be89bb..500279a82167a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4747,6 +4747,80 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 
**sadb)
  }
  EXPORT_SYMBOL(drm_edid_to_speaker_allocation);
  
+/**

+ * drm_cea_sad_get_sample_rate - Extract the sample rate from cea_sad
+ * @sad: Pointer to the cea_sad struct
+ *
+ * Extracts the cea_sad frequency field and returns the sample rate in Hz.
+ *
+ * Return: Sample rate in Hz or a negative errno if parsing failed.
+ */
+int drm_cea_sad_get_sample_rate(const struct cea_sad *sad)
+{
+   switch (sad->freq) {
+   case DRM_CEA_SAD_FREQ_32KHZ:
+   return 32000;
+   case DRM_CEA_SAD_FREQ_44KHZ:
+   return 44100;
+   case DRM_CEA_SAD_FREQ_48KHZ:
+   return 48000;
+   case DRM_CEA_SAD_FREQ_88KHZ:
+   return 88200;
+   case DRM_CEA_SAD_FREQ_96KHZ:
+   return 96000;
+   case DRM_CEA_SAD_FREQ_176KHZ:
+   return 176400;
+   case DRM_CEA_SAD_FREQ_192KHZ:
+   return 192000;
+   default:
+   return -EINVAL;
+   }
+}
+EXPORT_SYMBOL(drm_cea_sad_get_sample_rate);
+
+static bool drm_cea_sad_is_uncompressed(const struct cea_sad *sad)
+{
+   switch (sad->format) {
+   case HDMI_AUDIO_CODING_TYPE_STREAM:
+   case HDMI_AUDIO_CODING_TYPE_PCM:
+   return true;
+   default:
+   return false;
+   }
+}
+
+/**
+ * drm_cea_sad_get_uncompressed_word_length - Extract word length
+ * @sad: Pointer to the cea_sad struct
+ *
+ * Extracts the cea_sad byte2 field and returns the word length for an
+ * uncompressed stream.
+ *
+ * Note: This function may only be called for uncompressed audio.
+ *
+ * Return: Word length in bits or a negative errno if parsing failed.
+ */
+int drm_cea_sad_get_uncompressed_word_length(const struct cea_sad *sad)
+{
+   if (!drm_cea_sad_is_uncompressed(sad)) {
+   DRM_WARN("Unable to get the uncompressed word length for a 
compressed format: %u\n",
+sad->format);
+   return -EINVAL;
+   }
+
+   switch (sad->byte2) {
+   case DRM_CEA_SAD_UNCOMPRESSED_WORD_16BIT:
+   return 16;
+   case DRM_CEA_SAD_UNCOMPRESSED_WORD_20BIT:
+   return 20;
+   case DRM_CEA_SAD_UNCOMPRESSED_WORD_24BIT:
+   return 24;
+   default:
+   return -EINVAL;
+   }
+}
+EXPORT_SYMBOL(drm_cea_sad_get_uncompressed_word_length);
+
  /**
   * drm_av_sync_delay - compute the HDMI/DP sink audio-video sync delay
   * @connector: connector associated with the HDMI/DP sink
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 18f6c700f6d02..a30452b313979 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -361,12 +361,24 @@ struct edid {
  
  /* Short Audio Descriptor */

  struct cea_sad {
-   u8 format;
+   u8 format; /* See HDMI_AUDIO_CODING_TYPE_* */


Hello Guillaume,

since you're adding comments to all the rest of the struct members,
I think that a small effort to instead convert this to kerneldoc is
totally worth it.
Can you please do that?

Thanks,
Angelo


u8 channels; /* max number of channels - 1 */
-   u8 freq;
+   u8 freq; /* See CEA_SAD_FREQ_* */
u8 byte2; /* meaning depends on format */
  };
  
+#define DRM_CEA_SAD_FREQ_32KHZ  BIT(0)

+#define DRM_CEA_SAD_FREQ_44KHZ  BIT(1)
+#define DRM_CEA_SAD_FREQ_48KHZ  BIT(2)
+#define DRM_CEA_SAD_FREQ_88KHZ  BIT(3)
+#define DRM_CEA_SAD_FREQ_96KHZ  BIT(4)
+#define DRM_CEA_SAD_FREQ_176KHZ BIT(5)
+#define DRM_CEA_SAD_FREQ_192KHZ BIT(6)
+
+#define DRM_CEA_SAD_UNCOMPRESSED_WORD_16BIT BIT(0)
+#define DRM_CEA_SAD_UNCOMPRESSED_WORD_20BIT BIT(1)
+#define DRM_CEA_SAD_UNCOMPRESSED_WORD_24BIT BIT(2)
+
  struct drm_encoder;
  struct drm_connector;
  struct drm_connector_state;
@@ -374,6 +386,8 @@ struct drm_display_mode;
  
  int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);

  int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb);
+int drm_cea_sad_get_sample_rate(const struct cea_sad *sad);
+int drm_cea_sad_get_uncompressed_word_length(const struct cea_sad *sad);
  int drm_av_sync_delay(struct drm_connector *connector,
  const struct drm_display_mode *mode);
  





Re: [PATCH v8 06/19] drm/mediatek: dpi: implement a CK/DE pol toggle in board config

2022-02-21 Thread AngeloGioacchino Del Regno

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

Adds a bit of flexibility to support boards without CK/DE pol support

Signed-off-by: Guillaume Ranquet 


s/board/SoC/g

After the change,
Reviewed-by: AngeloGioacchino Del Regno 




Re: [PATCH v8 05/19] drm/mediatek: dpi: move dpi limits to board config

2022-02-21 Thread AngeloGioacchino Del Regno

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

Add flexibility by moving the dpi limits to the board config


s/board/SoC/g

After the change,
Reviewed-by: AngeloGioacchino Del Regno 




Signed-off-by: Guillaume Ranquet 




Re: [PATCH v8 13/19] drm/mediatek: dpi: Add dpintf support

2022-02-21 Thread AngeloGioacchino Del Regno

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

dpintf is the displayport interface hardware unit. This unit is similar
to dpi and can reuse most of the code.

This patch adds support for mt8195-dpintf to this dpi driver. Main
differences are:
  - Some features/functional components are not available for dpintf
which are now excluded from code execution once is_dpintf is set
  - dpintf can and needs to choose between different clockdividers based
on the clockspeed. This is done by choosing a different clock parent.
  - There are two additional clocks that need to be managed. These are
only set for dpintf and will be set to NULL if not supplied. The
clk_* calls handle these as normal clocks then.
  - Some register contents differ slightly between the two components. To
work around this I added register bits/masks with a DPINTF_ prefix
and use them where different.

Based on a separate driver for dpintf created by
Jason-JH.Lin .

Signed-off-by: Markus Schneider-Pargmann 
Signed-off-by: Guillaume Ranquet 
---
  drivers/gpu/drm/mediatek/mtk_dpi.c  | 164 +---
  drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  38 +
  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   8 +
  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
  drivers/gpu/drm/mediatek/mtk_drm_drv.c  |   5 +-
  include/linux/soc/mediatek/mtk-mmsys.h  |   2 +
  6 files changed, 198 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index be99399faf1bb..c5639ada868f8 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -63,6 +63,14 @@ enum mtk_dpi_out_color_format {
MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL
  };
  
+enum TVDPLL_CLK {

+   TVDPLL_PLL = 0,
+   TVDPLL_D2 = 2,
+   TVDPLL_D4 = 4,
+   TVDPLL_D8 = 8,
+   TVDPLL_D16 = 16,
+};
+
  struct mtk_dpi {
struct drm_encoder encoder;
struct drm_bridge bridge;
@@ -71,8 +79,10 @@ struct mtk_dpi {
void __iomem *regs;
struct device *dev;
struct clk *engine_clk;
+   struct clk *dpi_ck_cg;
struct clk *pixel_clk;
struct clk *tvd_clk;
+   struct clk *pclk_src[5];
int irq;
struct drm_display_mode mode;
const struct mtk_dpi_conf *conf;
@@ -126,6 +136,7 @@ struct mtk_dpi_conf {
const u32 *output_fmts;
u32 num_output_fmts;
bool is_ck_de_pol;
+   bool is_dpintf;
bool swap_input_support;
// Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift)
u32 dimension_mask;
@@ -384,6 +395,25 @@ static void mtk_dpi_config_disable_edge(struct mtk_dpi 
*dpi)
mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN);
  }
  
+static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi, enum mtk_dpi_out_color_format format)

+{
+   u32 matrix_sel = 0;
+
+   switch (format) {
+   case MTK_DPI_COLOR_FORMAT_YCBCR_422:
+   case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
+   case MTK_DPI_COLOR_FORMAT_YCBCR_444:
+   case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:
+   case MTK_DPI_COLOR_FORMAT_XV_YCC:
+   if (dpi->mode.hdisplay <= 720)
+   matrix_sel = 0x2;
+   break;
+   default:
+   break;
+   }
+   mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel, INT_MATRIX_SEL_MASK);
+}
+
  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
enum mtk_dpi_out_color_format format)
  {
@@ -391,6 +421,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
(format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
mtk_dpi_config_yuv422_enable(dpi, false);
mtk_dpi_config_csc_enable(dpi, true);
+   mtk_dpi_matrix_sel(dpi, format);
if (dpi->conf->swap_input_support)
mtk_dpi_config_swap_input(dpi, false);
mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_BGR);
@@ -398,6 +429,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
   (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
mtk_dpi_config_yuv422_enable(dpi, true);
mtk_dpi_config_csc_enable(dpi, true);
+   mtk_dpi_matrix_sel(dpi, format);
if (dpi->conf->swap_input_support)
mtk_dpi_config_swap_input(dpi, true);
mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_RGB);
@@ -438,6 +470,8 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi)
mtk_dpi_disable(dpi);
clk_disable_unprepare(dpi->pixel_clk);
clk_disable_unprepare(dpi->engine_clk);
+   clk_disable_unprepare(dpi->dpi_ck_cg);
+   clk_disable_unprepare(dpi->tvd_clk);
  }
  
  static int mtk_dpi_power_on(struct mtk_dpi *dpi)

@@ -447,12 +481,24 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi)
   

Re: [PATCH v8 08/19] drm/mediatek: dpi: move dimension mask to board config

2022-02-21 Thread AngeloGioacchino Del Regno

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

Add flexibility by moving the dimension mask to the board config



s/board/SoC/g



Signed-off-by: Guillaume Ranquet 
---
  drivers/gpu/drm/mediatek/mtk_dpi.c | 26 --
  1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 454f8563efae4..8ca3455ed64ee 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -127,6 +127,8 @@ struct mtk_dpi_conf {
u32 num_output_fmts;
bool is_ck_de_pol;
bool swap_input_support;
+   // Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift)


Also, C-style comments, please.

After the change,
Reviewed-by: AngeloGioacchino Del Regno 





+   u32 dimension_mask;
const struct mtk_dpi_yc_limit *limit;
  };
  




Re: [PATCH v8 09/19] drm/mediatek: dpi: move dimension_mask to board config

2022-02-21 Thread AngeloGioacchino Del Regno

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

Add flexibility by moving the dimension mask to board config



You should really fix both title and description for this commit,
as this is moving hvsize_mask, not dimension_mask.
Also, s/board/SoC/g.


Signed-off-by: Guillaume Ranquet 
---
  drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 8ca3455ed64ee..0d3acd08ea358 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -129,6 +129,8 @@ struct mtk_dpi_conf {
bool swap_input_support;
// Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift)
u32 dimension_mask;
+   // Mask used for HSIZE and VSIZE (no shift)


C-style comments please


+   u32 hvsize_mask;
const struct mtk_dpi_yc_limit *limit;
  };
  




Re: [PATCH v8 10/19] drm/mediatek: dpi: move swap_shift to board config

2022-02-21 Thread AngeloGioacchino Del Regno

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

Add flexibility by moving the swap shift value to board config



Same board->SoC as all the other commits..


Signed-off-by: Guillaume Ranquet 
---
  drivers/gpu/drm/mediatek/mtk_dpi.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 0d3acd08ea358..ec221e24e0fee 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -131,6 +131,7 @@ struct mtk_dpi_conf {
u32 dimension_mask;
// Mask used for HSIZE and VSIZE (no shift)
u32 hvsize_mask;
+   u32 channel_swap_shift;
const struct mtk_dpi_yc_limit *limit;
  };
  
@@ -349,7 +350,8 @@ static void mtk_dpi_config_channel_swap(struct mtk_dpi *dpi,

break;
}
  
-	mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, val << CH_SWAP, CH_SWAP_MASK);

+   mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, val << CH_SWAP,
+CH_SWAP_MASK << dpi->conf->channel_swap_shift);


I would say that this should be instead:
mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING,

 val << dpi->conf->channel_swap_shift, CH_SWAP_MASK);



  }
  
  static void mtk_dpi_config_yuv422_enable(struct mtk_dpi *dpi, bool enable)

@@ -821,6 +823,7 @@ static const struct mtk_dpi_conf mt8173_conf = {
.swap_input_support = true,
.dimension_mask = HPW_MASK,
.hvsize_mask = HSIZE_MASK,
+   .channel_swap_shift = CH_SWAP,
.limit = &mtk_dpi_limit,
  };
  
@@ -835,6 +838,7 @@ static const struct mtk_dpi_conf mt2701_conf = {

.swap_input_support = true,
.dimension_mask = HPW_MASK,
.hvsize_mask = HSIZE_MASK,
+   .channel_swap_shift = CH_SWAP,
.limit = &mtk_dpi_limit,
  };
  
@@ -848,6 +852,7 @@ static const struct mtk_dpi_conf mt8183_conf = {

.swap_input_support = true,
.dimension_mask = HPW_MASK,
.hvsize_mask = HSIZE_MASK,
+   .channel_swap_shift = CH_SWAP,
.limit = &mtk_dpi_limit,
  };
  
@@ -861,6 +866,7 @@ static const struct mtk_dpi_conf mt8192_conf = {

.swap_input_support = true,
.dimension_mask = HPW_MASK,
.hvsize_mask = HSIZE_MASK,
+   .channel_swap_shift = CH_SWAP,
.limit = &mtk_dpi_limit,
  };
  






  1   2   3   >