Re: [Freedreno] [PATCH v9 12/15] drm/msm: Add deadline based boost support

2023-03-03 Thread Dmitry Baryshkov

On 03/03/2023 19:03, Rob Clark wrote:

On Fri, Mar 3, 2023 at 2:10 AM Dmitry Baryshkov
 wrote:


On 03/03/2023 01:53, Rob Clark wrote:

From: Rob Clark 

Track the nearest deadline on a fence timeline and set a timer to expire
shortly before to trigger boost if the fence has not yet been signaled.

v2: rebase

Signed-off-by: Rob Clark 
---
   drivers/gpu/drm/msm/msm_fence.c | 74 +
   drivers/gpu/drm/msm/msm_fence.h | 20 +
   2 files changed, 94 insertions(+)


Reviewed-by: Dmitry Baryshkov 

A small question: do we boost to fit into the deadline or to miss the
deadline for as little as possible? If the former is the case, we might
need to adjust 3ms depending on the workload.


The goal is as much to run with higher clock on the next frame as it
is to not miss a deadline.  Ie. we don't want devfreq to come to the
conclusion that running at <50% clks is best due to the amount of
utilization caused by missing ever other vblank.


Ack, thanks for the explanation.



But 3ms is mostly just "seems like a good compromise" value.  It might change.

BR,
-R



diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 56641408ea74..51b461f32103 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -8,6 +8,35 @@

   #include "msm_drv.h"
   #include "msm_fence.h"
+#include "msm_gpu.h"
+
+static struct msm_gpu *fctx2gpu(struct msm_fence_context *fctx)
+{
+ struct msm_drm_private *priv = fctx->dev->dev_private;
+ return priv->gpu;
+}
+
+static enum hrtimer_restart deadline_timer(struct hrtimer *t)
+{
+ struct msm_fence_context *fctx = container_of(t,
+ struct msm_fence_context, deadline_timer);
+
+ kthread_queue_work(fctx2gpu(fctx)->worker, >deadline_work);
+
+ return HRTIMER_NORESTART;
+}
+
+static void deadline_work(struct kthread_work *work)
+{
+ struct msm_fence_context *fctx = container_of(work,
+ struct msm_fence_context, deadline_work);
+
+ /* If deadline fence has already passed, nothing to do: */
+ if (msm_fence_completed(fctx, fctx->next_deadline_fence))
+ return;
+
+ msm_devfreq_boost(fctx2gpu(fctx), 2);
+}


   struct msm_fence_context *
@@ -36,6 +65,13 @@ msm_fence_context_alloc(struct drm_device *dev, volatile 
uint32_t *fenceptr,
   fctx->completed_fence = fctx->last_fence;
   *fctx->fenceptr = fctx->last_fence;

+ hrtimer_init(>deadline_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
+ fctx->deadline_timer.function = deadline_timer;
+
+ kthread_init_work(>deadline_work, deadline_work);
+
+ fctx->next_deadline = ktime_get();
+
   return fctx;
   }

@@ -62,6 +98,8 @@ void msm_update_fence(struct msm_fence_context *fctx, 
uint32_t fence)
   spin_lock_irqsave(>spinlock, flags);
   if (fence_after(fence, fctx->completed_fence))
   fctx->completed_fence = fence;
+ if (msm_fence_completed(fctx, fctx->next_deadline_fence))
+ hrtimer_cancel(>deadline_timer);
   spin_unlock_irqrestore(>spinlock, flags);
   }

@@ -92,10 +130,46 @@ static bool msm_fence_signaled(struct dma_fence *fence)
   return msm_fence_completed(f->fctx, f->base.seqno);
   }

+static void msm_fence_set_deadline(struct dma_fence *fence, ktime_t deadline)
+{
+ struct msm_fence *f = to_msm_fence(fence);
+ struct msm_fence_context *fctx = f->fctx;
+ unsigned long flags;
+ ktime_t now;
+
+ spin_lock_irqsave(>spinlock, flags);
+ now = ktime_get();
+
+ if (ktime_after(now, fctx->next_deadline) ||
+ ktime_before(deadline, fctx->next_deadline)) {
+ fctx->next_deadline = deadline;
+ fctx->next_deadline_fence =
+ max(fctx->next_deadline_fence, (uint32_t)fence->seqno);
+
+ /*
+  * Set timer to trigger boost 3ms before deadline, or
+  * if we are already less than 3ms before the deadline
+  * schedule boost work immediately.
+  */
+ deadline = ktime_sub(deadline, ms_to_ktime(3));
+
+ if (ktime_after(now, deadline)) {
+ kthread_queue_work(fctx2gpu(fctx)->worker,
+ >deadline_work);
+ } else {
+ hrtimer_start(>deadline_timer, deadline,
+ HRTIMER_MODE_ABS);
+ }
+ }
+
+ spin_unlock_irqrestore(>spinlock, flags);
+}
+
   static const struct dma_fence_ops msm_fence_ops = {
   .get_driver_name = msm_fence_get_driver_name,
   .get_timeline_name = msm_fence_get_timeline_name,
   .signaled = msm_fence_signaled,
+ .set_deadline = msm_fence_set_deadline,
   };

   struct dma_fence *
diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h
index 7f1798c54cd1..cdaebfb94f5c 100644
--- a/drivers/gpu/drm/msm/msm_fence.h
+++ b/drivers/gpu/drm/msm/msm_fence.h
@@ -52,6 +52,26 

Re: [Freedreno] [PATCH] dt-bindings: yamllint: Require a space after a comment '#'

2023-03-03 Thread Jakub Kicinski
On Fri,  3 Mar 2023 15:42:23 -0600 Rob Herring wrote:
> Enable yamllint to check the prefered commenting style of requiring a
> space after a comment character '#'. Fix the cases in the tree which
> have a warning with this enabled. Most cases just need a space after the
> '#'. A couple of cases with comments which were not intended to be
> comments are revealed. Those were in ti,sa2ul.yaml, ti,cal.yaml, and
> brcm,bcmgenet.yaml.
> 
> Signed-off-by: Rob Herring 

Acked-by: Jakub Kicinski 


Re: [Freedreno] [PATCH] dt-bindings: yamllint: Require a space after a comment '#'

2023-03-03 Thread Dmitry Baryshkov

On 03/03/2023 23:42, Rob Herring wrote:

Enable yamllint to check the prefered commenting style of requiring a
space after a comment character '#'. Fix the cases in the tree which
have a warning with this enabled. Most cases just need a space after the
'#'. A couple of cases with comments which were not intended to be
comments are revealed. Those were in ti,sa2ul.yaml, ti,cal.yaml, and
brcm,bcmgenet.yaml.

Signed-off-by: Rob Herring 
---
Cc: Krzysztof Kozlowski 
Cc: Stephen Boyd 
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: Rob Clark 
Cc: Abhinav Kumar 
Cc: Dmitry Baryshkov 
Cc: Sean Paul 
Cc: Thomas Gleixner 
Cc: Marc Zyngier 
Cc: Mauro Carvalho Chehab 
Cc: Eric Dumazet 
Cc: Jakub Kicinski 
Cc: Paolo Abeni 
Cc: Andrew Lunn 
Cc: Heiner Kallweit 
Cc: Vinod Koul 
Cc: Kishon Vijay Abraham I 
Cc: Mark Brown 
Cc: Conor Dooley 
Cc: linux-...@vger.kernel.org
Cc: linux-cry...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-...@lists.infradead.org
Cc: linux-g...@vger.kernel.org
Cc: alsa-de...@alsa-project.org
Cc: linux-ri...@lists.infradead.org
Cc: linux-...@vger.kernel.org
---
  Documentation/devicetree/bindings/.yamllint   |  2 +-
  .../bindings/clock/qcom,a53pll.yaml   |  4 ++--
  .../devicetree/bindings/crypto/ti,sa2ul.yaml  |  4 ++--
  .../bindings/display/msm/qcom,mdp5.yaml   |  2 +-
  .../interrupt-controller/arm,gic.yaml |  4 ++--
  .../loongson,pch-msi.yaml |  2 +-
  .../bindings/media/renesas,vin.yaml   |  4 ++--
  .../devicetree/bindings/media/ti,cal.yaml |  4 ++--
  .../bindings/net/brcm,bcmgenet.yaml   |  2 --
  .../bindings/net/cortina,gemini-ethernet.yaml |  6 ++---
  .../devicetree/bindings/net/mdio-gpio.yaml|  4 ++--
  .../phy/marvell,armada-cp110-utmi-phy.yaml|  2 +-
  .../bindings/phy/phy-stm32-usbphyc.yaml   |  2 +-
  .../phy/qcom,sc7180-qmp-usb3-dp-phy.yaml  |  2 +-
  .../bindings/pinctrl/pinctrl-mt8192.yaml  |  2 +-
  .../regulator/nxp,pca9450-regulator.yaml  |  8 +++
  .../regulator/rohm,bd71828-regulator.yaml | 20 
  .../regulator/rohm,bd71837-regulator.yaml |  6 ++---
  .../regulator/rohm,bd71847-regulator.yaml |  6 ++---
  .../bindings/soc/renesas/renesas.yaml |  2 +-
  .../devicetree/bindings/soc/ti/ti,pruss.yaml  |  2 +-
  .../bindings/sound/amlogic,axg-tdm-iface.yaml |  2 +-
  .../bindings/sound/qcom,lpass-rx-macro.yaml   |  4 ++--
  .../bindings/sound/qcom,lpass-tx-macro.yaml   |  4 ++--
  .../bindings/sound/qcom,lpass-va-macro.yaml   |  4 ++--
  .../sound/qcom,q6dsp-lpass-ports.yaml |  2 +-
  .../bindings/sound/simple-card.yaml   | 24 +--
  .../bindings/spi/microchip,mpfs-spi.yaml  |  2 +-
  28 files changed, 65 insertions(+), 67 deletions(-)


Reviewed-by: Dmitry Baryshkov  # drm/msm
(and other Qualcom-specific schemas)

--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v3] drm/msm/dp: check core_initialized flag at both host_init() and host_deinit()

2023-03-03 Thread Dmitry Baryshkov

On 04/03/2023 00:45, Kuogee Hsieh wrote:


On 3/2/2023 11:04 AM, Dmitry Baryshkov wrote:
On Thu, 2 Mar 2023 at 20:41, Kuogee Hsieh  
wrote:


On 3/1/2023 1:15 PM, Dmitry Baryshkov wrote:

On 01/03/2023 18:57, Kuogee Hsieh wrote:

On 2/28/2023 6:16 PM, Dmitry Baryshkov wrote:

On Wed, 1 Mar 2023 at 02:17, Kuogee Hsieh 
wrote:

There is a reboot/suspend test case where system suspend is forced
during system booting up. Since dp_display_host_init() of external
DP is executed at hpd thread context, this test case may created a
scenario that dp_display_host_deinit() from pm_suspend() run before
dp_display_host_init() if hpd thread has no chance to run during
booting up while suspend request command was issued. At this 
scenario

system will crash at aux register access at dp_display_host_deinit()
since aux clock had not yet been enabled by dp_display_host_init().
Therefore we have to ensure aux clock enabled by checking
core_initialized flag before access aux registers at pm_suspend.

Can a call to dp_display_host_init() be moved from
dp_display_config_hpd() to dp_display_bind()?

yes,  Sankeerth's  "drm/msm/dp: enable pm_runtime support for dp
driver" patch is doing that which is under review.

https://patchwork.freedesktop.org/patch/523879/?series=114297=1

No, he is doing another thing. He is moving these calls to pm_runtime
callbacks, not to the dp_display_bind().


Related question: what is the primary reason for having
EV_HPD_INIT_SETUP and calling dp_display_config_hpd() via the event
thread? Does DP driver really depend on DPU irqs being installed? As
far as I understand, DP device uses MDSS interrupts and those IRQs 
are

available and working at the time of dp_display_probe() /
dp_display_bind().

HDP gpio pin has to run through DP aux module 100ms denouncing logic
and have its mask bits.

Therefore DP irq has to be enabled to receive DP isr with mask bits 
set.

So... DP irq is enabled by the MDSS, not by the DPU. Again, why does
DP driver depend on DPU irqs being installed?

sorry, previously i mis understand your question -- why does DP driver
depend on DPU irqs being installed?

now, I think you are asking why  dpu_irq_postinstall() ==>
msm_dp_irq_postinstall() ==> event_thread ==> dp_display_config_hdp()
==> enable_irq(dp->irq)

With the below test i had run, i think the reason is to make sure
dp->irq be requested before enable it.

I just run the execution timing order test and collect execution order
as descending order at below,

1) dp_display_probe() -- start

2) dp_display_bind()

3) msm_dp_modeset_init()  ==> dp_display_request_irq() ==>
dp_display_get_next_bridge()

4) dpu_irq_postinstall() ==> msm_dp_irq_postinstall() ==>
enable_irq(dp->irq)

5) dp_display_probe() -- end

dp->irq is request at msm_dp_modeset_init() and enabled after.

Should be moved to probe.


That bring up the issue to move DP's dp_display_host_init() executed at
dp_display_bind().

Since eDP have dp_dispaly_host_init() executed at
dp_display_get_next_bridge() which executed after dp_display_bind().

If moved DP's dp_display_host_init() to dp_dispaly_bind() which means DP
will be ready to receive HPD irq before eDP ready.

And the AUX bus population should also be moved to probe(), which
means we should call dp_display_host_init() from probe() too.
Having aux_bus_populate in probe would allow moving component_add() to
the done_probing() callback, making probe/defer case more robust


This may create some uncertainties at execution flow and complicate
things up.

Hopefully the changes suggested above will make it simpler.


ok, I will create another patch to


patchset



1) move dp_display_host_init() to probe()

2) move component_add() to done_probing() for eDP

3) keep DP as simple platform device (component_add() still executed in 
probe())


4) move devm_request_irq() to probe, add IRQF_NO_AUTOEN instead of 
calling disable_irq() right after request_irq()


5) drop DP_HPD_INIT_SETUP and related code



Meanwhile, can you approve this patch so that it will not block our 
internal daily testing?


Quoting your commit message: "Since dp_display_host_init() of external
DP is executed at hpd thread context...". After these changes the 
mentioned function will no longer be executed from the hpd thread. So, 
let's rework the probe/init sequence first, then we can see if this 
patch is still necessary and how should it look.


--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v3] drm/msm/dp: check core_initialized flag at both host_init() and host_deinit()

2023-03-03 Thread Kuogee Hsieh



On 3/2/2023 11:04 AM, Dmitry Baryshkov wrote:

On Thu, 2 Mar 2023 at 20:41, Kuogee Hsieh  wrote:


On 3/1/2023 1:15 PM, Dmitry Baryshkov wrote:

On 01/03/2023 18:57, Kuogee Hsieh wrote:

On 2/28/2023 6:16 PM, Dmitry Baryshkov wrote:

On Wed, 1 Mar 2023 at 02:17, Kuogee Hsieh 
wrote:

There is a reboot/suspend test case where system suspend is forced
during system booting up. Since dp_display_host_init() of external
DP is executed at hpd thread context, this test case may created a
scenario that dp_display_host_deinit() from pm_suspend() run before
dp_display_host_init() if hpd thread has no chance to run during
booting up while suspend request command was issued. At this scenario
system will crash at aux register access at dp_display_host_deinit()
since aux clock had not yet been enabled by dp_display_host_init().
Therefore we have to ensure aux clock enabled by checking
core_initialized flag before access aux registers at pm_suspend.

Can a call to dp_display_host_init() be moved from
dp_display_config_hpd() to dp_display_bind()?

yes,  Sankeerth's  "drm/msm/dp: enable pm_runtime support for dp
driver" patch is doing that which is under review.

https://patchwork.freedesktop.org/patch/523879/?series=114297=1

No, he is doing another thing. He is moving these calls to pm_runtime
callbacks, not to the dp_display_bind().


Related question: what is the primary reason for having
EV_HPD_INIT_SETUP and calling dp_display_config_hpd() via the event
thread? Does DP driver really depend on DPU irqs being installed? As
far as I understand, DP device uses MDSS interrupts and those IRQs are
available and working at the time of dp_display_probe() /
dp_display_bind().

HDP gpio pin has to run through DP aux module 100ms denouncing logic
and have its mask bits.

Therefore DP irq has to be enabled to receive DP isr with mask bits set.

So... DP irq is enabled by the MDSS, not by the DPU. Again, why does
DP driver depend on DPU irqs being installed?

sorry, previously i mis understand your question -- why does DP driver
depend on DPU irqs being installed?

now, I think you are asking why  dpu_irq_postinstall() ==>
msm_dp_irq_postinstall() ==> event_thread ==> dp_display_config_hdp()
==> enable_irq(dp->irq)

With the below test i had run, i think the reason is to make sure
dp->irq be requested before enable it.

I just run the execution timing order test and collect execution order
as descending order at below,

1) dp_display_probe() -- start

2) dp_display_bind()

3) msm_dp_modeset_init()  ==> dp_display_request_irq() ==>
dp_display_get_next_bridge()

4) dpu_irq_postinstall() ==> msm_dp_irq_postinstall() ==>
enable_irq(dp->irq)

5) dp_display_probe() -- end

dp->irq is request at msm_dp_modeset_init() and enabled after.

Should be moved to probe.


That bring up the issue to move DP's dp_display_host_init() executed at
dp_display_bind().

Since eDP have dp_dispaly_host_init() executed at
dp_display_get_next_bridge() which executed after dp_display_bind().

If moved DP's dp_display_host_init() to dp_dispaly_bind() which means DP
will be ready to receive HPD irq before eDP ready.

And the AUX bus population should also be moved to probe(), which
means we should call dp_display_host_init() from probe() too.
Having aux_bus_populate in probe would allow moving component_add() to
the done_probing() callback, making probe/defer case more robust


This may create some uncertainties at execution flow and complicate
things up.

Hopefully the changes suggested above will make it simpler.


ok, I will create another patch to

1) move dp_display_host_init() to probe()

2) move component_add() to done_probing() for eDP

3) keep DP as simple platform device (component_add() still executed in 
probe())


Meanwhile, can you approve this patch so that it will not block our 
internal daily testing?








Re: [Freedreno] [PATCH] dt-bindings: yamllint: Require a space after a comment '#'

2023-03-03 Thread Conor Dooley
On Fri, Mar 03, 2023 at 03:42:23PM -0600, Rob Herring wrote:
> Enable yamllint to check the prefered commenting style of requiring a
> space after a comment character '#'. Fix the cases in the tree which
> have a warning with this enabled. Most cases just need a space after the
> '#'. A couple of cases with comments which were not intended to be
> comments are revealed. Those were in ti,sa2ul.yaml, ti,cal.yaml, and
> brcm,bcmgenet.yaml.
> 
> Signed-off-by: Rob Herring 
> ---

> Cc: Conor Dooley 

> diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml 
> b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> index 1051690e3753..74a817cc7d94 100644
> --- a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
> @@ -22,7 +22,7 @@ properties:
>- items:
>- const: microchip,mpfs-qspi
>- const: microchip,coreqspi-rtl-v2
> -  - const: microchip,coreqspi-rtl-v2 #FPGA QSPI
> +  - const: microchip,coreqspi-rtl-v2 # FPGA QSPI
>- const: microchip,mpfs-spi

I had to think for a minute as to what that comment even meant...
Reviewed-by: Conor Dooley 

Thanks,
Conor.


signature.asc
Description: PGP signature


[Freedreno] [PATCH] dt-bindings: yamllint: Require a space after a comment '#'

2023-03-03 Thread Rob Herring
Enable yamllint to check the prefered commenting style of requiring a
space after a comment character '#'. Fix the cases in the tree which
have a warning with this enabled. Most cases just need a space after the
'#'. A couple of cases with comments which were not intended to be
comments are revealed. Those were in ti,sa2ul.yaml, ti,cal.yaml, and
brcm,bcmgenet.yaml.

Signed-off-by: Rob Herring 
---
Cc: Krzysztof Kozlowski 
Cc: Stephen Boyd 
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: Rob Clark 
Cc: Abhinav Kumar 
Cc: Dmitry Baryshkov 
Cc: Sean Paul 
Cc: Thomas Gleixner 
Cc: Marc Zyngier 
Cc: Mauro Carvalho Chehab 
Cc: Eric Dumazet 
Cc: Jakub Kicinski 
Cc: Paolo Abeni 
Cc: Andrew Lunn 
Cc: Heiner Kallweit 
Cc: Vinod Koul 
Cc: Kishon Vijay Abraham I 
Cc: Mark Brown 
Cc: Conor Dooley 
Cc: linux-...@vger.kernel.org
Cc: linux-cry...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-...@lists.infradead.org
Cc: linux-g...@vger.kernel.org
Cc: alsa-de...@alsa-project.org
Cc: linux-ri...@lists.infradead.org
Cc: linux-...@vger.kernel.org
---
 Documentation/devicetree/bindings/.yamllint   |  2 +-
 .../bindings/clock/qcom,a53pll.yaml   |  4 ++--
 .../devicetree/bindings/crypto/ti,sa2ul.yaml  |  4 ++--
 .../bindings/display/msm/qcom,mdp5.yaml   |  2 +-
 .../interrupt-controller/arm,gic.yaml |  4 ++--
 .../loongson,pch-msi.yaml |  2 +-
 .../bindings/media/renesas,vin.yaml   |  4 ++--
 .../devicetree/bindings/media/ti,cal.yaml |  4 ++--
 .../bindings/net/brcm,bcmgenet.yaml   |  2 --
 .../bindings/net/cortina,gemini-ethernet.yaml |  6 ++---
 .../devicetree/bindings/net/mdio-gpio.yaml|  4 ++--
 .../phy/marvell,armada-cp110-utmi-phy.yaml|  2 +-
 .../bindings/phy/phy-stm32-usbphyc.yaml   |  2 +-
 .../phy/qcom,sc7180-qmp-usb3-dp-phy.yaml  |  2 +-
 .../bindings/pinctrl/pinctrl-mt8192.yaml  |  2 +-
 .../regulator/nxp,pca9450-regulator.yaml  |  8 +++
 .../regulator/rohm,bd71828-regulator.yaml | 20 
 .../regulator/rohm,bd71837-regulator.yaml |  6 ++---
 .../regulator/rohm,bd71847-regulator.yaml |  6 ++---
 .../bindings/soc/renesas/renesas.yaml |  2 +-
 .../devicetree/bindings/soc/ti/ti,pruss.yaml  |  2 +-
 .../bindings/sound/amlogic,axg-tdm-iface.yaml |  2 +-
 .../bindings/sound/qcom,lpass-rx-macro.yaml   |  4 ++--
 .../bindings/sound/qcom,lpass-tx-macro.yaml   |  4 ++--
 .../bindings/sound/qcom,lpass-va-macro.yaml   |  4 ++--
 .../sound/qcom,q6dsp-lpass-ports.yaml |  2 +-
 .../bindings/sound/simple-card.yaml   | 24 +--
 .../bindings/spi/microchip,mpfs-spi.yaml  |  2 +-
 28 files changed, 65 insertions(+), 67 deletions(-)

diff --git a/Documentation/devicetree/bindings/.yamllint 
b/Documentation/devicetree/bindings/.yamllint
index 214abd3ec440..4abe9f0a1d46 100644
--- a/Documentation/devicetree/bindings/.yamllint
+++ b/Documentation/devicetree/bindings/.yamllint
@@ -19,7 +19,7 @@ rules:
   colons: {max-spaces-before: 0, max-spaces-after: 1}
   commas: {min-spaces-after: 1, max-spaces-after: 1}
   comments:
-require-starting-space: false
+require-starting-space: true
 min-spaces-from-content: 1
   comments-indentation: disable
   document-start:
diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml 
b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
index 525ebaa93c85..64bfd0f5d4d0 100644
--- a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
@@ -45,14 +45,14 @@ required:
 additionalProperties: false
 
 examples:
-  #Example 1 - A53 PLL found on MSM8916 devices
+  # Example 1 - A53 PLL found on MSM8916 devices
   - |
 a53pll: clock@b016000 {
 compatible = "qcom,msm8916-a53pll";
 reg = <0xb016000 0x40>;
 #clock-cells = <0>;
 };
-  #Example 2 - A53 PLL found on IPQ6018 devices
+  # Example 2 - A53 PLL found on IPQ6018 devices
   - |
 a53pll_ipq: clock-controller@b116000 {
 compatible = "qcom,ipq6018-a53pll";
diff --git a/Documentation/devicetree/bindings/crypto/ti,sa2ul.yaml 
b/Documentation/devicetree/bindings/crypto/ti,sa2ul.yaml
index 0c15fefb6671..77ec8bc70bf7 100644
--- a/Documentation/devicetree/bindings/crypto/ti,sa2ul.yaml
+++ b/Documentation/devicetree/bindings/crypto/ti,sa2ul.yaml
@@ -26,8 +26,8 @@ properties:
   dmas:
 items:
   - description: TX DMA Channel
-  - description: RX DMA Channel #1
-  - description: RX DMA Channel #2
+  - description: 'RX DMA Channel #1'
+  - description: 'RX DMA Channel #2'
 
   dma-names:
 items:
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml 
b/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml
index ef461ad6ce4a..a763cf8da122 100644
--- 

Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Matt Turner
On Fri, Mar 3, 2023 at 10:08 AM Tvrtko Ursulin
 wrote:
>
>
> On 03/03/2023 14:48, Rob Clark wrote:
> > On Fri, Mar 3, 2023 at 1:58 AM Tvrtko Ursulin
> >  wrote:
> >>
> >>
> >> On 03/03/2023 03:21, Rodrigo Vivi wrote:
> >>> On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
>  From: Rob Clark 
> 
> >>>
> >>> missing some wording here...
> >>>
>  v2: rebase
> 
>  Signed-off-by: Rob Clark 
>  ---
> drivers/gpu/drm/i915/i915_request.c | 20 
> 1 file changed, 20 insertions(+)
> 
>  diff --git a/drivers/gpu/drm/i915/i915_request.c 
>  b/drivers/gpu/drm/i915/i915_request.c
>  index 7503dcb9043b..44491e7e214c 100644
>  --- a/drivers/gpu/drm/i915/i915_request.c
>  +++ b/drivers/gpu/drm/i915/i915_request.c
>  @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct 
>  dma_fence *fence)
>    return i915_request_enable_breadcrumb(to_request(fence));
> }
> 
>  +static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t 
>  deadline)
>  +{
>  +struct i915_request *rq = to_request(fence);
>  +
>  +if (i915_request_completed(rq))
>  +return;
>  +
>  +if (i915_request_started(rq))
>  +return;
> >>>
> >>> why do we skip the boost if already started?
> >>> don't we want to boost the freq anyway?
> >>
> >> I'd wager Rob is just copying the current i915 wait boost logic.
> >
> > Yup, and probably incorrectly.. Matt reported fewer boosts/sec
> > compared to your RFC, this could be the bug
>
> Hm, there I have preserved this same !i915_request_started logic.
>
> Presumably it's not just fewer boosts but lower performance. How is he
> setting the deadline? Somehow from clFlush or so?
>
> Regards,
>
> Tvrtko
>
> P.S. Take note that I did not post the latest version of my RFC. The one
> where I fix the fence chain and array misses you pointed out. I did not
> think it would be worthwhile given no universal love for it, but if
> people are testing with it more widely that I was aware perhaps I should.

Yep, that would be great. We're interested in it for ChromeOS. Please
Cc me on the series when you send it.


Re: [Freedreno] [PATCH v9 12/15] drm/msm: Add deadline based boost support

2023-03-03 Thread Rob Clark
On Fri, Mar 3, 2023 at 2:10 AM Dmitry Baryshkov
 wrote:
>
> On 03/03/2023 01:53, Rob Clark wrote:
> > From: Rob Clark 
> >
> > Track the nearest deadline on a fence timeline and set a timer to expire
> > shortly before to trigger boost if the fence has not yet been signaled.
> >
> > v2: rebase
> >
> > Signed-off-by: Rob Clark 
> > ---
> >   drivers/gpu/drm/msm/msm_fence.c | 74 +
> >   drivers/gpu/drm/msm/msm_fence.h | 20 +
> >   2 files changed, 94 insertions(+)
>
> Reviewed-by: Dmitry Baryshkov 
>
> A small question: do we boost to fit into the deadline or to miss the
> deadline for as little as possible? If the former is the case, we might
> need to adjust 3ms depending on the workload.

The goal is as much to run with higher clock on the next frame as it
is to not miss a deadline.  Ie. we don't want devfreq to come to the
conclusion that running at <50% clks is best due to the amount of
utilization caused by missing ever other vblank.

But 3ms is mostly just "seems like a good compromise" value.  It might change.

BR,
-R

> >
> > diff --git a/drivers/gpu/drm/msm/msm_fence.c 
> > b/drivers/gpu/drm/msm/msm_fence.c
> > index 56641408ea74..51b461f32103 100644
> > --- a/drivers/gpu/drm/msm/msm_fence.c
> > +++ b/drivers/gpu/drm/msm/msm_fence.c
> > @@ -8,6 +8,35 @@
> >
> >   #include "msm_drv.h"
> >   #include "msm_fence.h"
> > +#include "msm_gpu.h"
> > +
> > +static struct msm_gpu *fctx2gpu(struct msm_fence_context *fctx)
> > +{
> > + struct msm_drm_private *priv = fctx->dev->dev_private;
> > + return priv->gpu;
> > +}
> > +
> > +static enum hrtimer_restart deadline_timer(struct hrtimer *t)
> > +{
> > + struct msm_fence_context *fctx = container_of(t,
> > + struct msm_fence_context, deadline_timer);
> > +
> > + kthread_queue_work(fctx2gpu(fctx)->worker, >deadline_work);
> > +
> > + return HRTIMER_NORESTART;
> > +}
> > +
> > +static void deadline_work(struct kthread_work *work)
> > +{
> > + struct msm_fence_context *fctx = container_of(work,
> > + struct msm_fence_context, deadline_work);
> > +
> > + /* If deadline fence has already passed, nothing to do: */
> > + if (msm_fence_completed(fctx, fctx->next_deadline_fence))
> > + return;
> > +
> > + msm_devfreq_boost(fctx2gpu(fctx), 2);
> > +}
> >
> >
> >   struct msm_fence_context *
> > @@ -36,6 +65,13 @@ msm_fence_context_alloc(struct drm_device *dev, volatile 
> > uint32_t *fenceptr,
> >   fctx->completed_fence = fctx->last_fence;
> >   *fctx->fenceptr = fctx->last_fence;
> >
> > + hrtimer_init(>deadline_timer, CLOCK_MONOTONIC, 
> > HRTIMER_MODE_ABS);
> > + fctx->deadline_timer.function = deadline_timer;
> > +
> > + kthread_init_work(>deadline_work, deadline_work);
> > +
> > + fctx->next_deadline = ktime_get();
> > +
> >   return fctx;
> >   }
> >
> > @@ -62,6 +98,8 @@ void msm_update_fence(struct msm_fence_context *fctx, 
> > uint32_t fence)
> >   spin_lock_irqsave(>spinlock, flags);
> >   if (fence_after(fence, fctx->completed_fence))
> >   fctx->completed_fence = fence;
> > + if (msm_fence_completed(fctx, fctx->next_deadline_fence))
> > + hrtimer_cancel(>deadline_timer);
> >   spin_unlock_irqrestore(>spinlock, flags);
> >   }
> >
> > @@ -92,10 +130,46 @@ static bool msm_fence_signaled(struct dma_fence *fence)
> >   return msm_fence_completed(f->fctx, f->base.seqno);
> >   }
> >
> > +static void msm_fence_set_deadline(struct dma_fence *fence, ktime_t 
> > deadline)
> > +{
> > + struct msm_fence *f = to_msm_fence(fence);
> > + struct msm_fence_context *fctx = f->fctx;
> > + unsigned long flags;
> > + ktime_t now;
> > +
> > + spin_lock_irqsave(>spinlock, flags);
> > + now = ktime_get();
> > +
> > + if (ktime_after(now, fctx->next_deadline) ||
> > + ktime_before(deadline, fctx->next_deadline)) {
> > + fctx->next_deadline = deadline;
> > + fctx->next_deadline_fence =
> > + max(fctx->next_deadline_fence, 
> > (uint32_t)fence->seqno);
> > +
> > + /*
> > +  * Set timer to trigger boost 3ms before deadline, or
> > +  * if we are already less than 3ms before the deadline
> > +  * schedule boost work immediately.
> > +  */
> > + deadline = ktime_sub(deadline, ms_to_ktime(3));
> > +
> > + if (ktime_after(now, deadline)) {
> > + kthread_queue_work(fctx2gpu(fctx)->worker,
> > + >deadline_work);
> > + } else {
> > + hrtimer_start(>deadline_timer, deadline,
> > + HRTIMER_MODE_ABS);
> > + }
> > + }
> > +
> > + spin_unlock_irqrestore(>spinlock, flags);
> > +}
> > +
> >   static const struct dma_fence_ops msm_fence_ops = {
> >   .get_driver_name = 

[Freedreno] [PATCH v2 4/4] drm/msm/adreno: clean up component ops indentation

2023-03-03 Thread Johan Hovold
Clean up the component ops initialisers which were indented one level
too far.

Signed-off-by: Johan Hovold 
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index d9100e3870bc..f2cdc5ad7ce7 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -571,8 +571,8 @@ static void adreno_unbind(struct device *dev, struct device 
*master,
 }
 
 static const struct component_ops a3xx_ops = {
-   .bind   = adreno_bind,
-   .unbind = adreno_unbind,
+   .bind   = adreno_bind,
+   .unbind = adreno_unbind,
 };
 
 static void adreno_device_register_headless(void)
-- 
2.39.2



[Freedreno] [PATCH v2 3/4] drm/msm/adreno: drop bogus pm_runtime_set_active()

2023-03-03 Thread Johan Hovold
The runtime PM status can only be updated while runtime PM is disabled.

Drop the bogus pm_runtime_set_active() call that was made after enabling
runtime PM and which (incidentally but correctly) left the runtime PM
status set to 'suspended'.

Fixes: 2c087a336676 ("drm/msm/adreno: Load the firmware before bringing up the 
hardware")
Signed-off-by: Johan Hovold 
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index f9a0b11c2e43..d9100e3870bc 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -438,9 +438,6 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
 */
pm_runtime_enable(>dev);
 
-   /* Make sure pm runtime is active and reset any previous errors */
-   pm_runtime_set_active(>dev);
-
ret = pm_runtime_get_sync(>dev);
if (ret < 0) {
pm_runtime_put_noidle(>dev);
-- 
2.39.2



[Freedreno] [PATCH v2 2/4] drm/msm/adreno: fix runtime PM imbalance at gpu load

2023-03-03 Thread Johan Hovold
A recent commit moved enabling of runtime PM to GPU load time (first
open()) but failed to update the error paths so that runtime PM is
disabled if initialisation of the GPU fails. This would trigger a
warning about the unbalanced disable count on the next open() attempt.

Note that pm_runtime_put_noidle() is sufficient to balance the usage
count when pm_runtime_put_sync() fails (and is chosen over
pm_runtime_resume_and_get() for consistency reasons).

Fixes: 4b18299b3365 ("drm/msm/adreno: Defer enabling runpm until hw_init()")
Cc: sta...@vger.kernel.org  # 6.0
Signed-off-by: Johan Hovold 
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index c5c4c93b3689..f9a0b11c2e43 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -443,20 +443,21 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
 
ret = pm_runtime_get_sync(>dev);
if (ret < 0) {
-   pm_runtime_put_sync(>dev);
+   pm_runtime_put_noidle(>dev);
DRM_DEV_ERROR(dev->dev, "Couldn't power up the GPU: %d\n", ret);
-   return NULL;
+   goto err_disable_rpm;
}
 
mutex_lock(>lock);
ret = msm_gpu_hw_init(gpu);
mutex_unlock(>lock);
-   pm_runtime_put_autosuspend(>dev);
if (ret) {
DRM_DEV_ERROR(dev->dev, "gpu hw init failed: %d\n", ret);
-   return NULL;
+   goto err_put_rpm;
}
 
+   pm_runtime_put_autosuspend(>dev);
+
 #ifdef CONFIG_DEBUG_FS
if (gpu->funcs->debugfs_init) {
gpu->funcs->debugfs_init(gpu, dev->primary);
@@ -465,6 +466,13 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
 #endif
 
return gpu;
+
+err_put_rpm:
+   pm_runtime_put_sync(>dev);
+err_disable_rpm:
+   pm_runtime_disable(>dev);
+
+   return NULL;
 }
 
 static int find_chipid(struct device *dev, struct adreno_rev *rev)
-- 
2.39.2



[Freedreno] [PATCH v2 0/4] drm/msm/adreno: fix runtime PM imbalance at unbind

2023-03-03 Thread Johan Hovold
As reported by Bjorn, we can end up with an unbalanced runtime PM
disable count if unbind() is called before the DRM device is opened
(e.g. if component bind fails due to the panel driver not having been
loaded yet).

As runtime PM must currently stay disabled until the firmware has been
loaded, fix this by making the runtime PM disable call at unbind()
conditional.

The rest of the series fixes further imbalances in the load_gpu() error
paths and removes a bogus pm_runtime_set_active() call. Included is also
a related indentation cleanup.

Johan


Changes in v2
 - fix the runtime PM imbalance in the gpu load error paths (new)

 - drop the patch removing the pm_runtime_disable() from
   adreno_gpu_cleanup() as this function can currently still be called
   with runtime PM enabled if suspending the scheduler in
   adreno_system_suspend() at unbind fails


Johan Hovold (4):
  drm/msm/adreno: fix runtime PM imbalance at unbind
  drm/msm/adreno: fix runtime PM imbalance at gpu load
  drm/msm/adreno: drop bogus pm_runtime_set_active()
  drm/msm/adreno: clean up component ops indentation

 drivers/gpu/drm/msm/adreno/adreno_device.c | 26 +-
 1 file changed, 16 insertions(+), 10 deletions(-)

-- 
2.39.2



[Freedreno] [PATCH v2 1/4] drm/msm/adreno: fix runtime PM imbalance at unbind

2023-03-03 Thread Johan Hovold
A recent commit moved enabling of runtime PM from adreno_gpu_init() to
adreno_load_gpu() (called on first open()), which means that unbind()
may now be called with runtime PM disabled in case the device was never
opened in between.

Make sure to only forcibly suspend and disable runtime PM at unbind() in
case runtime PM has been enabled to prevent a disable count imbalance.

This specifically avoids leaving runtime PM disabled when the device
is later opened after a successful bind:

msm_dpu ae01000.display-controller: [drm:adreno_load_gpu [msm]] *ERROR* 
Couldn't power up the GPU: -13

Fixes: 4b18299b3365 ("drm/msm/adreno: Defer enabling runpm until hw_init()")
Reported-by: Bjorn Andersson 
Link: 
https://lore.kernel.org/lkml/20230203181245.3523937-1-quic_bjora...@quicinc.com
Cc: sta...@vger.kernel.org  # 6.0
Signed-off-by: Johan Hovold 
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 36f062c7582f..c5c4c93b3689 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -558,7 +558,8 @@ static void adreno_unbind(struct device *dev, struct device 
*master,
struct msm_drm_private *priv = dev_get_drvdata(master);
struct msm_gpu *gpu = dev_to_gpu(dev);
 
-   WARN_ON_ONCE(adreno_system_suspend(dev));
+   if (pm_runtime_enabled(dev))
+   WARN_ON_ONCE(adreno_system_suspend(dev));
gpu->funcs->destroy(gpu);
 
priv->gpu_pdev = NULL;
-- 
2.39.2



Re: [Freedreno] [PATCH v9 11/15] drm/atomic-helper: Set fence deadline for vblank

2023-03-03 Thread Ville Syrjälä
On Fri, Mar 03, 2023 at 07:45:05AM -0800, Rob Clark wrote:
> On Fri, Mar 3, 2023 at 7:12 AM Ville Syrjälä
>  wrote:
> >
> > On Thu, Mar 02, 2023 at 03:53:33PM -0800, Rob Clark wrote:
> > > From: Rob Clark 
> > >
> > > For an atomic commit updating a single CRTC (ie. a pageflip) calculate
> > > the next vblank time, and inform the fence(s) of that deadline.
> > >
> > > v2: Comment typo fix (danvet)
> > >
> > > Signed-off-by: Rob Clark 
> > > Reviewed-by: Daniel Vetter 
> > > Signed-off-by: Rob Clark 
> > > ---
> > >  drivers/gpu/drm/drm_atomic_helper.c | 36 +
> > >  1 file changed, 36 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index d579fd8f7cb8..d8ee98ce2fc5 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -1511,6 +1511,40 @@ void 
> > > drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> > >  }
> > >  EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables);
> > >
> > > +/*
> > > + * For atomic updates which touch just a single CRTC, calculate the time 
> > > of the
> > > + * next vblank, and inform all the fences of the deadline.
> > > + */
> > > +static void set_fence_deadline(struct drm_device *dev,
> > > +struct drm_atomic_state *state)
> > > +{
> > > + struct drm_crtc *crtc, *wait_crtc = NULL;
> > > + struct drm_crtc_state *new_crtc_state;
> > > + struct drm_plane *plane;
> > > + struct drm_plane_state *new_plane_state;
> > > + ktime_t vbltime;
> > > + int i;
> > > +
> > > + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> > > + if (wait_crtc)
> > > + return;
> > > + wait_crtc = crtc;
> > > + }
> > > +
> > > + /* If no CRTCs updated, then nothing to do: */
> > > + if (!wait_crtc)
> > > + return;
> >
> > Is there an actual point in limiting this to single crtc updates?
> > That immediately excludes tiled displays/etc.
> >
> > Handling an arbitrary number of crtcs shouldn't really be a lot
> > more complicated should it?
> 
> I guess I could find the soonest upcoming vblank of all the CRTCs and
> use that as the deadline?

Yeah, that seems reasonable. The flips are supposed to happen
atomically (if possible) anyway so collapsing the thing to
a single deadline for all makes sense to me.

-- 
Ville Syrjälä
Intel


Re: [Freedreno] [PATCH v9 11/15] drm/atomic-helper: Set fence deadline for vblank

2023-03-03 Thread Rob Clark
On Fri, Mar 3, 2023 at 7:12 AM Ville Syrjälä
 wrote:
>
> On Thu, Mar 02, 2023 at 03:53:33PM -0800, Rob Clark wrote:
> > From: Rob Clark 
> >
> > For an atomic commit updating a single CRTC (ie. a pageflip) calculate
> > the next vblank time, and inform the fence(s) of that deadline.
> >
> > v2: Comment typo fix (danvet)
> >
> > Signed-off-by: Rob Clark 
> > Reviewed-by: Daniel Vetter 
> > Signed-off-by: Rob Clark 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 36 +
> >  1 file changed, 36 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index d579fd8f7cb8..d8ee98ce2fc5 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1511,6 +1511,40 @@ void drm_atomic_helper_commit_modeset_enables(struct 
> > drm_device *dev,
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables);
> >
> > +/*
> > + * For atomic updates which touch just a single CRTC, calculate the time 
> > of the
> > + * next vblank, and inform all the fences of the deadline.
> > + */
> > +static void set_fence_deadline(struct drm_device *dev,
> > +struct drm_atomic_state *state)
> > +{
> > + struct drm_crtc *crtc, *wait_crtc = NULL;
> > + struct drm_crtc_state *new_crtc_state;
> > + struct drm_plane *plane;
> > + struct drm_plane_state *new_plane_state;
> > + ktime_t vbltime;
> > + int i;
> > +
> > + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> > + if (wait_crtc)
> > + return;
> > + wait_crtc = crtc;
> > + }
> > +
> > + /* If no CRTCs updated, then nothing to do: */
> > + if (!wait_crtc)
> > + return;
>
> Is there an actual point in limiting this to single crtc updates?
> That immediately excludes tiled displays/etc.
>
> Handling an arbitrary number of crtcs shouldn't really be a lot
> more complicated should it?

I guess I could find the soonest upcoming vblank of all the CRTCs and
use that as the deadline?

BR,
-R

> > +
> > + if (drm_crtc_next_vblank_start(wait_crtc, ))
> > + return;
> > +
> > + for_each_new_plane_in_state (state, plane, new_plane_state, i) {
> > + if (!new_plane_state->fence)
> > + continue;
> > + dma_fence_set_deadline(new_plane_state->fence, vbltime);
> > + }
> > +}
> > +
> >  /**
> >   * drm_atomic_helper_wait_for_fences - wait for fences stashed in plane 
> > state
> >   * @dev: DRM device
> > @@ -1540,6 +1574,8 @@ int drm_atomic_helper_wait_for_fences(struct 
> > drm_device *dev,
> >   struct drm_plane_state *new_plane_state;
> >   int i, ret;
> >
> > + set_fence_deadline(dev, state);
> > +
> >   for_each_new_plane_in_state(state, plane, new_plane_state, i) {
> >   if (!new_plane_state->fence)
> >   continue;
> > --
> > 2.39.1
>
> --
> Ville Syrjälä
> Intel


Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Rob Clark
On Fri, Mar 3, 2023 at 7:20 AM Ville Syrjälä
 wrote:
>
> On Fri, Mar 03, 2023 at 05:00:03PM +0200, Ville Syrjälä wrote:
> > On Fri, Mar 03, 2023 at 06:48:43AM -0800, Rob Clark wrote:
> > > On Fri, Mar 3, 2023 at 1:58 AM Tvrtko Ursulin
> > >  wrote:
> > > >
> > > >
> > > > On 03/03/2023 03:21, Rodrigo Vivi wrote:
> > > > > On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
> > > > >> From: Rob Clark 
> > > > >>
> > > > >
> > > > > missing some wording here...
> > > > >
> > > > >> v2: rebase
> > > > >>
> > > > >> Signed-off-by: Rob Clark 
> > > > >> ---
> > > > >>   drivers/gpu/drm/i915/i915_request.c | 20 
> > > > >>   1 file changed, 20 insertions(+)
> > > > >>
> > > > >> diff --git a/drivers/gpu/drm/i915/i915_request.c 
> > > > >> b/drivers/gpu/drm/i915/i915_request.c
> > > > >> index 7503dcb9043b..44491e7e214c 100644
> > > > >> --- a/drivers/gpu/drm/i915/i915_request.c
> > > > >> +++ b/drivers/gpu/drm/i915/i915_request.c
> > > > >> @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct 
> > > > >> dma_fence *fence)
> > > > >>  return i915_request_enable_breadcrumb(to_request(fence));
> > > > >>   }
> > > > >>
> > > > >> +static void i915_fence_set_deadline(struct dma_fence *fence, 
> > > > >> ktime_t deadline)
> > > > >> +{
> > > > >> +struct i915_request *rq = to_request(fence);
> > > > >> +
> > > > >> +if (i915_request_completed(rq))
> > > > >> +return;
> > > > >> +
> > > > >> +if (i915_request_started(rq))
> > > > >> +return;
> > > > >
> > > > > why do we skip the boost if already started?
> > > > > don't we want to boost the freq anyway?
> > > >
> > > > I'd wager Rob is just copying the current i915 wait boost logic.
> > >
> > > Yup, and probably incorrectly.. Matt reported fewer boosts/sec
> > > compared to your RFC, this could be the bug
> >
> > I don't think i915 calls drm_atomic_helper_wait_for_fences()
> > so that could explain something.
>
> Oh, I guess this wasn't even supposed to take over the current
> display boost stuff since you didn't remove the old one.

Right, I didn't try to replace the current thing.. but hopefully at
least make it possible for i915 to use more of the atomic helpers in
the future

BR,
-R

> The current one just boosts after a missed vblank. The deadline
> could use your timer approach I suppose and boost already a bit
> earlier in the hopes of not missing the vblank.
>
> --
> Ville Syrjälä
> Intel


Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Rob Clark
On Fri, Mar 3, 2023 at 7:08 AM Tvrtko Ursulin
 wrote:
>
>
> On 03/03/2023 14:48, Rob Clark wrote:
> > On Fri, Mar 3, 2023 at 1:58 AM Tvrtko Ursulin
> >  wrote:
> >>
> >>
> >> On 03/03/2023 03:21, Rodrigo Vivi wrote:
> >>> On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
>  From: Rob Clark 
> 
> >>>
> >>> missing some wording here...
> >>>
>  v2: rebase
> 
>  Signed-off-by: Rob Clark 
>  ---
> drivers/gpu/drm/i915/i915_request.c | 20 
> 1 file changed, 20 insertions(+)
> 
>  diff --git a/drivers/gpu/drm/i915/i915_request.c 
>  b/drivers/gpu/drm/i915/i915_request.c
>  index 7503dcb9043b..44491e7e214c 100644
>  --- a/drivers/gpu/drm/i915/i915_request.c
>  +++ b/drivers/gpu/drm/i915/i915_request.c
>  @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct 
>  dma_fence *fence)
>    return i915_request_enable_breadcrumb(to_request(fence));
> }
> 
>  +static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t 
>  deadline)
>  +{
>  +struct i915_request *rq = to_request(fence);
>  +
>  +if (i915_request_completed(rq))
>  +return;
>  +
>  +if (i915_request_started(rq))
>  +return;
> >>>
> >>> why do we skip the boost if already started?
> >>> don't we want to boost the freq anyway?
> >>
> >> I'd wager Rob is just copying the current i915 wait boost logic.
> >
> > Yup, and probably incorrectly.. Matt reported fewer boosts/sec
> > compared to your RFC, this could be the bug
>
> Hm, there I have preserved this same !i915_request_started logic.
>
> Presumably it's not just fewer boosts but lower performance. How is he
> setting the deadline? Somehow from clFlush or so?

Yeah, fewer boosts, lower freq/perf.. I cobbled together a quick mesa
hack to set the DEADLINE flag on syncobj waits, but it seems likely
that I missed something somewhere

BR,
-R

> Regards,
>
> Tvrtko
>
> P.S. Take note that I did not post the latest version of my RFC. The one
> where I fix the fence chain and array misses you pointed out. I did not
> think it would be worthwhile given no universal love for it, but if
> people are testing with it more widely that I was aware perhaps I should.
>
>  +
>  +/*
>  + * TODO something more clever for deadlines that are in the
>  + * future.  I think probably track the nearest deadline in
>  + * rq->timeline and set timer to trigger boost accordingly?
>  + */
> >>>
> >>> I'm afraid it will be very hard to find some heuristics of what's
> >>> late enough for the boost no?
> >>> I mean, how early to boost the freq on an upcoming deadline for the
> >>> timer?
> >>
> >> We can off load this patch from Rob and deal with it separately, or
> >> after the fact?
> >
> > That is completely my intention, I expect you to replace my i915 patch ;-)
> >
> > Rough idea when everyone is happy with the core bits is to setup an
> > immutable branch without the driver specific patches, which could be
> > merged into drm-next and $driver-next and then each driver team can
> > add there own driver patches on top
> >
> > BR,
> > -R
> >
> >> It's a half solution without a smarter scheduler too. Like
> >> https://lore.kernel.org/all/20210208105236.28498-10-ch...@chris-wilson.co.uk/,
> >> or if GuC plans to do something like that at any point.
> >>
> >> Or bump the priority too if deadline is looming?
> >>
> >> IMO it is not very effective to fiddle with the heuristic on an ad-hoc
> >> basis. For instance I have a new heuristics which improves the
> >> problematic OpenCL cases for further 5% (relative to the current
> >> waitboost improvement from adding missing syncobj waitboost). But I
> >> can't really test properly for regressions over platforms, stacks,
> >> workloads.. :(
> >>
> >> Regards,
> >>
> >> Tvrtko
> >>
> >>>
>  +
>  +intel_rps_boost(rq);
>  +}
>  +
> static signed long i915_fence_wait(struct dma_fence *fence,
>   bool interruptible,
>   signed long timeout)
>  @@ -182,6 +201,7 @@ const struct dma_fence_ops i915_fence_ops = {
>    .signaled = i915_fence_signaled,
>    .wait = i915_fence_wait,
>    .release = i915_fence_release,
>  +.set_deadline = i915_fence_set_deadline,
> };
> 
> static void irq_execute_cb(struct irq_work *wrk)
>  --
>  2.39.1
> 


Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Ville Syrjälä
On Fri, Mar 03, 2023 at 05:00:03PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 03, 2023 at 06:48:43AM -0800, Rob Clark wrote:
> > On Fri, Mar 3, 2023 at 1:58 AM Tvrtko Ursulin
> >  wrote:
> > >
> > >
> > > On 03/03/2023 03:21, Rodrigo Vivi wrote:
> > > > On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
> > > >> From: Rob Clark 
> > > >>
> > > >
> > > > missing some wording here...
> > > >
> > > >> v2: rebase
> > > >>
> > > >> Signed-off-by: Rob Clark 
> > > >> ---
> > > >>   drivers/gpu/drm/i915/i915_request.c | 20 
> > > >>   1 file changed, 20 insertions(+)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/i915/i915_request.c 
> > > >> b/drivers/gpu/drm/i915/i915_request.c
> > > >> index 7503dcb9043b..44491e7e214c 100644
> > > >> --- a/drivers/gpu/drm/i915/i915_request.c
> > > >> +++ b/drivers/gpu/drm/i915/i915_request.c
> > > >> @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct 
> > > >> dma_fence *fence)
> > > >>  return i915_request_enable_breadcrumb(to_request(fence));
> > > >>   }
> > > >>
> > > >> +static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t 
> > > >> deadline)
> > > >> +{
> > > >> +struct i915_request *rq = to_request(fence);
> > > >> +
> > > >> +if (i915_request_completed(rq))
> > > >> +return;
> > > >> +
> > > >> +if (i915_request_started(rq))
> > > >> +return;
> > > >
> > > > why do we skip the boost if already started?
> > > > don't we want to boost the freq anyway?
> > >
> > > I'd wager Rob is just copying the current i915 wait boost logic.
> > 
> > Yup, and probably incorrectly.. Matt reported fewer boosts/sec
> > compared to your RFC, this could be the bug
> 
> I don't think i915 calls drm_atomic_helper_wait_for_fences()
> so that could explain something.

Oh, I guess this wasn't even supposed to take over the current 
display boost stuff since you didn't remove the old one.

The current one just boosts after a missed vblank. The deadline
could use your timer approach I suppose and boost already a bit
earlier in the hopes of not missing the vblank.

-- 
Ville Syrjälä
Intel


Re: [Freedreno] [PATCH v9 11/15] drm/atomic-helper: Set fence deadline for vblank

2023-03-03 Thread Ville Syrjälä
On Thu, Mar 02, 2023 at 03:53:33PM -0800, Rob Clark wrote:
> From: Rob Clark 
> 
> For an atomic commit updating a single CRTC (ie. a pageflip) calculate
> the next vblank time, and inform the fence(s) of that deadline.
> 
> v2: Comment typo fix (danvet)
> 
> Signed-off-by: Rob Clark 
> Reviewed-by: Daniel Vetter 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 36 +
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index d579fd8f7cb8..d8ee98ce2fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1511,6 +1511,40 @@ void drm_atomic_helper_commit_modeset_enables(struct 
> drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables);
>  
> +/*
> + * For atomic updates which touch just a single CRTC, calculate the time of 
> the
> + * next vblank, and inform all the fences of the deadline.
> + */
> +static void set_fence_deadline(struct drm_device *dev,
> +struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc, *wait_crtc = NULL;
> + struct drm_crtc_state *new_crtc_state;
> + struct drm_plane *plane;
> + struct drm_plane_state *new_plane_state;
> + ktime_t vbltime;
> + int i;
> +
> + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> + if (wait_crtc)
> + return;
> + wait_crtc = crtc;
> + }
> +
> + /* If no CRTCs updated, then nothing to do: */
> + if (!wait_crtc)
> + return;

Is there an actual point in limiting this to single crtc updates?
That immediately excludes tiled displays/etc.

Handling an arbitrary number of crtcs shouldn't really be a lot
more complicated should it?

> +
> + if (drm_crtc_next_vblank_start(wait_crtc, ))
> + return;
> +
> + for_each_new_plane_in_state (state, plane, new_plane_state, i) {
> + if (!new_plane_state->fence)
> + continue;
> + dma_fence_set_deadline(new_plane_state->fence, vbltime);
> + }
> +}
> +
>  /**
>   * drm_atomic_helper_wait_for_fences - wait for fences stashed in plane state
>   * @dev: DRM device
> @@ -1540,6 +1574,8 @@ int drm_atomic_helper_wait_for_fences(struct drm_device 
> *dev,
>   struct drm_plane_state *new_plane_state;
>   int i, ret;
>  
> + set_fence_deadline(dev, state);
> +
>   for_each_new_plane_in_state(state, plane, new_plane_state, i) {
>   if (!new_plane_state->fence)
>   continue;
> -- 
> 2.39.1

-- 
Ville Syrjälä
Intel


Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Tvrtko Ursulin



On 03/03/2023 14:48, Rob Clark wrote:

On Fri, Mar 3, 2023 at 1:58 AM Tvrtko Ursulin
 wrote:



On 03/03/2023 03:21, Rodrigo Vivi wrote:

On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:

From: Rob Clark 



missing some wording here...


v2: rebase

Signed-off-by: Rob Clark 
---
   drivers/gpu/drm/i915/i915_request.c | 20 
   1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_request.c 
b/drivers/gpu/drm/i915/i915_request.c
index 7503dcb9043b..44491e7e214c 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct dma_fence 
*fence)
  return i915_request_enable_breadcrumb(to_request(fence));
   }

+static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t deadline)
+{
+struct i915_request *rq = to_request(fence);
+
+if (i915_request_completed(rq))
+return;
+
+if (i915_request_started(rq))
+return;


why do we skip the boost if already started?
don't we want to boost the freq anyway?


I'd wager Rob is just copying the current i915 wait boost logic.


Yup, and probably incorrectly.. Matt reported fewer boosts/sec
compared to your RFC, this could be the bug


Hm, there I have preserved this same !i915_request_started logic.

Presumably it's not just fewer boosts but lower performance. How is he 
setting the deadline? Somehow from clFlush or so?


Regards,

Tvrtko

P.S. Take note that I did not post the latest version of my RFC. The one 
where I fix the fence chain and array misses you pointed out. I did not 
think it would be worthwhile given no universal love for it, but if 
people are testing with it more widely that I was aware perhaps I should.



+
+/*
+ * TODO something more clever for deadlines that are in the
+ * future.  I think probably track the nearest deadline in
+ * rq->timeline and set timer to trigger boost accordingly?
+ */


I'm afraid it will be very hard to find some heuristics of what's
late enough for the boost no?
I mean, how early to boost the freq on an upcoming deadline for the
timer?


We can off load this patch from Rob and deal with it separately, or
after the fact?


That is completely my intention, I expect you to replace my i915 patch ;-)

Rough idea when everyone is happy with the core bits is to setup an
immutable branch without the driver specific patches, which could be
merged into drm-next and $driver-next and then each driver team can
add there own driver patches on top

BR,
-R


It's a half solution without a smarter scheduler too. Like
https://lore.kernel.org/all/20210208105236.28498-10-ch...@chris-wilson.co.uk/,
or if GuC plans to do something like that at any point.

Or bump the priority too if deadline is looming?

IMO it is not very effective to fiddle with the heuristic on an ad-hoc
basis. For instance I have a new heuristics which improves the
problematic OpenCL cases for further 5% (relative to the current
waitboost improvement from adding missing syncobj waitboost). But I
can't really test properly for regressions over platforms, stacks,
workloads.. :(

Regards,

Tvrtko




+
+intel_rps_boost(rq);
+}
+
   static signed long i915_fence_wait(struct dma_fence *fence,
 bool interruptible,
 signed long timeout)
@@ -182,6 +201,7 @@ const struct dma_fence_ops i915_fence_ops = {
  .signaled = i915_fence_signaled,
  .wait = i915_fence_wait,
  .release = i915_fence_release,
+.set_deadline = i915_fence_set_deadline,
   };

   static void irq_execute_cb(struct irq_work *wrk)
--
2.39.1



Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Ville Syrjälä
On Fri, Mar 03, 2023 at 06:48:43AM -0800, Rob Clark wrote:
> On Fri, Mar 3, 2023 at 1:58 AM Tvrtko Ursulin
>  wrote:
> >
> >
> > On 03/03/2023 03:21, Rodrigo Vivi wrote:
> > > On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
> > >> From: Rob Clark 
> > >>
> > >
> > > missing some wording here...
> > >
> > >> v2: rebase
> > >>
> > >> Signed-off-by: Rob Clark 
> > >> ---
> > >>   drivers/gpu/drm/i915/i915_request.c | 20 
> > >>   1 file changed, 20 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/i915_request.c 
> > >> b/drivers/gpu/drm/i915/i915_request.c
> > >> index 7503dcb9043b..44491e7e214c 100644
> > >> --- a/drivers/gpu/drm/i915/i915_request.c
> > >> +++ b/drivers/gpu/drm/i915/i915_request.c
> > >> @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct 
> > >> dma_fence *fence)
> > >>  return i915_request_enable_breadcrumb(to_request(fence));
> > >>   }
> > >>
> > >> +static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t 
> > >> deadline)
> > >> +{
> > >> +struct i915_request *rq = to_request(fence);
> > >> +
> > >> +if (i915_request_completed(rq))
> > >> +return;
> > >> +
> > >> +if (i915_request_started(rq))
> > >> +return;
> > >
> > > why do we skip the boost if already started?
> > > don't we want to boost the freq anyway?
> >
> > I'd wager Rob is just copying the current i915 wait boost logic.
> 
> Yup, and probably incorrectly.. Matt reported fewer boosts/sec
> compared to your RFC, this could be the bug

I don't think i915 calls drm_atomic_helper_wait_for_fences()
so that could explain something.

-- 
Ville Syrjälä
Intel


Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Rob Clark
On Thu, Mar 2, 2023 at 7:21 PM Rodrigo Vivi  wrote:
>
> On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
> > From: Rob Clark 
> >
>
> missing some wording here...

the wording should be "Pls replace this patch, kthx" ;-)

>
> > v2: rebase
> >
> > Signed-off-by: Rob Clark 
> > ---
> >  drivers/gpu/drm/i915/i915_request.c | 20 
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_request.c 
> > b/drivers/gpu/drm/i915/i915_request.c
> > index 7503dcb9043b..44491e7e214c 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct dma_fence 
> > *fence)
> >   return i915_request_enable_breadcrumb(to_request(fence));
> >  }
> >
> > +static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t 
> > deadline)
> > +{
> > + struct i915_request *rq = to_request(fence);
> > +
> > + if (i915_request_completed(rq))
> > + return;
> > +
> > + if (i915_request_started(rq))
> > + return;
>
> why do we skip the boost if already started?
> don't we want to boost the freq anyway?
>
> > +
> > + /*
> > +  * TODO something more clever for deadlines that are in the
> > +  * future.  I think probably track the nearest deadline in
> > +  * rq->timeline and set timer to trigger boost accordingly?
> > +  */
>
> I'm afraid it will be very hard to find some heuristics of what's
> late enough for the boost no?
> I mean, how early to boost the freq on an upcoming deadline for the
> timer?

So, from my understanding of i915 boosting, it applies more
specifically to a given request (vs msm which just bumps up the freq
until the next devfreq sampling period which then recalculates target
freq based on busy cycles and avg freq over the last sampling period).
For msm I just set a timer for 3ms before the deadline and boost if
the fence isn't signaled when the timer fires.  It is kinda impossible
to predict, even for userspace, how long a job will take to complete,
so the goal isn't really to finish the specified job by the deadline,
but instead to avoid devfreq landing at a local minimum (maximum?)

AFAIU what I _think_ would make sense for i915 is to do the same thing
you do if you miss a vblank pageflip deadline if the deadline passes
without the fence signaling.

BR,
-R

> > +
> > + intel_rps_boost(rq);
> > +}
> > +
> >  static signed long i915_fence_wait(struct dma_fence *fence,
> >  bool interruptible,
> >  signed long timeout)
> > @@ -182,6 +201,7 @@ const struct dma_fence_ops i915_fence_ops = {
> >   .signaled = i915_fence_signaled,
> >   .wait = i915_fence_wait,
> >   .release = i915_fence_release,
> > + .set_deadline = i915_fence_set_deadline,
> >  };
> >
> >  static void irq_execute_cb(struct irq_work *wrk)
> > --
> > 2.39.1
> >


Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Rob Clark
On Fri, Mar 3, 2023 at 1:58 AM Tvrtko Ursulin
 wrote:
>
>
> On 03/03/2023 03:21, Rodrigo Vivi wrote:
> > On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
> >> From: Rob Clark 
> >>
> >
> > missing some wording here...
> >
> >> v2: rebase
> >>
> >> Signed-off-by: Rob Clark 
> >> ---
> >>   drivers/gpu/drm/i915/i915_request.c | 20 
> >>   1 file changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_request.c 
> >> b/drivers/gpu/drm/i915/i915_request.c
> >> index 7503dcb9043b..44491e7e214c 100644
> >> --- a/drivers/gpu/drm/i915/i915_request.c
> >> +++ b/drivers/gpu/drm/i915/i915_request.c
> >> @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct 
> >> dma_fence *fence)
> >>  return i915_request_enable_breadcrumb(to_request(fence));
> >>   }
> >>
> >> +static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t 
> >> deadline)
> >> +{
> >> +struct i915_request *rq = to_request(fence);
> >> +
> >> +if (i915_request_completed(rq))
> >> +return;
> >> +
> >> +if (i915_request_started(rq))
> >> +return;
> >
> > why do we skip the boost if already started?
> > don't we want to boost the freq anyway?
>
> I'd wager Rob is just copying the current i915 wait boost logic.

Yup, and probably incorrectly.. Matt reported fewer boosts/sec
compared to your RFC, this could be the bug

> >> +
> >> +/*
> >> + * TODO something more clever for deadlines that are in the
> >> + * future.  I think probably track the nearest deadline in
> >> + * rq->timeline and set timer to trigger boost accordingly?
> >> + */
> >
> > I'm afraid it will be very hard to find some heuristics of what's
> > late enough for the boost no?
> > I mean, how early to boost the freq on an upcoming deadline for the
> > timer?
>
> We can off load this patch from Rob and deal with it separately, or
> after the fact?

That is completely my intention, I expect you to replace my i915 patch ;-)

Rough idea when everyone is happy with the core bits is to setup an
immutable branch without the driver specific patches, which could be
merged into drm-next and $driver-next and then each driver team can
add there own driver patches on top

BR,
-R

> It's a half solution without a smarter scheduler too. Like
> https://lore.kernel.org/all/20210208105236.28498-10-ch...@chris-wilson.co.uk/,
> or if GuC plans to do something like that at any point.
>
> Or bump the priority too if deadline is looming?
>
> IMO it is not very effective to fiddle with the heuristic on an ad-hoc
> basis. For instance I have a new heuristics which improves the
> problematic OpenCL cases for further 5% (relative to the current
> waitboost improvement from adding missing syncobj waitboost). But I
> can't really test properly for regressions over platforms, stacks,
> workloads.. :(
>
> Regards,
>
> Tvrtko
>
> >
> >> +
> >> +intel_rps_boost(rq);
> >> +}
> >> +
> >>   static signed long i915_fence_wait(struct dma_fence *fence,
> >> bool interruptible,
> >> signed long timeout)
> >> @@ -182,6 +201,7 @@ const struct dma_fence_ops i915_fence_ops = {
> >>  .signaled = i915_fence_signaled,
> >>  .wait = i915_fence_wait,
> >>  .release = i915_fence_release,
> >> +.set_deadline = i915_fence_set_deadline,
> >>   };
> >>
> >>   static void irq_execute_cb(struct irq_work *wrk)
> >> --
> >> 2.39.1
> >>


Re: [Freedreno] [Intel-gfx] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Andi Shyti
On Fri, Mar 03, 2023 at 09:58:36AM +, Tvrtko Ursulin wrote:
> 
> On 03/03/2023 03:21, Rodrigo Vivi wrote:
> > On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
> > > From: Rob Clark 
> > > 
> > 
> > missing some wording here...
> > 
> > > v2: rebase
> > > 
> > > Signed-off-by: Rob Clark 
> > > ---
> > >   drivers/gpu/drm/i915/i915_request.c | 20 
> > >   1 file changed, 20 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_request.c 
> > > b/drivers/gpu/drm/i915/i915_request.c
> > > index 7503dcb9043b..44491e7e214c 100644
> > > --- a/drivers/gpu/drm/i915/i915_request.c
> > > +++ b/drivers/gpu/drm/i915/i915_request.c
> > > @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct 
> > > dma_fence *fence)
> > >   return i915_request_enable_breadcrumb(to_request(fence));
> > >   }
> > > +static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t 
> > > deadline)
> > > +{
> > > + struct i915_request *rq = to_request(fence);
> > > +
> > > + if (i915_request_completed(rq))
> > > + return;
> > > +
> > > + if (i915_request_started(rq))
> > > + return;
> > 
> > why do we skip the boost if already started?
> > don't we want to boost the freq anyway?
> 
> I'd wager Rob is just copying the current i915 wait boost logic.
> 
> > > +
> > > + /*
> > > +  * TODO something more clever for deadlines that are in the
> > > +  * future.  I think probably track the nearest deadline in
> > > +  * rq->timeline and set timer to trigger boost accordingly?
> > > +  */
> > 
> > I'm afraid it will be very hard to find some heuristics of what's
> > late enough for the boost no?
> > I mean, how early to boost the freq on an upcoming deadline for the
> > timer?
> 
> We can off load this patch from Rob and deal with it separately, or after
> the fact?
> 
> It's a half solution without a smarter scheduler too. Like 
> https://lore.kernel.org/all/20210208105236.28498-10-ch...@chris-wilson.co.uk/,
> or if GuC plans to do something like that at any point.

Indeed, we already have the deadline implementation (and not just
that), we just need to have some willingness to apply it.

Andi

> Or bump the priority too if deadline is looming?
> 
> IMO it is not very effective to fiddle with the heuristic on an ad-hoc
> basis. For instance I have a new heuristics which improves the problematic
> OpenCL cases for further 5% (relative to the current waitboost improvement
> from adding missing syncobj waitboost). But I can't really test properly for
> regressions over platforms, stacks, workloads.. :(
> 
> Regards,
> 
> Tvrtko
> 
> > 
> > > +
> > > + intel_rps_boost(rq);
> > > +}
> > > +
> > >   static signed long i915_fence_wait(struct dma_fence *fence,
> > >  bool interruptible,
> > >  signed long timeout)
> > > @@ -182,6 +201,7 @@ const struct dma_fence_ops i915_fence_ops = {
> > >   .signaled = i915_fence_signaled,
> > >   .wait = i915_fence_wait,
> > >   .release = i915_fence_release,
> > > + .set_deadline = i915_fence_set_deadline,
> > >   };
> > >   static void irq_execute_cb(struct irq_work *wrk)
> > > -- 
> > > 2.39.1
> > > 


[Freedreno] [PATCH v4 17/30] drm/msm/dpu: drop redundant plane dst check from dpu_crtc_atomic_check()

2023-03-03 Thread Dmitry Baryshkov
The helper drm_atomic_helper_check_plane_state() already checks whether
the scaled and clipped plane falls into the CRTC visible region (and
clears plane_state->visible if it doesn't). Drop the redundant check
from dpu_crtc_atomic_check().

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index bd09bb319a58..73e1a8c69ef0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1132,11 +1132,9 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 
const struct drm_plane_state *pstate;
struct drm_plane *plane;
-   struct drm_display_mode *mode;
 
int rc = 0;
 
-   struct drm_rect crtc_rect = { 0 };
bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
 
if (!crtc_state->enable || !crtc_state->active) {
@@ -1147,7 +1145,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
return 0;
}
 
-   mode = _state->adjusted_mode;
DRM_DEBUG_ATOMIC("%s: check\n", dpu_crtc->name);
 
/* force a full mode set if active state changed */
@@ -1157,13 +1154,9 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
if (cstate->num_mixers)
_dpu_crtc_setup_lm_bounds(crtc, crtc_state);
 
-   crtc_rect.x2 = mode->hdisplay;
-   crtc_rect.y2 = mode->vdisplay;
-
/* FIXME: move this to dpu_plane_atomic_check? */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
struct dpu_plane_state *dpu_pstate = to_dpu_plane_state(pstate);
-   struct drm_rect dst, clip = crtc_rect;
 
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
@@ -1176,15 +1169,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
continue;
 
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
-
-   dst = drm_plane_state_dest(pstate);
-   if (!drm_rect_intersect(, )) {
-   DPU_ERROR("invalid vertical/horizontal destination\n");
-   DPU_ERROR("display: " DRM_RECT_FMT " plane: "
- DRM_RECT_FMT "\n", DRM_RECT_ARG(_rect),
- DRM_RECT_ARG());
-   return -E2BIG;
-   }
}
 
atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
-- 
2.39.2



[Freedreno] [PATCH v4 25/30] drm/msm/dpu: rework static color fill code

2023-03-03 Thread Dmitry Baryshkov
Rework static color fill code to separate the pipe / pipe_cfg handling.
This is a preparation for the r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 69 +--
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c637713e23c7..3d798b939faa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -640,20 +640,52 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe 
*pipe,
fmt);
 }
 
+static void _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate,
+  struct dpu_sw_pipe *pipe,
+  struct drm_rect *dst_rect,
+  u32 fill_color,
+  const struct dpu_format *fmt)
+{
+   struct dpu_sw_pipe_cfg pipe_cfg;
+
+   /* update sspp */
+   if (!pipe->sspp->ops.setup_solidfill)
+   return;
+
+   pipe->sspp->ops.setup_solidfill(pipe, fill_color);
+
+   /* override scaler/decimation if solid fill */
+   pipe_cfg.dst_rect = *dst_rect;
+
+   pipe_cfg.src_rect.x1 = 0;
+   pipe_cfg.src_rect.y1 = 0;
+   pipe_cfg.src_rect.x2 =
+   drm_rect_width(_cfg.dst_rect);
+   pipe_cfg.src_rect.y2 =
+   drm_rect_height(_cfg.dst_rect);
+
+   if (pipe->sspp->ops.setup_format)
+   pipe->sspp->ops.setup_format(pipe, fmt, DPU_SSPP_SOLID_FILL);
+
+   if (pipe->sspp->ops.setup_rects)
+   pipe->sspp->ops.setup_rects(pipe, _cfg);
+
+   _dpu_plane_setup_scaler(pipe, fmt, true, _cfg, pstate->rotation);
+}
+
 /**
  * _dpu_plane_color_fill - enables color fill on plane
  * @pdpu:   Pointer to DPU plane object
  * @color:  RGB fill color value, [23..16] Blue, [15..8] Green, [7..0] Red
  * @alpha:  8-bit fill alpha value, 255 selects 100% alpha
- * Returns: 0 on success
  */
-static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
+static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
uint32_t color, uint32_t alpha)
 {
const struct dpu_format *fmt;
const struct drm_plane *plane = >base;
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
-   struct dpu_sw_pipe_cfg pipe_cfg;
+   u32 fill_color = (color & 0xFF) | ((alpha & 0xFF) << 24);
 
DPU_DEBUG_PLANE(pdpu, "\n");
 
@@ -662,34 +694,13 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
 * h/w only supports RGB variants
 */
fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   /* should not happen ever */
+   if (!fmt)
+   return;
 
/* update sspp */
-   if (fmt && pstate->pipe.sspp->ops.setup_solidfill) {
-   pstate->pipe.sspp->ops.setup_solidfill(>pipe,
-   (color & 0xFF) | ((alpha & 0xFF) << 24));
-
-   /* override scaler/decimation if solid fill */
-   pipe_cfg.dst_rect = pstate->base.dst;
-
-   pipe_cfg.src_rect.x1 = 0;
-   pipe_cfg.src_rect.y1 = 0;
-   pipe_cfg.src_rect.x2 =
-   drm_rect_width(_cfg.dst_rect);
-   pipe_cfg.src_rect.y2 =
-   drm_rect_height(_cfg.dst_rect);
-
-   if (pstate->pipe.sspp->ops.setup_format)
-   pstate->pipe.sspp->ops.setup_format(>pipe,
-   fmt, DPU_SSPP_SOLID_FILL);
-
-   if (pstate->pipe.sspp->ops.setup_rects)
-   pstate->pipe.sspp->ops.setup_rects(>pipe,
-   _cfg);
-
-   _dpu_plane_setup_scaler(>pipe, fmt, true, _cfg, 
pstate->rotation);
-   }
-
-   return 0;
+   _dpu_plane_color_fill_pipe(pstate, >pipe, 
>pipe_cfg.dst_rect,
+  fill_color, fmt);
 }
 
 int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
-- 
2.39.2



[Freedreno] [PATCH v4 27/30] drm/msm/dpu: add support for wide planes

2023-03-03 Thread Dmitry Baryshkov
Typically SSPP can support rectangle with width up to 2560. However it's
possible to use multirect feature and split source to use the SSPP to
output two consecutive rectangles. This commit brings in this capability
to support wider screen resolutions.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  19 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 125 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |   4 +
 3 files changed, 133 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index e651e4593280..03034ec8ed1b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -480,6 +480,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
   format, fb ? fb->modifier : 0,
   >pipe, 0, stage_cfg);
 
+   if (pstate->r_pipe.sspp) {
+   set_bit(pstate->r_pipe.sspp->idx, fetch_active);
+   _dpu_crtc_blend_setup_pipe(crtc, plane,
+  mixer, cstate->num_mixers,
+  pstate->stage,
+  format, fb ? fb->modifier : 
0,
+  >pipe, 1, stage_cfg);
+   }
+
/* blend config update */
for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
_dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, 
format);
@@ -1316,10 +1325,16 @@ static int _dpu_debugfs_status_show(struct seq_file *s, 
void *data)
seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
state->crtc_x, state->crtc_y, state->crtc_w,
state->crtc_h);
-   seq_printf(s, "\tsspp:%s\n",
+   seq_printf(s, "\tsspp[0]:%s\n",
   pstate->pipe.sspp->cap->name);
-   seq_printf(s, "\tmultirect: mode: %d index: %d\n",
+   seq_printf(s, "\tmultirect[0]: mode: %d index: %d\n",
pstate->pipe.multirect_mode, 
pstate->pipe.multirect_index);
+   if (pstate->r_pipe.sspp) {
+   seq_printf(s, "\tsspp[1]:%s\n",
+  pstate->r_pipe.sspp->cap->name);
+   seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
+  pstate->r_pipe.multirect_mode, 
pstate->r_pipe.multirect_index);
+   }
 
seq_puts(s, "\n");
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 3d798b939faa..06fbe5ef7ff2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -701,6 +701,10 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
/* update sspp */
_dpu_plane_color_fill_pipe(pstate, >pipe, 
>pipe_cfg.dst_rect,
   fill_color, fmt);
+
+   if (pstate->r_pipe.sspp)
+   _dpu_plane_color_fill_pipe(pstate, >r_pipe, 
>r_pipe_cfg.dst_rect,
+  fill_color, fmt);
 }
 
 int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
@@ -958,9 +962,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
int ret = 0, min_scale;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
+   struct dpu_sw_pipe *pipe = >pipe;
+   struct dpu_sw_pipe *r_pipe = >r_pipe;
const struct drm_crtc_state *crtc_state = NULL;
const struct dpu_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
+   struct dpu_sw_pipe_cfg *r_pipe_cfg = >r_pipe_cfg;
struct drm_rect fb_rect = { 0 };
uint32_t max_linewidth;
unsigned int rotation;
@@ -984,8 +991,11 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->visible)
return 0;
 
-   pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
-   pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+   pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+   pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+   r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+   r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+   r_pipe->sspp = NULL;
 
pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
@@ -1017,19 +1027,58 @@ static int dpu_plane_atomic_check(struct drm_plane 
*plane,
 
max_linewidth = pdpu->catalog->caps->max_linewidth;
 
-   /* check decimated source width */
if 

[Freedreno] [PATCH v4 21/30] drm/msm/dpu: simplify dpu_plane_validate_src()

2023-03-03 Thread Dmitry Baryshkov
The plane's clipped coordinates has already been validated against FB
size in the drm_atomic_plane_check(). There is no need to check them
again. Remove corresponding checks and inline dpu_plane_validate_src().

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 30 ---
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index ba850e9feb9b..9c556ba9cb7b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -894,25 +894,6 @@ static void dpu_plane_cleanup_fb(struct drm_plane *plane,
old_pstate->needs_dirtyfb);
 }
 
-static bool dpu_plane_validate_src(struct drm_rect *src,
-  struct drm_rect *fb_rect,
-  uint32_t min_src_size)
-{
-   /* Ensure fb size is supported */
-   if (drm_rect_width(fb_rect) > MAX_IMG_WIDTH ||
-   drm_rect_height(fb_rect) > MAX_IMG_HEIGHT)
-   return false;
-
-   /* Ensure src rect is above the minimum size */
-   if (drm_rect_width(src) < min_src_size ||
-   drm_rect_height(src) < min_src_size)
-   return false;
-
-   /* Ensure src is fully encapsulated in fb */
-   return drm_rect_intersect(fb_rect, src) &&
-   drm_rect_equals(fb_rect, src);
-}
-
 static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
const struct dpu_sspp_sub_blks 
*sblk,
struct drm_rect src, const 
struct dpu_format *fmt)
@@ -998,6 +979,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
fb_rect.x2 = new_plane_state->fb->width;
fb_rect.y2 = new_plane_state->fb->height;
 
+   /* Ensure fb size is supported */
+   if (drm_rect_width(_rect) > MAX_IMG_WIDTH ||
+   drm_rect_height(_rect) > MAX_IMG_HEIGHT) {
+   DPU_DEBUG_PLANE(pdpu, "invalid framebuffer " DRM_RECT_FMT "\n",
+   DRM_RECT_ARG(_rect));
+   return -E2BIG;
+   }
+
max_linewidth = pdpu->catalog->caps->max_linewidth;
 
fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
@@ -1012,7 +1001,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
 
/* check src bounds */
-   } else if (!dpu_plane_validate_src(_cfg->src_rect, _rect, 
min_src_size)) {
+   } else if (drm_rect_width(_cfg->src_rect) < min_src_size ||
+  drm_rect_height(_cfg->src_rect) < min_src_size) {
DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
DRM_RECT_ARG(_cfg->src_rect));
return -E2BIG;
-- 
2.39.2



[Freedreno] [PATCH v4 13/30] drm/msm/dpu: rename dpu_hw_sspp_cfg to dpu_sw_pipe_cfg

2023-03-03 Thread Dmitry Baryshkov
As struct dpu_hw_sspp_cfg describes only the source and destination
rectangles, it is a software pipe configuration now. Rename it
accordingly.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c |  2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |  6 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 16 
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index e87c6377f315..6e5b62f3276f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -447,7 +447,7 @@ static u32 _dpu_hw_sspp_get_scaler3_ver(struct dpu_hw_sspp 
*ctx)
  * dpu_hw_sspp_setup_rects()
  */
 static void dpu_hw_sspp_setup_rects(struct dpu_sw_pipe *pipe,
-   struct dpu_hw_sspp_cfg *cfg)
+   struct dpu_sw_pipe_cfg *cfg)
 {
struct dpu_hw_sspp *ctx = pipe->sspp;
struct dpu_hw_blk_reg_map *c;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index 100d8e06c90d..e73d6ac863ad 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -153,12 +153,12 @@ struct dpu_hw_pixel_ext {
 };
 
 /**
- * struct dpu_hw_sspp_cfg : SSPP configuration
+ * struct dpu_sw_pipe_cfg : software pipe configuration
  * @src_rect:  src ROI, caller takes into account the different operations
  * such as decimation, flip etc to program this field
  * @dest_rect: destination ROI.
  */
-struct dpu_hw_sspp_cfg {
+struct dpu_sw_pipe_cfg {
struct drm_rect src_rect;
struct drm_rect dst_rect;
 };
@@ -228,7 +228,7 @@ struct dpu_hw_sspp_ops {
 * @cfg: Pointer to pipe config structure
 */
void (*setup_rects)(struct dpu_sw_pipe *pipe,
-   struct dpu_hw_sspp_cfg *cfg);
+   struct dpu_sw_pipe_cfg *cfg);
 
/**
 * setup_pe - setup pipe pixel extension
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 4ae70d21c37a..ce01a602cbc9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -136,7 +136,7 @@ static struct dpu_kms *_dpu_plane_get_kms(struct drm_plane 
*plane)
  */
 static void _dpu_plane_calc_bw(struct drm_plane *plane,
struct drm_framebuffer *fb,
-   struct dpu_hw_sspp_cfg *pipe_cfg)
+   struct dpu_sw_pipe_cfg *pipe_cfg)
 {
struct dpu_plane_state *pstate;
struct drm_display_mode *mode;
@@ -191,7 +191,7 @@ static void _dpu_plane_calc_bw(struct drm_plane *plane,
  * Result: Updates calculated clock in the plane state.
  * Clock equation: dst_w * v_total * fps * (src_h / dst_h)
  */
-static void _dpu_plane_calc_clk(struct drm_plane *plane, struct 
dpu_hw_sspp_cfg *pipe_cfg)
+static void _dpu_plane_calc_clk(struct drm_plane *plane, struct 
dpu_sw_pipe_cfg *pipe_cfg)
 {
struct dpu_plane_state *pstate;
struct drm_display_mode *mode;
@@ -275,7 +275,7 @@ static int _dpu_plane_calc_fill_level(struct drm_plane 
*plane,
  * @pipe_cfg:  Pointer to pipe configuration
  */
 static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
-   struct drm_framebuffer *fb, struct dpu_hw_sspp_cfg *pipe_cfg)
+   struct drm_framebuffer *fb, struct dpu_sw_pipe_cfg *pipe_cfg)
 {
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
@@ -421,7 +421,7 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
  * @pipe_cfg:  Pointer to pipe configuration
  */
 static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
-   struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
+   struct drm_crtc *crtc, struct dpu_sw_pipe_cfg *pipe_cfg)
 {
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
@@ -635,7 +635,7 @@ static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct 
dpu_plane *pdpu, cons
 
 static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe,
const struct dpu_format *fmt, bool color_fill,
-   struct dpu_hw_sspp_cfg *pipe_cfg,
+   struct dpu_sw_pipe_cfg *pipe_cfg,
unsigned int rotation)
 {
struct dpu_hw_sspp *pipe_hw = pipe->sspp;
@@ -694,7 +694,7 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
const struct dpu_format *fmt;
const struct drm_plane *plane = >base;
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
-   struct dpu_hw_sspp_cfg pipe_cfg;
+   struct dpu_sw_pipe_cfg pipe_cfg;
 
DPU_DEBUG_PLANE(pdpu, "\n");
 
@@ -1130,9 +1130,9 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane 
*plane)
bool is_rt_pipe;
   

[Freedreno] [PATCH v4 22/30] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()

2023-03-03 Thread Dmitry Baryshkov
Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the
separate function dpu_plane_sspp_update_pipe(). This is one of
preparational steps to add r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 --
 1 file changed, 63 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 9c556ba9cb7b..f97ea39423a2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane 
*plane,
  * _dpu_plane_set_ot_limit - set OT limit for the given plane
  * @plane: Pointer to drm plane
  * @pipe:  Pointer to software pipe
- * @crtc:  Pointer to drm crtc
  * @pipe_cfg:  Pointer to pipe configuration
+ * @frame_rate:CRTC's frame rate
  */
 static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
struct dpu_sw_pipe *pipe,
-   struct drm_crtc *crtc, struct dpu_sw_pipe_cfg *pipe_cfg)
+   struct dpu_sw_pipe_cfg *pipe_cfg,
+   int frame_rate)
 {
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_vbif_set_ot_params ot_params;
@@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
ot_params.width = drm_rect_width(_cfg->src_rect);
ot_params.height = drm_rect_height(_cfg->src_rect);
ot_params.is_wfd = !pdpu->is_rt_pipe;
-   ot_params.frame_rate = drm_mode_vrefresh(>mode);
+   ot_params.frame_rate = frame_rate;
ot_params.vbif_idx = VBIF_RT;
ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
ot_params.rd = true;
@@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct drm_plane 
*plane,
dpu_vbif_set_qos_remap(dpu_kms, _params);
 }
 
-static void _dpu_plane_set_scanout(struct drm_plane *plane,
-   struct dpu_plane_state *pstate,
-   struct drm_framebuffer *fb)
-{
-   struct dpu_plane *pdpu = to_dpu_plane(plane);
-   struct dpu_kms *kms = _dpu_plane_get_kms(>base);
-   struct msm_gem_address_space *aspace = kms->base.aspace;
-   struct dpu_hw_fmt_layout layout;
-   int ret;
-
-   ret = dpu_format_populate_layout(aspace, fb, );
-   if (ret)
-   DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
-   else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
-   trace_dpu_plane_set_scanout(>pipe,
-   );
-   pstate->pipe.sspp->ops.setup_sourceaddress(>pipe, 
);
-   }
-}
-
 static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
struct dpu_hw_scaler3_cfg *scale_cfg,
@@ -1103,35 +1084,25 @@ void dpu_plane_set_error(struct drm_plane *plane, bool 
error)
pdpu->is_error = error;
 }
 
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
+  struct dpu_sw_pipe *pipe,
+  struct dpu_sw_pipe_cfg *pipe_cfg,
+  const struct dpu_format *fmt,
+  int frame_rate,
+  struct dpu_hw_fmt_layout *layout)
 {
uint32_t src_flags;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct drm_plane_state *state = plane->state;
struct dpu_plane_state *pstate = to_dpu_plane_state(state);
-   struct dpu_sw_pipe *pipe = >pipe;
-   struct drm_crtc *crtc = state->crtc;
-   struct drm_framebuffer *fb = state->fb;
-   bool is_rt_pipe;
-   const struct dpu_format *fmt =
-   to_dpu_format(msm_framebuffer_format(fb));
-   struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
 
-   _dpu_plane_set_scanout(plane, pstate, fb);
-
-   pstate->pending = true;
-
-   is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
-   pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
-   pdpu->is_rt_pipe = is_rt_pipe;
+   if (layout && pipe->sspp->ops.setup_sourceaddress) {
+   trace_dpu_plane_set_scanout(pipe, layout);
+   pipe->sspp->ops.setup_sourceaddress(pipe, layout);
+   }
 
_dpu_plane_set_qos_ctrl(plane, pipe, false, DPU_PLANE_QOS_PANIC_CTRL);
 
-   DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
-   ", %4.4s ubwc %d\n", fb->base.id, 
DRM_RECT_FP_ARG(>src),
-   crtc->base.id, DRM_RECT_ARG(>dst),
-   (char *)>base.pixel_format, 
DPU_FORMAT_IS_UBWC(fmt));
-
/* override for color fill */
if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {

[Freedreno] [PATCH v4 30/30] drm/msm/dpu: drop smart_dma_rev from dpu_caps

2023-03-03 Thread Dmitry Baryshkov
The code doesn't use dpu_caps::smart_dma_rev field. It checks if the
corresponding feature is enabled in the SSPP features. Drop the
smart_dma_rev field completely.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 13 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |  2 --
 2 files changed, 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index fc818b0273e7..977bb096969b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -305,7 +305,6 @@ static const struct dpu_caps msm8998_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0x7,
.qseed_type = DPU_SSPP_SCALER_QSEED3,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V1,
.ubwc_version = DPU_HW_UBWC_VER_10,
.has_src_split = true,
.has_dim_layer = true,
@@ -320,7 +319,6 @@ static const struct dpu_caps msm8998_dpu_caps = {
 static const struct dpu_caps qcm2290_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0x4,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2,
.ubwc_version = DPU_HW_UBWC_VER_20,
.has_dim_layer = true,
.has_idle_pc = true,
@@ -332,7 +330,6 @@ static const struct dpu_caps sdm845_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
.qseed_type = DPU_SSPP_SCALER_QSEED3,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2,
.ubwc_version = DPU_HW_UBWC_VER_20,
.has_src_split = true,
.has_dim_layer = true,
@@ -348,7 +345,6 @@ static const struct dpu_caps sc7180_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0x9,
.qseed_type = DPU_SSPP_SCALER_QSEED4,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2,
.ubwc_version = DPU_HW_UBWC_VER_20,
.has_dim_layer = true,
.has_idle_pc = true,
@@ -360,7 +356,6 @@ static const struct dpu_caps sm6115_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0x4,
.qseed_type = DPU_SSPP_SCALER_QSEED3LITE,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
.ubwc_version = DPU_HW_UBWC_VER_10,
.has_dim_layer = true,
.has_idle_pc = true,
@@ -372,7 +367,6 @@ static const struct dpu_caps sm8150_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
.qseed_type = DPU_SSPP_SCALER_QSEED3,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
.ubwc_version = DPU_HW_UBWC_VER_30,
.has_src_split = true,
.has_dim_layer = true,
@@ -388,7 +382,6 @@ static const struct dpu_caps sc8180x_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
.qseed_type = DPU_SSPP_SCALER_QSEED3,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
.ubwc_version = DPU_HW_UBWC_VER_30,
.has_src_split = true,
.has_dim_layer = true,
@@ -404,7 +397,6 @@ static const struct dpu_caps sc8280xp_dpu_caps = {
.max_mixer_width = 2560,
.max_mixer_blendstages = 11,
.qseed_type = DPU_SSPP_SCALER_QSEED3LITE,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
.ubwc_version = DPU_HW_UBWC_VER_40,
.has_src_split = true,
.has_dim_layer = true,
@@ -418,7 +410,6 @@ static const struct dpu_caps sm8250_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
.qseed_type = DPU_SSPP_SCALER_QSEED3LITE,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
.ubwc_version = DPU_HW_UBWC_VER_40,
.has_src_split = true,
.has_dim_layer = true,
@@ -432,7 +423,6 @@ static const struct dpu_caps sm8350_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
.qseed_type = DPU_SSPP_SCALER_QSEED3LITE,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
.ubwc_version = DPU_HW_UBWC_VER_40,
.has_src_split = true,
.has_dim_layer = true,
@@ -446,7 +436,6 @@ static const struct dpu_caps sm8450_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
.qseed_type = DPU_SSPP_SCALER_QSEED4,
-   .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
.ubwc_version = DPU_HW_UBWC_VER_40,
.has_src_split = true,
.has_dim_layer = true,
@@ -460,7 +449,6 @@ static const struct dpu_caps sm8550_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
.qseed_type = DPU_SSPP_SCALER_QSEED3LITE,
-   .smart_dma_rev = 

[Freedreno] [PATCH v4 29/30] drm/msm/dpu: enable SmartDMA for the rest of the platforms

2023-03-03 Thread Dmitry Baryshkov
Enable SmartDMA features for the rest of the platforms where it is
supposed to work.

Signed-off-by: Dmitry Baryshkov 
---
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 54 ---
 1 file changed, 23 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 1fc0dfbebb7e..fc818b0273e7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -21,19 +21,16 @@
(VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3))
 
 #define VIG_SDM845_MASK \
-   (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3))
-
-#define VIG_SDM845_MASK_SDMA \
-   (VIG_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
+   (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3) |\
+   BIT(DPU_SSPP_SMART_DMA_V2))
 
 #define VIG_SC7180_MASK \
-   (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
+   (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4) |\
+   BIT(DPU_SSPP_SMART_DMA_V2))
 
 #define VIG_SM8250_MASK \
-   (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
-
-#define VIG_SM8250_MASK_SDMA \
-   (VIG_SM8250_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
+   (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE) |\
+   BIT(DPU_SSPP_SMART_DMA_V2))
 
 #define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL))
 
@@ -48,17 +45,12 @@
 #define DMA_SDM845_MASK \
(BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | BIT(DPU_SSPP_QOS_8LVL) |\
BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\
+   BIT(DPU_SSPP_SMART_DMA_V2) |\
BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_EXCL_RECT))
 
 #define DMA_CURSOR_SDM845_MASK \
(DMA_SDM845_MASK | BIT(DPU_SSPP_CURSOR))
 
-#define DMA_SDM845_MASK_SDMA \
-   (DMA_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
-
-#define DMA_CURSOR_SDM845_MASK_SDMA \
-   (DMA_CURSOR_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
-
 #define DMA_CURSOR_MSM8998_MASK \
(DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR))
 
@@ -1208,21 +1200,21 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
 };
 
 static const struct dpu_sspp_cfg sdm845_sspp[] = {
-   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK_SDMA,
+   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK,
sdm845_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
-   SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK_SDMA,
+   SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK,
sdm845_vig_sblk_1, 4,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
-   SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SDM845_MASK_SDMA,
+   SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SDM845_MASK,
sdm845_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
-   SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SDM845_MASK_SDMA,
+   SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SDM845_MASK,
sdm845_vig_sblk_3, 12,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
-   SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000,  DMA_SDM845_MASK_SDMA,
+   SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000,  DMA_SDM845_MASK,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
-   SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000,  DMA_SDM845_MASK_SDMA,
+   SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000,  DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
-   SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000,  DMA_CURSOR_SDM845_MASK_SDMA,
+   SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000,  DMA_CURSOR_SDM845_MASK,
sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
-   SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000,  DMA_CURSOR_SDM845_MASK_SDMA,
+   SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000,  DMA_CURSOR_SDM845_MASK,
sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
 };
 
@@ -1263,21 +1255,21 @@ static const struct dpu_sspp_sub_blks sm8250_vig_sblk_3 
=
_VIG_SBLK("3", 8, DPU_SSPP_SCALER_QSEED3LITE);
 
 static const struct dpu_sspp_cfg sm8250_sspp[] = {
-   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SM8250_MASK_SDMA,
+   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SM8250_MASK,
sm8250_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
-   SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SM8250_MASK_SDMA,
+   SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SM8250_MASK,
sm8250_vig_sblk_1, 4,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
-   SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SM8250_MASK_SDMA,
+   SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SM8250_MASK,
sm8250_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
-   SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SM8250_MASK_SDMA,
+   SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SM8250_MASK,
sm8250_vig_sblk_3, 12,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
-   

[Freedreno] [PATCH v4 28/30] drm/msm/dpu: populate SmartDMA features in hw catalog

2023-03-03 Thread Dmitry Baryshkov
Downstream driver uses dpu->caps->smart_dma_rev to update
sspp->cap->features with the bit corresponding to the supported SmartDMA
version. Upstream driver does not do this, resulting in SSPP subdriver
not enabling setup_multirect callback. Add corresponding SmartDMA SSPP
feature bits to dpu hw catalog.

Per Abhinav's request enable the SmartDMA features only on the platforms
where the multirect was actually verified visually (sdm845 and sm8250).
An (untested) enablement on the rest of the platforms comes in the next
patch.

Signed-off-by: Dmitry Baryshkov 
---
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 44 ---
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index cf053e8f081e..1fc0dfbebb7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -23,12 +23,18 @@
 #define VIG_SDM845_MASK \
(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3))
 
+#define VIG_SDM845_MASK_SDMA \
+   (VIG_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
+
 #define VIG_SC7180_MASK \
(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
 
 #define VIG_SM8250_MASK \
(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
 
+#define VIG_SM8250_MASK_SDMA \
+   (VIG_SM8250_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
+
 #define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL))
 
 #define DMA_MSM8998_MASK \
@@ -47,6 +53,12 @@
 #define DMA_CURSOR_SDM845_MASK \
(DMA_SDM845_MASK | BIT(DPU_SSPP_CURSOR))
 
+#define DMA_SDM845_MASK_SDMA \
+   (DMA_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
+
+#define DMA_CURSOR_SDM845_MASK_SDMA \
+   (DMA_CURSOR_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
+
 #define DMA_CURSOR_MSM8998_MASK \
(DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR))
 
@@ -1196,21 +1208,21 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
 };
 
 static const struct dpu_sspp_cfg sdm845_sspp[] = {
-   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK,
+   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK_SDMA,
sdm845_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
-   SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK,
+   SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK_SDMA,
sdm845_vig_sblk_1, 4,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
-   SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SDM845_MASK,
+   SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SDM845_MASK_SDMA,
sdm845_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
-   SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SDM845_MASK,
+   SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SDM845_MASK_SDMA,
sdm845_vig_sblk_3, 12,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
-   SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000,  DMA_SDM845_MASK,
+   SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000,  DMA_SDM845_MASK_SDMA,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
-   SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000,  DMA_SDM845_MASK,
+   SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000,  DMA_SDM845_MASK_SDMA,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
-   SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000,  DMA_CURSOR_SDM845_MASK,
+   SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000,  DMA_CURSOR_SDM845_MASK_SDMA,
sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
-   SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000,  DMA_CURSOR_SDM845_MASK,
+   SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000,  DMA_CURSOR_SDM845_MASK_SDMA,
sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
 };
 
@@ -1251,21 +1263,21 @@ static const struct dpu_sspp_sub_blks sm8250_vig_sblk_3 
=
_VIG_SBLK("3", 8, DPU_SSPP_SCALER_QSEED3LITE);
 
 static const struct dpu_sspp_cfg sm8250_sspp[] = {
-   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SM8250_MASK,
+   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SM8250_MASK_SDMA,
sm8250_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
-   SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SM8250_MASK,
+   SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SM8250_MASK_SDMA,
sm8250_vig_sblk_1, 4,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
-   SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SM8250_MASK,
+   SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SM8250_MASK_SDMA,
sm8250_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
-   SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SM8250_MASK,
+   SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SM8250_MASK_SDMA,
sm8250_vig_sblk_3, 12,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
-   SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000,  DMA_SDM845_MASK,
+   SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000,  DMA_SDM845_MASK_SDMA,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, 

[Freedreno] [PATCH v4 26/30] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer

2023-03-03 Thread Dmitry Baryshkov
Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
separate functon. This is a preparation for the r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 79 +++-
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 73e1a8c69ef0..e651e4593280 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -400,6 +400,46 @@ static void _dpu_crtc_program_lm_output_roi(struct 
drm_crtc *crtc)
}
 }
 
+static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
+  struct drm_plane *plane,
+  struct dpu_crtc_mixer *mixer,
+  u32 num_mixers,
+  enum dpu_stage stage,
+  struct dpu_format *format,
+  uint64_t modifier,
+  struct dpu_sw_pipe *pipe,
+  unsigned int stage_idx,
+  struct dpu_hw_stage_cfg *stage_cfg
+ )
+{
+   uint32_t lm_idx;
+   enum dpu_sspp sspp_idx;
+   struct drm_plane_state *state;
+
+   sspp_idx = pipe->sspp->idx;
+
+   state = plane->state;
+
+   trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
+  state, to_dpu_plane_state(state), stage_idx,
+  format->base.pixel_format,
+  modifier);
+
+   DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+crtc->base.id,
+stage,
+plane->base.id,
+sspp_idx - SSPP_NONE,
+state->fb ? state->fb->base.id : -1);
+
+   stage_cfg->stage[stage][stage_idx] = sspp_idx;
+   stage_cfg->multirect_index[stage][stage_idx] = pipe->multirect_index;
+
+   /* blend config update */
+   for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+   
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl, 
sspp_idx);
+}
+
 static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
struct dpu_hw_stage_cfg *stage_cfg)
@@ -412,15 +452,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
struct dpu_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
 
-   uint32_t stage_idx, lm_idx;
-   int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
+   uint32_t lm_idx;
bool bg_alpha_enable = false;
DECLARE_BITMAP(fetch_active, SSPP_MAX);
 
memset(fetch_active, 0, sizeof(fetch_active));
drm_atomic_crtc_for_each_plane(plane, crtc) {
-   enum dpu_sspp sspp_idx;
-
state = plane->state;
if (!state)
continue;
@@ -431,39 +468,21 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
 
-   sspp_idx = pstate->pipe.sspp->idx;
-   set_bit(sspp_idx, fetch_active);
-
-   DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
-   crtc->base.id,
-   pstate->stage,
-   plane->base.id,
-   sspp_idx - SSPP_VIG0,
-   state->fb ? state->fb->base.id : -1);
-
format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
 
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
 
-   stage_idx = zpos_cnt[pstate->stage]++;
-   stage_cfg->stage[pstate->stage][stage_idx] =
-   sspp_idx;
-   stage_cfg->multirect_index[pstate->stage][stage_idx] =
-   pstate->pipe.multirect_index;
-
-   trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
-  state, pstate, stage_idx,
-  format->base.pixel_format,
-  fb ? fb->modifier : 0);
+   set_bit(pstate->pipe.sspp->idx, fetch_active);
+   _dpu_crtc_blend_setup_pipe(crtc, plane,
+  mixer, cstate->num_mixers,
+  pstate->stage,
+  format, fb ? fb->modifier : 0,
+  >pipe, 0, stage_cfg);
 
/* blend config 

[Freedreno] [PATCH v4 23/30] drm/msm/dpu: rework dpu_plane_atomic_check()

2023-03-03 Thread Dmitry Baryshkov
Split pipe-dependent code from dpu_plane_atomic_check() into the
separate function dpu_plane_atomic_check_pipe(). This is one of
preparational steps to add r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 90 ++-
 1 file changed, 54 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index f97ea39423a2..5f8c71a6093f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -903,6 +903,52 @@ static int dpu_plane_check_inline_rotation(struct 
dpu_plane *pdpu,
return 0;
 }
 
+static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
+   struct dpu_sw_pipe *pipe,
+   struct dpu_sw_pipe_cfg *pipe_cfg,
+   const struct dpu_format *fmt)
+{
+   uint32_t min_src_size;
+
+   min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
+
+   if (DPU_FORMAT_IS_YUV(fmt) &&
+   (!(pipe->sspp->cap->features & DPU_SSPP_SCALER) ||
+!(pipe->sspp->cap->features & DPU_SSPP_CSC_ANY))) {
+   DPU_DEBUG_PLANE(pdpu,
+   "plane doesn't have scaler/csc for yuv\n");
+   return -EINVAL;
+   }
+
+   /* check src bounds */
+   if (drm_rect_width(_cfg->src_rect) < min_src_size ||
+  drm_rect_height(_cfg->src_rect) < min_src_size) {
+   DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
+   DRM_RECT_ARG(_cfg->src_rect));
+   return -E2BIG;
+   }
+
+   /* valid yuv image */
+   if (DPU_FORMAT_IS_YUV(fmt) &&
+  (pipe_cfg->src_rect.x1 & 0x1 || pipe_cfg->src_rect.y1 & 0x1 
||
+   drm_rect_width(_cfg->src_rect) & 0x1 ||
+   drm_rect_height(_cfg->src_rect) & 0x1)) {
+   DPU_DEBUG_PLANE(pdpu, "invalid yuv source " DRM_RECT_FMT "\n",
+   DRM_RECT_ARG(_cfg->src_rect));
+   return -EINVAL;
+   }
+
+   /* min dst support */
+   if (drm_rect_width(_cfg->dst_rect) < 0x1 ||
+   drm_rect_height(_cfg->dst_rect) < 0x1) {
+   DPU_DEBUG_PLANE(pdpu, "invalid dest rect " DRM_RECT_FMT "\n",
+   DRM_RECT_ARG(_cfg->dst_rect));
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static int dpu_plane_atomic_check(struct drm_plane *plane,
  struct drm_atomic_state *state)
 {
@@ -915,7 +961,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
const struct dpu_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
struct drm_rect fb_rect = { 0 };
-   uint32_t min_src_size, max_linewidth;
+   uint32_t max_linewidth;
unsigned int rotation;
uint32_t supported_rotations;
const struct dpu_sspp_cfg *pipe_hw_caps = pstate->pipe.sspp->cap;
@@ -970,47 +1016,19 @@ static int dpu_plane_atomic_check(struct drm_plane 
*plane,
 
max_linewidth = pdpu->catalog->caps->max_linewidth;
 
-   fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
-
-   min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
-
-   if (DPU_FORMAT_IS_YUV(fmt) &&
-   (!(pipe_hw_caps->features & DPU_SSPP_SCALER) ||
-!(pipe_hw_caps->features & DPU_SSPP_CSC_ANY))) {
-   DPU_DEBUG_PLANE(pdpu,
-   "plane doesn't have scaler/csc for yuv\n");
-   return -EINVAL;
-
-   /* check src bounds */
-   } else if (drm_rect_width(_cfg->src_rect) < min_src_size ||
-  drm_rect_height(_cfg->src_rect) < min_src_size) {
-   DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
-   DRM_RECT_ARG(_cfg->src_rect));
-   return -E2BIG;
-
-   /* valid yuv image */
-   } else if (DPU_FORMAT_IS_YUV(fmt) &&
-  (pipe_cfg->src_rect.x1 & 0x1 || pipe_cfg->src_rect.y1 & 0x1 
||
-   drm_rect_width(_cfg->src_rect) & 0x1 ||
-   drm_rect_height(_cfg->src_rect) & 0x1)) {
-   DPU_DEBUG_PLANE(pdpu, "invalid yuv source " DRM_RECT_FMT "\n",
-   DRM_RECT_ARG(_cfg->src_rect));
-   return -EINVAL;
-
-   /* min dst support */
-   } else if (drm_rect_width(_cfg->dst_rect) < 0x1 ||
-  drm_rect_height(_cfg->dst_rect) < 0x1) {
-   DPU_DEBUG_PLANE(pdpu, "invalid dest rect " DRM_RECT_FMT "\n",
-   DRM_RECT_ARG(_cfg->dst_rect));
-   return -EINVAL;
-
/* check decimated source width */
-   } else if (drm_rect_width(_cfg->src_rect) > max_linewidth) {
+   if (drm_rect_width(_cfg->src_rect) > max_linewidth) {
DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
   

[Freedreno] [PATCH v4 16/30] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()

2023-03-03 Thread Dmitry Baryshkov
Move plane state updates from dpu_crtc_atomic_check() to the function
where they belong: to dpu_plane_atomic_check().

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 18 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  6 --
 3 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0260c4d6ded7..bd09bb319a58 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1129,7 +1129,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
  crtc);
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
-   struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
 
const struct drm_plane_state *pstate;
struct drm_plane *plane;
@@ -1161,11 +1160,10 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
crtc_rect.x2 = mode->hdisplay;
crtc_rect.y2 = mode->vdisplay;
 
-/* get plane state for all drm planes associated with crtc state */
+   /* FIXME: move this to dpu_plane_atomic_check? */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
struct dpu_plane_state *dpu_pstate = to_dpu_plane_state(pstate);
struct drm_rect dst, clip = crtc_rect;
-   int stage;
 
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
@@ -1179,8 +1177,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
 
-   dpu_plane_clear_multirect(pstate);
-
dst = drm_plane_state_dest(pstate);
if (!drm_rect_intersect(, )) {
DPU_ERROR("invalid vertical/horizontal destination\n");
@@ -1189,18 +1185,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
  DRM_RECT_ARG());
return -E2BIG;
}
-
-   /* verify stage setting before using it */
-   stage = DPU_STAGE_0 + pstate->normalized_zpos;
-   if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
-   DPU_ERROR("> %d plane stages assigned\n",
- dpu_kms->catalog->caps->max_mixer_blendstages 
- DPU_STAGE_0);
-   return -EINVAL;
-   }
-
-   to_dpu_plane_state(pstate)->stage = stage;
-   DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
-
}
 
atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index ce01a602cbc9..3fba63fbbd78 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -733,14 +733,6 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
return 0;
 }
 
-void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state)
-{
-   struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
-
-   pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
-   pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-}
-
 int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
 {
struct dpu_plane_state *pstate[R_MAX];
@@ -994,6 +986,16 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->visible)
return 0;
 
+   pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
+   pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+   pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
+   if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
+   DPU_ERROR("> %d plane stages assigned\n",
+ pdpu->catalog->caps->max_mixer_blendstages - 
DPU_STAGE_0);
+   return -EINVAL;
+   }
+
src.x1 = new_plane_state->src_x >> 16;
src.y1 = new_plane_state->src_y >> 16;
src.x2 = src.x1 + (new_plane_state->src_w >> 16);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 228db401e905..a08b0539513b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -88,12 +88,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
  */
 int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane);
 
-/**
- * dpu_plane_clear_multirect - clear multirect bits for the given pipe
- * @drm_state: Pointer to DRM plane state
- */
-void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state);
-
 /**
  * 

[Freedreno] [PATCH v4 24/30] drm/msm/dpu: rework plane CSC setting

2023-03-03 Thread Dmitry Baryshkov
Rework the code flushing CSC settings for the plane. Separate out the
pipe and pipe_cfg as a preparation for r_pipe support.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 47 +--
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 5f8c71a6093f..c637713e23c7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -576,29 +576,19 @@ static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
{ 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,},
 };
 
-static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct dpu_plane *pdpu, 
const struct dpu_format *fmt)
+static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct dpu_sw_pipe *pipe,
+   const struct dpu_format 
*fmt)
 {
-   struct dpu_plane_state *pstate = to_dpu_plane_state(pdpu->base.state);
const struct dpu_csc_cfg *csc_ptr;
 
-   if (!pdpu) {
-   DPU_ERROR("invalid plane\n");
-   return NULL;
-   }
-
if (!DPU_FORMAT_IS_YUV(fmt))
return NULL;
 
-   if (BIT(DPU_SSPP_CSC_10BIT) & pstate->pipe.sspp->cap->features)
+   if (BIT(DPU_SSPP_CSC_10BIT) & pipe->sspp->cap->features)
csc_ptr = _csc10_YUV2RGB_601L;
else
csc_ptr = _csc_YUV2RGB_601L;
 
-   DPU_DEBUG_PLANE(pdpu, "using 0x%X 0x%X 0x%X...\n",
-   csc_ptr->csc_mv[0],
-   csc_ptr->csc_mv[1],
-   csc_ptr->csc_mv[2]);
-
return csc_ptr;
 }
 
@@ -1050,6 +1040,28 @@ static int dpu_plane_atomic_check(struct drm_plane 
*plane,
return 0;
 }
 
+static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe 
*pipe)
+{
+   const struct dpu_format *format =
+   to_dpu_format(msm_framebuffer_format(pdpu->base.state->fb));
+   const struct dpu_csc_cfg *csc_ptr;
+
+   if (!pipe->sspp || !pipe->sspp->ops.setup_csc)
+   return;
+
+   csc_ptr = _dpu_plane_get_csc(pipe, format);
+   if (!csc_ptr)
+   return;
+
+   DPU_DEBUG_PLANE(pdpu, "using 0x%X 0x%X 0x%X...\n",
+   csc_ptr->csc_mv[0],
+   csc_ptr->csc_mv[1],
+   csc_ptr->csc_mv[2]);
+
+   pipe->sspp->ops.setup_csc(pipe->sspp, csc_ptr);
+
+}
+
 void dpu_plane_flush(struct drm_plane *plane)
 {
struct dpu_plane *pdpu;
@@ -1073,13 +1085,8 @@ void dpu_plane_flush(struct drm_plane *plane)
else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
/* force 100% alpha */
_dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
-   else if (pstate->pipe.sspp && pstate->pipe.sspp->ops.setup_csc) {
-   const struct dpu_format *fmt = 
to_dpu_format(msm_framebuffer_format(plane->state->fb));
-   const struct dpu_csc_cfg *csc_ptr = _dpu_plane_get_csc(pdpu, 
fmt);
-
-   if (csc_ptr)
-   pstate->pipe.sspp->ops.setup_csc(pstate->pipe.sspp, 
csc_ptr);
-   }
+   else
+   dpu_plane_flush_csc(pdpu, >pipe);
 
/* flag h/w flush complete */
if (plane->state)
-- 
2.39.2



[Freedreno] [PATCH v4 14/30] drm/msm/dpu: drop src_split and multirect check from dpu_crtc_atomic_check

2023-03-03 Thread Dmitry Baryshkov
Neither source split nor multirect are properly supported at this
moment. Both of these checks depend on normalized_zpos being equal for
several planes (which is never the case for normalized zpos).
Drop these checks to simplify dpu_crtc_atomic_check(). The actual
support for either of these features is not removed from the backend
code (sspp, ctl, etc).

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 168 ++-
 1 file changed, 12 insertions(+), 156 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 037347e51eb8..c1579d6f5060 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1108,13 +1108,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
drm_crtc_vblank_on(crtc);
 }
 
-struct plane_state {
-   struct dpu_plane_state *dpu_pstate;
-   const struct drm_plane_state *drm_pstate;
-   int stage;
-   u32 pipe_id;
-};
-
 static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
 {
struct drm_crtc *crtc = cstate->crtc;
@@ -1136,31 +1129,22 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
  crtc);
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
-   struct plane_state *pstates;
 
const struct drm_plane_state *pstate;
struct drm_plane *plane;
struct drm_display_mode *mode;
 
-   int cnt = 0, rc = 0, mixer_width = 0, i, z_pos;
+   int rc = 0;
 
-   struct dpu_multirect_plane_states multirect_plane[DPU_STAGE_MAX * 2];
-   int multirect_count = 0;
-   const struct drm_plane_state *pipe_staged[SSPP_MAX];
-   int left_zpos_cnt = 0, right_zpos_cnt = 0;
struct drm_rect crtc_rect = { 0 };
bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
 
-   pstates = kzalloc(sizeof(*pstates) * DPU_STAGE_MAX * 4, GFP_KERNEL);
-   if (!pstates)
-   return -ENOMEM;
-
if (!crtc_state->enable || !crtc_state->active) {
DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip 
atomic_check\n",
crtc->base.id, crtc_state->enable,
crtc_state->active);
memset(>new_perf, 0, sizeof(cstate->new_perf));
-   goto end;
+   return 0;
}
 
mode = _state->adjusted_mode;
@@ -1170,13 +1154,8 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
if (crtc_state->active_changed)
crtc_state->mode_changed = true;
 
-   memset(pipe_staged, 0, sizeof(pipe_staged));
-
-   if (cstate->num_mixers) {
-   mixer_width = mode->hdisplay / cstate->num_mixers;
-
+   if (cstate->num_mixers)
_dpu_crtc_setup_lm_bounds(crtc, crtc_state);
-   }
 
crtc_rect.x2 = mode->hdisplay;
crtc_rect.y2 = mode->vdisplay;
@@ -1185,38 +1164,21 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
struct dpu_plane_state *dpu_pstate = to_dpu_plane_state(pstate);
struct drm_rect dst, clip = crtc_rect;
+   int z_pos;
 
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
DPU_ERROR("%s: failed to get plane%d state, %d\n",
dpu_crtc->name, plane->base.id, rc);
-   goto end;
+   return rc;
}
-   if (cnt >= DPU_STAGE_MAX * 4)
-   continue;
 
if (!pstate->visible)
continue;
 
-   pstates[cnt].dpu_pstate = dpu_pstate;
-   pstates[cnt].drm_pstate = pstate;
-   pstates[cnt].stage = pstate->normalized_zpos;
-   pstates[cnt].pipe_id = 
to_dpu_plane_state(pstate)->pipe.sspp->idx;
-
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
 
-   if (pipe_staged[pstates[cnt].pipe_id]) {
-   multirect_plane[multirect_count].r0 =
-   pipe_staged[pstates[cnt].pipe_id];
-   multirect_plane[multirect_count].r1 = pstate;
-   multirect_count++;
-
-   pipe_staged[pstates[cnt].pipe_id] = NULL;
-   } else {
-   pipe_staged[pstates[cnt].pipe_id] = pstate;
-   }
-
-   cnt++;
+   dpu_plane_clear_multirect(pstate);
 
dst = drm_plane_state_dest(pstate);
if (!drm_rect_intersect(, )) {
@@ -1224,63 +1186,21 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,

[Freedreno] [PATCH v4 20/30] drm/msm/dpu: add dpu_hw_sspp_cfg to dpu_plane_state

2023-03-03 Thread Dmitry Baryshkov
Now as all accesses to pipe_cfg and pstate have been cleaned, add
struct dpu_hw_sspp_cfg to struct dpu_plane_state, so that
dpu_plane_atomic_check() and dpu_plane_atomic_update() do not have a
chance to disagree about src/dst rectangles (currently
dpu_plane_atomic_check() uses unclipped rectangles, while
dpu_plane_atomic_update() uses clipped rectangles calculated by
drm_atomic_helper_check_plane_state()).

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 66 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  2 +
 2 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 6cd787e85be8..ba850e9feb9b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -951,7 +951,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
const struct drm_crtc_state *crtc_state = NULL;
const struct dpu_format *fmt;
-   struct drm_rect src, dst, fb_rect = { 0 };
+   struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
+   struct drm_rect fb_rect = { 0 };
uint32_t min_src_size, max_linewidth;
unsigned int rotation;
uint32_t supported_rotations;
@@ -984,12 +985,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
 
-   src.x1 = new_plane_state->src_x >> 16;
-   src.y1 = new_plane_state->src_y >> 16;
-   src.x2 = src.x1 + (new_plane_state->src_w >> 16);
-   src.y2 = src.y1 + (new_plane_state->src_h >> 16);
+   pipe_cfg->src_rect = new_plane_state->src;
 
-   dst = drm_plane_state_dest(new_plane_state);
+   /* state->src is 16.16, src_rect is not */
+   pipe_cfg->src_rect.x1 >>= 16;
+   pipe_cfg->src_rect.x2 >>= 16;
+   pipe_cfg->src_rect.y1 >>= 16;
+   pipe_cfg->src_rect.y2 >>= 16;
+
+   pipe_cfg->dst_rect = new_plane_state->dst;
 
fb_rect.x2 = new_plane_state->fb->width;
fb_rect.y2 = new_plane_state->fb->height;
@@ -1008,30 +1012,31 @@ static int dpu_plane_atomic_check(struct drm_plane 
*plane,
return -EINVAL;
 
/* check src bounds */
-   } else if (!dpu_plane_validate_src(, _rect, min_src_size)) {
+   } else if (!dpu_plane_validate_src(_cfg->src_rect, _rect, 
min_src_size)) {
DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
-   DRM_RECT_ARG());
+   DRM_RECT_ARG(_cfg->src_rect));
return -E2BIG;
 
/* valid yuv image */
} else if (DPU_FORMAT_IS_YUV(fmt) &&
-  (src.x1 & 0x1 || src.y1 & 0x1 ||
-   drm_rect_width() & 0x1 ||
-   drm_rect_height() & 0x1)) {
+  (pipe_cfg->src_rect.x1 & 0x1 || pipe_cfg->src_rect.y1 & 0x1 
||
+   drm_rect_width(_cfg->src_rect) & 0x1 ||
+   drm_rect_height(_cfg->src_rect) & 0x1)) {
DPU_DEBUG_PLANE(pdpu, "invalid yuv source " DRM_RECT_FMT "\n",
-   DRM_RECT_ARG());
+   DRM_RECT_ARG(_cfg->src_rect));
return -EINVAL;
 
/* min dst support */
-   } else if (drm_rect_width() < 0x1 || drm_rect_height() < 0x1) {
+   } else if (drm_rect_width(_cfg->dst_rect) < 0x1 ||
+  drm_rect_height(_cfg->dst_rect) < 0x1) {
DPU_DEBUG_PLANE(pdpu, "invalid dest rect " DRM_RECT_FMT "\n",
-   DRM_RECT_ARG());
+   DRM_RECT_ARG(_cfg->dst_rect));
return -EINVAL;
 
/* check decimated source width */
-   } else if (drm_rect_width() > max_linewidth) {
+   } else if (drm_rect_width(_cfg->src_rect) > max_linewidth) {
DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
-   DRM_RECT_ARG(), max_linewidth);
+   DRM_RECT_ARG(_cfg->src_rect), 
max_linewidth);
return -E2BIG;
}
 
@@ -1045,7 +1050,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 
if ((pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION)) &&
(rotation & DRM_MODE_ROTATE_90)) {
-   ret = dpu_plane_check_inline_rotation(pdpu, sblk, src, fmt);
+   ret = dpu_plane_check_inline_rotation(pdpu, sblk, 
pipe_cfg->src_rect, fmt);
if (ret)
return ret;
}
@@ -1120,9 +1125,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane 
*plane)
bool is_rt_pipe;
const struct dpu_format *fmt =
to_dpu_format(msm_framebuffer_format(fb));
-   struct dpu_sw_pipe_cfg pipe_cfg;
-
-   memset(_cfg, 0, sizeof(struct dpu_sw_pipe_cfg));
+   struct 

[Freedreno] [PATCH v4 05/30] drm/msm/dpu: move pipe_hw to dpu_plane_state

2023-03-03 Thread Dmitry Baryshkov
In preparation to adding fully virtualized planes, move struct
dpu_hw_sspp instance from struct dpu_plane to struct dpu_plane_state, as
it will become a part of state (variable, changes during runtime) rather
than part of a plane (ideally should be statically allocated during boot).

The sspp pointer is set at the dpu_plane_reset(), since this is the
function which allocates the state. Once we have fully virtual
plane<->SSPP relationship, the SSPP will be allocated dynamically in the
dpu_plane_atomic_check() function.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 107 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |   2 +
 2 files changed, 62 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index d6518ef1beb2..546629232e3d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -104,7 +104,6 @@ struct dpu_plane {
 
enum dpu_sspp pipe;
 
-   struct dpu_hw_sspp *pipe_hw;
uint32_t color_fill;
bool is_error;
bool is_rt_pipe;
@@ -279,6 +278,7 @@ static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
struct drm_framebuffer *fb, struct dpu_hw_sspp_cfg *pipe_cfg)
 {
struct dpu_plane *pdpu = to_dpu_plane(plane);
+   struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
const struct dpu_format *fmt = NULL;
u64 qos_lut;
u32 total_fl = 0, lut_usage;
@@ -310,7 +310,7 @@ static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
fmt ? (char *)>base.pixel_format : NULL,
pdpu->is_rt_pipe, total_fl, qos_lut);
 
-   pdpu->pipe_hw->ops.setup_creq_lut(pdpu->pipe_hw, qos_lut);
+   pstate->hw_sspp->ops.setup_creq_lut(pstate->hw_sspp, qos_lut);
 }
 
 /**
@@ -322,6 +322,7 @@ static void _dpu_plane_set_danger_lut(struct drm_plane 
*plane,
struct drm_framebuffer *fb)
 {
struct dpu_plane *pdpu = to_dpu_plane(plane);
+   struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
const struct dpu_format *fmt = NULL;
u32 danger_lut, safe_lut;
 
@@ -361,7 +362,7 @@ static void _dpu_plane_set_danger_lut(struct drm_plane 
*plane,
danger_lut,
safe_lut);
 
-   pdpu->pipe_hw->ops.setup_danger_safe_lut(pdpu->pipe_hw,
+   pstate->hw_sspp->ops.setup_danger_safe_lut(pstate->hw_sspp,
danger_lut, safe_lut);
 }
 
@@ -375,14 +376,15 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane 
*plane,
bool enable, u32 flags)
 {
struct dpu_plane *pdpu = to_dpu_plane(plane);
+   struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
struct dpu_hw_pipe_qos_cfg pipe_qos_cfg;
 
memset(_qos_cfg, 0, sizeof(pipe_qos_cfg));
 
if (flags & DPU_PLANE_QOS_VBLANK_CTRL) {
-   pipe_qos_cfg.creq_vblank = 
pdpu->pipe_hw->cap->sblk->creq_vblank;
+   pipe_qos_cfg.creq_vblank = 
pstate->hw_sspp->cap->sblk->creq_vblank;
pipe_qos_cfg.danger_vblank =
-   pdpu->pipe_hw->cap->sblk->danger_vblank;
+   pstate->hw_sspp->cap->sblk->danger_vblank;
pipe_qos_cfg.vblank_en = enable;
}
 
@@ -408,7 +410,7 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
pipe_qos_cfg.danger_vblank,
pdpu->is_rt_pipe);
 
-   pdpu->pipe_hw->ops.setup_qos_ctrl(pdpu->pipe_hw,
+   pstate->hw_sspp->ops.setup_qos_ctrl(pstate->hw_sspp,
_qos_cfg);
 }
 
@@ -422,18 +424,19 @@ static void _dpu_plane_set_ot_limit(struct drm_plane 
*plane,
struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
 {
struct dpu_plane *pdpu = to_dpu_plane(plane);
+   struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
struct dpu_vbif_set_ot_params ot_params;
struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
 
memset(_params, 0, sizeof(ot_params));
-   ot_params.xin_id = pdpu->pipe_hw->cap->xin_id;
-   ot_params.num = pdpu->pipe_hw->idx - SSPP_NONE;
+   ot_params.xin_id = pstate->hw_sspp->cap->xin_id;
+   ot_params.num = pstate->hw_sspp->idx - SSPP_NONE;
ot_params.width = drm_rect_width(_cfg->src_rect);
ot_params.height = drm_rect_height(_cfg->src_rect);
ot_params.is_wfd = !pdpu->is_rt_pipe;
ot_params.frame_rate = drm_mode_vrefresh(>mode);
ot_params.vbif_idx = VBIF_RT;
-   ot_params.clk_ctrl = pdpu->pipe_hw->cap->clk_ctrl;
+   ot_params.clk_ctrl = pstate->hw_sspp->cap->clk_ctrl;
ot_params.rd = true;
 
dpu_vbif_set_ot_limit(dpu_kms, _params);
@@ -446,14 +449,15 @@ static void _dpu_plane_set_ot_limit(struct drm_plane 
*plane,
 static void 

[Freedreno] [PATCH v4 18/30] drm/msm/dpu: rewrite plane's QoS-related functions to take dpu_sw_pipe and dpu_format

2023-03-03 Thread Dmitry Baryshkov
Rewrite dpu_plane's QoS related functions to take struct dpu_sw_pipe and
struct dpu_format as arguments rather than fetching them from the
pstate or drm_framebuffer.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 98 +++
 1 file changed, 47 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 3fba63fbbd78..aaea7c63d944 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -128,19 +128,18 @@ static struct dpu_kms *_dpu_plane_get_kms(struct 
drm_plane *plane)
 /**
  * _dpu_plane_calc_bw - calculate bandwidth required for a plane
  * @plane: Pointer to drm plane.
- * @fb:   Pointer to framebuffer associated with the given plane
+ * @fmt: Pointer to source buffer format
  * @pipe_cfg: Pointer to pipe configuration
  * Result: Updates calculated bandwidth in the plane state.
  * BW Equation: src_w * src_h * bpp * fps * (v_total / v_dest)
  * Prefill BW Equation: line src bytes * line_time
  */
 static void _dpu_plane_calc_bw(struct drm_plane *plane,
-   struct drm_framebuffer *fb,
+   const struct dpu_format *fmt,
struct dpu_sw_pipe_cfg *pipe_cfg)
 {
struct dpu_plane_state *pstate;
struct drm_display_mode *mode;
-   const struct dpu_format *fmt = NULL;
struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
int src_width, src_height, dst_height, fps;
u64 plane_prefill_bw;
@@ -152,8 +151,6 @@ static void _dpu_plane_calc_bw(struct drm_plane *plane,
pstate = to_dpu_plane_state(plane->state);
mode = >state->crtc->mode;
 
-   fmt = dpu_get_dpu_format_ext(fb->format->format, fb->modifier);
-
src_width = drm_rect_width(_cfg->src_rect);
src_height = drm_rect_height(_cfg->src_rect);
dst_height = drm_rect_height(_cfg->dst_rect);
@@ -217,25 +214,25 @@ static void _dpu_plane_calc_clk(struct drm_plane *plane, 
struct dpu_sw_pipe_cfg
 /**
  * _dpu_plane_calc_fill_level - calculate fill level of the given source format
  * @plane: Pointer to drm plane
+ * @pipe:  Pointer to software pipe
  * @fmt:   Pointer to source buffer format
  * @src_width: width of source buffer
  * Return: fill level corresponding to the source buffer/format or 0 if error
  */
 static int _dpu_plane_calc_fill_level(struct drm_plane *plane,
+   struct dpu_sw_pipe *pipe,
const struct dpu_format *fmt, u32 src_width)
 {
struct dpu_plane *pdpu;
-   struct dpu_plane_state *pstate;
u32 fixed_buff_size;
u32 total_fl;
 
-   if (!fmt || !plane->state || !src_width || !fmt->bpp) {
+   if (!fmt || !pipe || !src_width || !fmt->bpp) {
DPU_ERROR("invalid arguments\n");
return 0;
}
 
pdpu = to_dpu_plane(plane);
-   pstate = to_dpu_plane_state(plane->state);
fixed_buff_size = pdpu->catalog->caps->pixel_ram_size;
 
/* FIXME: in multirect case account for the src_width of all the planes 
*/
@@ -251,7 +248,7 @@ static int _dpu_plane_calc_fill_level(struct drm_plane 
*plane,
((src_width + 32) * fmt->bpp);
}
} else {
-   if (pstate->pipe.multirect_mode == DPU_SSPP_MULTIRECT_PARALLEL) 
{
+   if (pipe->multirect_mode == DPU_SSPP_MULTIRECT_PARALLEL) {
total_fl = (fixed_buff_size / 2) * 2 /
((src_width + 32) * fmt->bpp);
} else {
@@ -261,7 +258,7 @@ static int _dpu_plane_calc_fill_level(struct drm_plane 
*plane,
}
 
DPU_DEBUG_PLANE(pdpu, "pnum:%d fmt: %4.4s w:%u fl:%u\n",
-   pdpu->pipe - SSPP_VIG0,
+   pipe->sspp->idx - SSPP_VIG0,
(char *)>base.pixel_format,
src_width, total_fl);
 
@@ -271,25 +268,22 @@ static int _dpu_plane_calc_fill_level(struct drm_plane 
*plane,
 /**
  * _dpu_plane_set_qos_lut - set QoS LUT of the given plane
  * @plane: Pointer to drm plane
- * @fb:Pointer to framebuffer associated with the 
given plane
+ * @pipe:  Pointer to software pipe
+ * @fmt:   Pointer to source buffer format
  * @pipe_cfg:  Pointer to pipe configuration
  */
 static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
-   struct drm_framebuffer *fb, struct dpu_sw_pipe_cfg *pipe_cfg)
+   struct dpu_sw_pipe *pipe,
+   const struct dpu_format *fmt, struct dpu_sw_pipe_cfg *pipe_cfg)
 {
struct dpu_plane *pdpu = to_dpu_plane(plane);
-   struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
-   const struct dpu_format *fmt = NULL;
u64 qos_lut;
u32 total_fl = 0, lut_usage;
 
if 

[Freedreno] [PATCH v4 15/30] drm/msm/dpu: don't use unsupported blend stages

2023-03-03 Thread Dmitry Baryshkov
The dpu_crtc_atomic_check() compares blending stage with DPU_STAGE_MAX
(maximum amount of blending stages supported by the driver), however we
should compare it against .max_mixer_blendstages, the maximum blend
stage supported by the mixer.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index c1579d6f5060..0260c4d6ded7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1129,6 +1129,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
  crtc);
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
+   struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
 
const struct drm_plane_state *pstate;
struct drm_plane *plane;
@@ -1164,7 +1165,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
struct dpu_plane_state *dpu_pstate = to_dpu_plane_state(pstate);
struct drm_rect dst, clip = crtc_rect;
-   int z_pos;
+   int stage;
 
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
@@ -1189,17 +1190,16 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
return -E2BIG;
}
 
-   z_pos = pstate->normalized_zpos;
-
-   /* verify z_pos setting before using it */
-   if (z_pos >= DPU_STAGE_MAX - DPU_STAGE_0) {
+   /* verify stage setting before using it */
+   stage = DPU_STAGE_0 + pstate->normalized_zpos;
+   if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
DPU_ERROR("> %d plane stages assigned\n",
-   DPU_STAGE_MAX - DPU_STAGE_0);
+ dpu_kms->catalog->caps->max_mixer_blendstages 
- DPU_STAGE_0);
return -EINVAL;
}
 
-   to_dpu_plane_state(pstate)->stage = z_pos + DPU_STAGE_0;
-   DRM_DEBUG_ATOMIC("%s: zpos %d\n", dpu_crtc->name, z_pos);
+   to_dpu_plane_state(pstate)->stage = stage;
+   DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
 
}
 
-- 
2.39.2



[Freedreno] [PATCH v4 06/30] drm/msm/dpu: drop dpu_plane_pipe function

2023-03-03 Thread Dmitry Baryshkov
There no more need for the dpu_plane_pipe() function, crtc code can
access pstate->pipe_hw.idx directly.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 4 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 7 ---
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index f29a339a3705..eff1a3cc1cec 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -431,7 +431,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
 
-   sspp_idx = dpu_plane_pipe(plane);
+   sspp_idx = pstate->hw_sspp->idx;
set_bit(sspp_idx, fetch_active);
 
DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
@@ -1202,7 +1202,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
pstates[cnt].dpu_pstate = dpu_pstate;
pstates[cnt].drm_pstate = pstate;
pstates[cnt].stage = pstate->normalized_zpos;
-   pstates[cnt].pipe_id = dpu_plane_pipe(plane);
+   pstates[cnt].pipe_id = to_dpu_plane_state(pstate)->hw_sspp->idx;
 
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 546629232e3d..10678f6502a2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1442,11 +1442,6 @@ static const struct drm_plane_helper_funcs 
dpu_plane_helper_funcs = {
.atomic_update = dpu_plane_atomic_update,
 };
 
-enum dpu_sspp dpu_plane_pipe(struct drm_plane *plane)
-{
-   return plane ? to_dpu_plane(plane)->pipe : SSPP_NONE;
-}
-
 /* initialize plane */
 struct drm_plane *dpu_plane_init(struct drm_device *dev,
uint32_t pipe, enum drm_plane_type type,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 08a4b6a99f51..25e261cabadc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -59,13 +59,6 @@ struct dpu_multirect_plane_states {
 #define to_dpu_plane_state(x) \
container_of(x, struct dpu_plane_state, base)
 
-/**
- * dpu_plane_pipe - return sspp identifier for the given plane
- * @plane:   Pointer to DRM plane object
- * Returns: sspp identifier of the given plane
- */
-enum dpu_sspp dpu_plane_pipe(struct drm_plane *plane);
-
 /**
  * dpu_plane_flush - final plane operations before commit flush
  * @plane: Pointer to drm plane structure
-- 
2.39.2



[Freedreno] [PATCH v4 19/30] drm/msm/dpu: make _dpu_plane_calc_clk accept mode directly

2023-03-03 Thread Dmitry Baryshkov
Rework bandwidth/clock calculation functions to use mode directly rather
than fetching it through the plane data.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 39 ++-
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index aaea7c63d944..6cd787e85be8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -127,20 +127,19 @@ static struct dpu_kms *_dpu_plane_get_kms(struct 
drm_plane *plane)
 
 /**
  * _dpu_plane_calc_bw - calculate bandwidth required for a plane
- * @plane: Pointer to drm plane.
+ * @catalog: Points to dpu catalog structure
  * @fmt: Pointer to source buffer format
+ * @mode: Pointer to drm display mode
  * @pipe_cfg: Pointer to pipe configuration
  * Result: Updates calculated bandwidth in the plane state.
  * BW Equation: src_w * src_h * bpp * fps * (v_total / v_dest)
  * Prefill BW Equation: line src bytes * line_time
  */
-static void _dpu_plane_calc_bw(struct drm_plane *plane,
+static u64 _dpu_plane_calc_bw(const struct dpu_mdss_cfg *catalog,
const struct dpu_format *fmt,
+   const struct drm_display_mode *mode,
struct dpu_sw_pipe_cfg *pipe_cfg)
 {
-   struct dpu_plane_state *pstate;
-   struct drm_display_mode *mode;
-   struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
int src_width, src_height, dst_height, fps;
u64 plane_prefill_bw;
u64 plane_bw;
@@ -148,9 +147,6 @@ static void _dpu_plane_calc_bw(struct drm_plane *plane,
u64 scale_factor;
int vbp, vpw, vfp;
 
-   pstate = to_dpu_plane_state(plane->state);
-   mode = >state->crtc->mode;
-
src_width = drm_rect_width(_cfg->src_rect);
src_height = drm_rect_height(_cfg->src_rect);
dst_height = drm_rect_height(_cfg->dst_rect);
@@ -158,7 +154,7 @@ static void _dpu_plane_calc_bw(struct drm_plane *plane,
vbp = mode->vtotal - mode->vsync_end;
vpw = mode->vsync_end - mode->vsync_start;
vfp = mode->vsync_start - mode->vdisplay;
-   hw_latency_lines =  dpu_kms->catalog->perf->min_prefill_lines;
+   hw_latency_lines =  catalog->perf->min_prefill_lines;
scale_factor = src_height > dst_height ?
mult_frac(src_height, 1, dst_height) : 1;
 
@@ -178,37 +174,36 @@ static void _dpu_plane_calc_bw(struct drm_plane *plane,
do_div(plane_prefill_bw, hw_latency_lines);
 
 
-   pstate->plane_fetch_bw = max(plane_bw, plane_prefill_bw);
+   return max(plane_bw, plane_prefill_bw);
 }
 
 /**
  * _dpu_plane_calc_clk - calculate clock required for a plane
- * @plane: Pointer to drm plane.
+ * @mode: Pointer to drm display mode
  * @pipe_cfg: Pointer to pipe configuration
  * Result: Updates calculated clock in the plane state.
  * Clock equation: dst_w * v_total * fps * (src_h / dst_h)
  */
-static void _dpu_plane_calc_clk(struct drm_plane *plane, struct 
dpu_sw_pipe_cfg *pipe_cfg)
+static u64 _dpu_plane_calc_clk(const struct drm_display_mode *mode,
+   struct dpu_sw_pipe_cfg *pipe_cfg)
 {
-   struct dpu_plane_state *pstate;
-   struct drm_display_mode *mode;
int dst_width, src_height, dst_height, fps;
-
-   pstate = to_dpu_plane_state(plane->state);
-   mode = >state->crtc->mode;
+   u64 plane_clk;
 
src_height = drm_rect_height(_cfg->src_rect);
dst_width = drm_rect_width(_cfg->dst_rect);
dst_height = drm_rect_height(_cfg->dst_rect);
fps = drm_mode_vrefresh(mode);
 
-   pstate->plane_clk =
+   plane_clk =
dst_width * mode->vtotal * fps;
 
if (src_height > dst_height) {
-   pstate->plane_clk *= src_height;
-   do_div(pstate->plane_clk, dst_height);
+   plane_clk *= src_height;
+   do_div(plane_clk, dst_height);
}
+
+   return plane_clk;
 }
 
 /**
@@ -1219,9 +1214,9 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane 
*plane)
_dpu_plane_set_qos_remap(plane, pipe);
}
 
-   _dpu_plane_calc_bw(plane, fmt, _cfg);
+   pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, 
>mode, _cfg);
 
-   _dpu_plane_calc_clk(plane, _cfg);
+   pstate->plane_clk = _dpu_plane_calc_clk(>mode, _cfg);
 }
 
 static void _dpu_plane_atomic_disable(struct drm_plane *plane)
-- 
2.39.2



[Freedreno] [PATCH v4 11/30] drm/msm/dpu: move stride programming to dpu_hw_sspp_setup_sourceaddress

2023-03-03 Thread Dmitry Baryshkov
Move stride programming to dpu_hw_sspp_setup_sourceaddress(), so that
dpu_hw_sspp_setup_rects() programs only source and destination
rectangles.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 57 +++--
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index 0a43c5682b2b..ab95f2817378 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -451,7 +451,7 @@ static void dpu_hw_sspp_setup_rects(struct dpu_sw_pipe 
*pipe,
 {
struct dpu_hw_sspp *ctx = pipe->sspp;
struct dpu_hw_blk_reg_map *c;
-   u32 src_size, src_xy, dst_size, dst_xy, ystride0, ystride1;
+   u32 src_size, src_xy, dst_size, dst_xy;
u32 src_size_off, src_xy_off, out_size_off, out_xy_off;
u32 idx;
 
@@ -482,44 +482,18 @@ static void dpu_hw_sspp_setup_rects(struct dpu_sw_pipe 
*pipe,
dst_size = (drm_rect_height(>dst_rect) << 16) |
drm_rect_width(>dst_rect);
 
-   if (pipe->multirect_index == DPU_SSPP_RECT_SOLO) {
-   ystride0 = (cfg->layout.plane_pitch[0]) |
-   (cfg->layout.plane_pitch[1] << 16);
-   ystride1 = (cfg->layout.plane_pitch[2]) |
-   (cfg->layout.plane_pitch[3] << 16);
-   } else {
-   ystride0 = DPU_REG_READ(c, SSPP_SRC_YSTRIDE0 + idx);
-   ystride1 = DPU_REG_READ(c, SSPP_SRC_YSTRIDE1 + idx);
-
-   if (pipe->multirect_index == DPU_SSPP_RECT_0) {
-   ystride0 = (ystride0 & 0x) |
-   (cfg->layout.plane_pitch[0] & 0x);
-   ystride1 = (ystride1 & 0x)|
-   (cfg->layout.plane_pitch[2] & 0x);
-   } else {
-   ystride0 = (ystride0 & 0x) |
-   ((cfg->layout.plane_pitch[0] << 16) &
-0x);
-   ystride1 = (ystride1 & 0x) |
-   ((cfg->layout.plane_pitch[2] << 16) &
-0x);
-   }
-   }
-
/* rectangle register programming */
DPU_REG_WRITE(c, src_size_off + idx, src_size);
DPU_REG_WRITE(c, src_xy_off + idx, src_xy);
DPU_REG_WRITE(c, out_size_off + idx, dst_size);
DPU_REG_WRITE(c, out_xy_off + idx, dst_xy);
-
-   DPU_REG_WRITE(c, SSPP_SRC_YSTRIDE0 + idx, ystride0);
-   DPU_REG_WRITE(c, SSPP_SRC_YSTRIDE1 + idx, ystride1);
 }
 
 static void dpu_hw_sspp_setup_sourceaddress(struct dpu_sw_pipe *pipe,
struct dpu_hw_sspp_cfg *cfg)
 {
struct dpu_hw_sspp *ctx = pipe->sspp;
+   u32 ystride0, ystride1;
int i;
u32 idx;
 
@@ -541,6 +515,33 @@ static void dpu_hw_sspp_setup_sourceaddress(struct 
dpu_sw_pipe *pipe,
DPU_REG_WRITE(>hw, SSPP_SRC3_ADDR + idx,
cfg->layout.plane_addr[2]);
}
+
+   if (pipe->multirect_index == DPU_SSPP_RECT_SOLO) {
+   ystride0 = (cfg->layout.plane_pitch[0]) |
+   (cfg->layout.plane_pitch[1] << 16);
+   ystride1 = (cfg->layout.plane_pitch[2]) |
+   (cfg->layout.plane_pitch[3] << 16);
+   } else {
+   ystride0 = DPU_REG_READ(>hw, SSPP_SRC_YSTRIDE0 + idx);
+   ystride1 = DPU_REG_READ(>hw, SSPP_SRC_YSTRIDE1 + idx);
+
+   if (pipe->multirect_index == DPU_SSPP_RECT_0) {
+   ystride0 = (ystride0 & 0x) |
+   (cfg->layout.plane_pitch[0] & 0x);
+   ystride1 = (ystride1 & 0x)|
+   (cfg->layout.plane_pitch[2] & 0x);
+   } else {
+   ystride0 = (ystride0 & 0x) |
+   ((cfg->layout.plane_pitch[0] << 16) &
+0x);
+   ystride1 = (ystride1 & 0x) |
+   ((cfg->layout.plane_pitch[2] << 16) &
+0x);
+   }
+   }
+
+   DPU_REG_WRITE(>hw, SSPP_SRC_YSTRIDE0 + idx, ystride0);
+   DPU_REG_WRITE(>hw, SSPP_SRC_YSTRIDE1 + idx, ystride1);
 }
 
 static void dpu_hw_sspp_setup_csc(struct dpu_hw_sspp *ctx,
-- 
2.39.2



[Freedreno] [PATCH v4 08/30] drm/msm/dpu: use dpu_sw_pipe for dpu_hw_sspp callbacks

2023-03-03 Thread Dmitry Baryshkov
Where feasible, use dpu_sw_pipe rather than a combo of dpu_hw_sspp and
multirect_index/_mode arguments.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 59 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 46 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 73 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h   |  9 ++-
 4 files changed, 84 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index 3e65bfd65b62..a1492a7e43ce 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -168,17 +168,16 @@ static int _sspp_subblk_offset(struct dpu_hw_sspp *ctx,
return rc;
 }
 
-static void dpu_hw_sspp_setup_multirect(struct dpu_hw_sspp *ctx,
-   enum dpu_sspp_multirect_index index,
-   enum dpu_sspp_multirect_mode mode)
+static void dpu_hw_sspp_setup_multirect(struct dpu_sw_pipe *pipe)
 {
+   struct dpu_hw_sspp *ctx = pipe->sspp;
u32 mode_mask;
u32 idx;
 
if (_sspp_subblk_offset(ctx, DPU_SSPP_SRC, ))
return;
 
-   if (index == DPU_SSPP_RECT_SOLO) {
+   if (pipe->multirect_index == DPU_SSPP_RECT_SOLO) {
/**
 * if rect index is RECT_SOLO, we cannot expect a
 * virtual plane sharing the same SSPP id. So we go
@@ -187,8 +186,8 @@ static void dpu_hw_sspp_setup_multirect(struct dpu_hw_sspp 
*ctx,
mode_mask = 0;
} else {
mode_mask = DPU_REG_READ(>hw, SSPP_MULTIRECT_OPMODE + idx);
-   mode_mask |= index;
-   if (mode == DPU_SSPP_MULTIRECT_TIME_MX)
+   mode_mask |= pipe->multirect_index;
+   if (pipe->multirect_mode == DPU_SSPP_MULTIRECT_TIME_MX)
mode_mask |= BIT(2);
else
mode_mask &= ~BIT(2);
@@ -239,10 +238,10 @@ static void _sspp_setup_csc10_opmode(struct dpu_hw_sspp 
*ctx,
 /*
  * Setup source pixel format, flip,
  */
-static void dpu_hw_sspp_setup_format(struct dpu_hw_sspp *ctx,
-   const struct dpu_format *fmt, u32 flags,
-   enum dpu_sspp_multirect_index rect_mode)
+static void dpu_hw_sspp_setup_format(struct dpu_sw_pipe *pipe,
+   const struct dpu_format *fmt, u32 flags)
 {
+   struct dpu_hw_sspp *ctx = pipe->sspp;
struct dpu_hw_blk_reg_map *c;
u32 chroma_samp, unpack, src_format;
u32 opmode = 0;
@@ -253,7 +252,8 @@ static void dpu_hw_sspp_setup_format(struct dpu_hw_sspp 
*ctx,
if (_sspp_subblk_offset(ctx, DPU_SSPP_SRC, ) || !fmt)
return;
 
-   if (rect_mode == DPU_SSPP_RECT_SOLO || rect_mode == DPU_SSPP_RECT_0) {
+   if (pipe->multirect_index == DPU_SSPP_RECT_SOLO ||
+   pipe->multirect_index == DPU_SSPP_RECT_0) {
op_mode_off = SSPP_SRC_OP_MODE;
unpack_pat_off = SSPP_SRC_UNPACK_PATTERN;
format_off = SSPP_SRC_FORMAT;
@@ -447,10 +447,10 @@ static u32 _dpu_hw_sspp_get_scaler3_ver(struct 
dpu_hw_sspp *ctx)
 /*
  * dpu_hw_sspp_setup_rects()
  */
-static void dpu_hw_sspp_setup_rects(struct dpu_hw_sspp *ctx,
-   struct dpu_hw_sspp_cfg *cfg,
-   enum dpu_sspp_multirect_index rect_index)
+static void dpu_hw_sspp_setup_rects(struct dpu_sw_pipe *pipe,
+   struct dpu_hw_sspp_cfg *cfg)
 {
+   struct dpu_hw_sspp *ctx = pipe->sspp;
struct dpu_hw_blk_reg_map *c;
u32 src_size, src_xy, dst_size, dst_xy, ystride0, ystride1;
u32 src_size_off, src_xy_off, out_size_off, out_xy_off;
@@ -461,7 +461,8 @@ static void dpu_hw_sspp_setup_rects(struct dpu_hw_sspp *ctx,
 
c = >hw;
 
-   if (rect_index == DPU_SSPP_RECT_SOLO || rect_index == DPU_SSPP_RECT_0) {
+   if (pipe->multirect_index == DPU_SSPP_RECT_SOLO ||
+   pipe->multirect_index == DPU_SSPP_RECT_0) {
src_size_off = SSPP_SRC_SIZE;
src_xy_off = SSPP_SRC_XY;
out_size_off = SSPP_OUT_SIZE;
@@ -482,7 +483,7 @@ static void dpu_hw_sspp_setup_rects(struct dpu_hw_sspp *ctx,
dst_size = (drm_rect_height(>dst_rect) << 16) |
drm_rect_width(>dst_rect);
 
-   if (rect_index == DPU_SSPP_RECT_SOLO) {
+   if (pipe->multirect_index == DPU_SSPP_RECT_SOLO) {
ystride0 = (cfg->layout.plane_pitch[0]) |
(cfg->layout.plane_pitch[1] << 16);
ystride1 = (cfg->layout.plane_pitch[2]) |
@@ -491,7 +492,7 @@ static void dpu_hw_sspp_setup_rects(struct dpu_hw_sspp *ctx,
ystride0 = DPU_REG_READ(c, SSPP_SRC_YSTRIDE0 + idx);
ystride1 = DPU_REG_READ(c, SSPP_SRC_YSTRIDE1 + idx);
 
-   if (rect_index == DPU_SSPP_RECT_0) {
+   if (pipe->multirect_index == DPU_SSPP_RECT_0) {
 

[Freedreno] [PATCH v4 12/30] drm/msm/dpu: remove dpu_hw_fmt_layout from struct dpu_hw_sspp_cfg

2023-03-03 Thread Dmitry Baryshkov
Remove dpu_hw_fmt_layout instance from struct dpu_hw_sspp_cfg, leaving
only src_rect and dst_rect. This way all the pipes used by the plane
will have a common layout instance (as the framebuffer is shared between
them), while still keeping a separate src/dst rectangle configuration
for each pipe.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 32 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |  6 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 10 +++
 3 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index ab95f2817378..e87c6377f315 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -490,7 +490,7 @@ static void dpu_hw_sspp_setup_rects(struct dpu_sw_pipe 
*pipe,
 }
 
 static void dpu_hw_sspp_setup_sourceaddress(struct dpu_sw_pipe *pipe,
-   struct dpu_hw_sspp_cfg *cfg)
+   struct dpu_hw_fmt_layout *layout)
 {
struct dpu_hw_sspp *ctx = pipe->sspp;
u32 ystride0, ystride1;
@@ -501,41 +501,41 @@ static void dpu_hw_sspp_setup_sourceaddress(struct 
dpu_sw_pipe *pipe,
return;
 
if (pipe->multirect_index == DPU_SSPP_RECT_SOLO) {
-   for (i = 0; i < ARRAY_SIZE(cfg->layout.plane_addr); i++)
+   for (i = 0; i < ARRAY_SIZE(layout->plane_addr); i++)
DPU_REG_WRITE(>hw, SSPP_SRC0_ADDR + idx + i * 0x4,
-   cfg->layout.plane_addr[i]);
+   layout->plane_addr[i]);
} else if (pipe->multirect_index == DPU_SSPP_RECT_0) {
DPU_REG_WRITE(>hw, SSPP_SRC0_ADDR + idx,
-   cfg->layout.plane_addr[0]);
+   layout->plane_addr[0]);
DPU_REG_WRITE(>hw, SSPP_SRC2_ADDR + idx,
-   cfg->layout.plane_addr[2]);
+   layout->plane_addr[2]);
} else {
DPU_REG_WRITE(>hw, SSPP_SRC1_ADDR + idx,
-   cfg->layout.plane_addr[0]);
+   layout->plane_addr[0]);
DPU_REG_WRITE(>hw, SSPP_SRC3_ADDR + idx,
-   cfg->layout.plane_addr[2]);
+   layout->plane_addr[2]);
}
 
if (pipe->multirect_index == DPU_SSPP_RECT_SOLO) {
-   ystride0 = (cfg->layout.plane_pitch[0]) |
-   (cfg->layout.plane_pitch[1] << 16);
-   ystride1 = (cfg->layout.plane_pitch[2]) |
-   (cfg->layout.plane_pitch[3] << 16);
+   ystride0 = (layout->plane_pitch[0]) |
+   (layout->plane_pitch[1] << 16);
+   ystride1 = (layout->plane_pitch[2]) |
+   (layout->plane_pitch[3] << 16);
} else {
ystride0 = DPU_REG_READ(>hw, SSPP_SRC_YSTRIDE0 + idx);
ystride1 = DPU_REG_READ(>hw, SSPP_SRC_YSTRIDE1 + idx);
 
if (pipe->multirect_index == DPU_SSPP_RECT_0) {
ystride0 = (ystride0 & 0x) |
-   (cfg->layout.plane_pitch[0] & 0x);
+   (layout->plane_pitch[0] & 0x);
ystride1 = (ystride1 & 0x)|
-   (cfg->layout.plane_pitch[2] & 0x);
+   (layout->plane_pitch[2] & 0x);
} else {
ystride0 = (ystride0 & 0x) |
-   ((cfg->layout.plane_pitch[0] << 16) &
+   ((layout->plane_pitch[0] << 16) &
 0x);
ystride1 = (ystride1 & 0x) |
-   ((cfg->layout.plane_pitch[2] << 16) &
+   ((layout->plane_pitch[2] << 16) &
 0x);
}
}
@@ -564,7 +564,7 @@ static void dpu_hw_sspp_setup_csc(struct dpu_hw_sspp *ctx,
 static void dpu_hw_sspp_setup_solidfill(struct dpu_sw_pipe *pipe, u32 color)
 {
struct dpu_hw_sspp *ctx = pipe->sspp;
-   struct dpu_hw_sspp_cfg cfg;
+   struct dpu_hw_fmt_layout cfg;
u32 idx;
 
if (_sspp_subblk_offset(ctx, DPU_SSPP_SRC, ))
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index 136b8713943f..100d8e06c90d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -154,13 +154,11 @@ struct dpu_hw_pixel_ext {
 
 /**
  * struct dpu_hw_sspp_cfg : SSPP configuration
- * @layout:format layout information for programming buffer to hardware
  * @src_rect:  src ROI, caller takes 

[Freedreno] [PATCH v4 10/30] drm/msm/dpu: clean up SRC addresses when setting up SSPP for solid fill

2023-03-03 Thread Dmitry Baryshkov
Set SSPP_SRCn_ADDR registers to 0 while setting up solid fill, as we can
not be sure that the previous address is still valid.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index 3030cd3b253a..0a43c5682b2b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -563,11 +563,16 @@ static void dpu_hw_sspp_setup_csc(struct dpu_hw_sspp *ctx,
 static void dpu_hw_sspp_setup_solidfill(struct dpu_sw_pipe *pipe, u32 color)
 {
struct dpu_hw_sspp *ctx = pipe->sspp;
+   struct dpu_hw_sspp_cfg cfg;
u32 idx;
 
if (_sspp_subblk_offset(ctx, DPU_SSPP_SRC, ))
return;
 
+   /* cleanup source addresses */
+   memset(, 0, sizeof(cfg));
+   ctx->ops.setup_sourceaddress(pipe, );
+
if (pipe->multirect_index == DPU_SSPP_RECT_SOLO ||
pipe->multirect_index == DPU_SSPP_RECT_0)
DPU_REG_WRITE(>hw, SSPP_SRC_CONSTANT_COLOR + idx, color);
-- 
2.39.2



[Freedreno] [PATCH v4 09/30] drm/msm/dpu: pass dpu_format to _dpu_hw_sspp_setup_scaler3()

2023-03-03 Thread Dmitry Baryshkov
There is no need to pass full dpu_hw_sspp_cfg instance to
_dpu_hw_sspp_setup_scaler3, pass just struct dpu_format pointer.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 9 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 9 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 4 ++--
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index a1492a7e43ce..3030cd3b253a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -419,19 +419,18 @@ static void dpu_hw_sspp_setup_pe_config(struct 
dpu_hw_sspp *ctx,
 }
 
 static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_sspp *ctx,
-   struct dpu_hw_sspp_cfg *sspp,
-   void *scaler_cfg)
+   struct dpu_hw_scaler3_cfg *scaler3_cfg,
+   const struct dpu_format *format)
 {
u32 idx;
-   struct dpu_hw_scaler3_cfg *scaler3_cfg = scaler_cfg;
 
-   if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, ) || !sspp
+   if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, )
|| !scaler3_cfg)
return;
 
dpu_hw_setup_scaler3(>hw, scaler3_cfg, idx,
ctx->cap->sblk->scaler_blk.version,
-   sspp->layout.format);
+   format);
 }
 
 static u32 _dpu_hw_sspp_get_scaler3_ver(struct dpu_hw_sspp *ctx)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index 5903413256ea..136b8713943f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -317,13 +317,12 @@ struct dpu_hw_sspp_ops {
 
/**
 * setup_scaler - setup scaler
-* @ctx: Pointer to pipe context
-* @pipe_cfg: Pointer to pipe configuration
-* @scaler_cfg: Pointer to scaler configuration
+* @scaler3_cfg: Pointer to scaler configuration
+* @format: pixel format parameters
 */
void (*setup_scaler)(struct dpu_hw_sspp *ctx,
-   struct dpu_hw_sspp_cfg *pipe_cfg,
-   void *scaler_cfg);
+   struct dpu_hw_scaler3_cfg *scaler3_cfg,
+   const struct dpu_format *format);
 
/**
 * get_scaler_ver - get scaler h/w version
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 6ec39f937042..8c98385303ea 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -677,8 +677,8 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe 
*pipe,
if (pipe_hw->ops.setup_scaler &&
pipe->multirect_index != DPU_SSPP_RECT_1)
pipe_hw->ops.setup_scaler(pipe_hw,
-   pipe_cfg,
-   _cfg);
+   _cfg,
+   fmt);
 }
 
 /**
-- 
2.39.2



[Freedreno] [PATCH v4 07/30] drm/msm/dpu: introduce struct dpu_sw_pipe

2023-03-03 Thread Dmitry Baryshkov
Wrap SSPP and multirect index/mode into a single structure that
represents software view on the pipe used.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c|   9 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |  16 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 133 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |   6 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h   |  10 +-
 5 files changed, 90 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index eff1a3cc1cec..037347e51eb8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -431,7 +431,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
 
-   sspp_idx = pstate->hw_sspp->idx;
+   sspp_idx = pstate->pipe.sspp->idx;
set_bit(sspp_idx, fetch_active);
 
DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
@@ -450,11 +450,10 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
stage_cfg->stage[pstate->stage][stage_idx] =
sspp_idx;
stage_cfg->multirect_index[pstate->stage][stage_idx] =
-   pstate->multirect_index;
+   pstate->pipe.multirect_index;
 
trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
   state, pstate, stage_idx,
-  sspp_idx - SSPP_VIG0,
   format->base.pixel_format,
   fb ? fb->modifier : 0);
 
@@ -1202,7 +1201,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
pstates[cnt].dpu_pstate = dpu_pstate;
pstates[cnt].drm_pstate = pstate;
pstates[cnt].stage = pstate->normalized_zpos;
-   pstates[cnt].pipe_id = to_dpu_plane_state(pstate)->hw_sspp->idx;
+   pstates[cnt].pipe_id = 
to_dpu_plane_state(pstate)->pipe.sspp->idx;
 
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
 
@@ -1475,7 +1474,7 @@ static int _dpu_debugfs_status_show(struct seq_file *s, 
void *data)
state->crtc_x, state->crtc_y, state->crtc_w,
state->crtc_h);
seq_printf(s, "\tmultirect: mode: %d index: %d\n",
-   pstate->multirect_mode, pstate->multirect_index);
+   pstate->pipe.multirect_mode, 
pstate->pipe.multirect_index);
 
seq_puts(s, "\n");
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index c30f168b6c0a..13d9e04a5153 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -158,15 +158,11 @@ struct dpu_hw_pixel_ext {
  * @src_rect:  src ROI, caller takes into account the different operations
  * such as decimation, flip etc to program this field
  * @dest_rect: destination ROI.
- * @index: index of the rectangle of SSPP
- * @mode:  parallel or time multiplex multirect mode
  */
 struct dpu_hw_sspp_cfg {
struct dpu_hw_fmt_layout layout;
struct drm_rect src_rect;
struct drm_rect dst_rect;
-   enum dpu_sspp_multirect_index index;
-   enum dpu_sspp_multirect_mode mode;
 };
 
 /**
@@ -201,6 +197,18 @@ struct dpu_hw_pipe_ts_cfg {
u64 time;
 };
 
+/**
+ * struct dpu_sw_pipe - software pipe description
+ * @sspp:  backing SSPP pipe
+ * @index: index of the rectangle of SSPP
+ * @mode:  parallel or time multiplex multirect mode
+ */
+struct dpu_sw_pipe {
+   struct dpu_hw_sspp *sspp;
+   enum dpu_sspp_multirect_index multirect_index;
+   enum dpu_sspp_multirect_mode multirect_mode;
+};
+
 /**
  * struct dpu_hw_sspp_ops - interface to the SSPP Hw driver functions
  * Caller must call the init function to get the pipe context for each pipe
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 10678f6502a2..ce726bec0c31 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -251,7 +251,7 @@ static int _dpu_plane_calc_fill_level(struct drm_plane 
*plane,
((src_width + 32) * fmt->bpp);
}
} else {
-   if (pstate->multirect_mode == DPU_SSPP_MULTIRECT_PARALLEL) {
+   if (pstate->pipe.multirect_mode == DPU_SSPP_MULTIRECT_PARALLEL) 
{
total_fl = (fixed_buff_size / 2) * 2 /
((src_width + 32) * fmt->bpp);
} else {
@@ -310,7 +310,7 @@ 

[Freedreno] [PATCH v4 04/30] drm/msm/dpu: drop EAGAIN check from dpu_format_populate_layout

2023-03-03 Thread Dmitry Baryshkov
The pipe's layout is not cached, corresponding data structure is zeroed
out each time in the dpu_plane_sspp_atomic_update(), right before the
call to _dpu_plane_set_scanout() -> dpu_format_populate_layout().

Drop plane_addr comparison against previous layout and corresponding
EAGAIN handling.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   |  4 +---
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index d95540309d4d..ec1001e10f4f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -918,8 +918,7 @@ int dpu_format_populate_layout(
struct drm_framebuffer *fb,
struct dpu_hw_fmt_layout *layout)
 {
-   uint32_t plane_addr[DPU_MAX_PLANES];
-   int i, ret;
+   int ret;
 
if (!fb || !layout) {
DRM_ERROR("invalid arguments\n");
@@ -940,9 +939,6 @@ int dpu_format_populate_layout(
if (ret)
return ret;
 
-   for (i = 0; i < DPU_MAX_PLANES; ++i)
-   plane_addr[i] = layout->plane_addr[i];
-
/* Populate the addresses given the fb */
if (DPU_FORMAT_IS_UBWC(layout->format) ||
DPU_FORMAT_IS_TILE(layout->format))
@@ -950,10 +946,6 @@ int dpu_format_populate_layout(
else
ret = _dpu_format_populate_addrs_linear(aspace, fb, layout);
 
-   /* check if anything changed */
-   if (!ret && !memcmp(plane_addr, layout->plane_addr, sizeof(plane_addr)))
-   ret = -EAGAIN;
-
return ret;
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 2b0ebdd4c207..d6518ef1beb2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -476,9 +476,7 @@ static void _dpu_plane_set_scanout(struct drm_plane *plane,
int ret;
 
ret = dpu_format_populate_layout(aspace, fb, _cfg->layout);
-   if (ret == -EAGAIN)
-   DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
-   else if (ret)
+   if (ret)
DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
else if (pdpu->pipe_hw->ops.setup_sourceaddress) {
trace_dpu_plane_set_scanout(pdpu->pipe_hw->idx,
-- 
2.39.2



[Freedreno] [PATCH v4 00/30] drm/msm/dpu: wide planes support

2023-03-03 Thread Dmitry Baryshkov
This patchset brings in multirect usage to support using two SSPP
rectangles for a single plane. Full virtual planes support is omitted
from this pull request, it will come later.

Changes since v3:

- moved if (!pipe->sspp) checks back to the calling site, the caller
  should know if there is a backing SSPP or not.
- Restored state_idx argument of trace_dpu_crtc_setup_mixer trace point
- Removed .smart_dma_rev from dpu_caps
- Added cleaning of multirect to _dpu_plane_atomic_disable()
- Per Abhinavs request split the SmartDMA enablement patch into the
  "verified by me" and "the rest of the platforms" patches, which is not
  supposed to be merged in. Users of other platforms are supposed to
  verify multirect support on their platforms and then send patches
  enabling SmartDMA for their SoC.
- Expanded several commit messages

Changes since v2:

- Renamed dpu_hw_pipe_cfg to dpu_hw_sspp_cfg
- Added a patch to clean up src add / layout for the solid fill planes
- Fixed several comments and commit messages which caused confusion
- Added documentation for new dpu_plane_state members
- Slightly reworked dpu_plane_atomic_check() to make it more logical and 
obvious.

Changes since v1 (which was ages ago):
- Rebased on top of 6.2-rc1
- Dropped the controversial _dpu_crtc_blend_setup() split patch
- Renamed dpu_hw_pipe to dpu_hw_sspp
- Other misc changes

Dmitry Baryshkov (30):
  drm/msm/dpu: rename struct dpu_hw_pipe(_cfg) to dpu_hw_sspp(_cfg)
  drm/msm/dpu: move SSPP allocation to the RM
  drm/msm/dpu: move SSPP debugfs creation to dpu_kms.c
  drm/msm/dpu: drop EAGAIN check from dpu_format_populate_layout
  drm/msm/dpu: move pipe_hw to dpu_plane_state
  drm/msm/dpu: drop dpu_plane_pipe function
  drm/msm/dpu: introduce struct dpu_sw_pipe
  drm/msm/dpu: use dpu_sw_pipe for dpu_hw_sspp callbacks
  drm/msm/dpu: pass dpu_format to _dpu_hw_sspp_setup_scaler3()
  drm/msm/dpu: clean up SRC addresses when setting up SSPP for solid
fill
  drm/msm/dpu: move stride programming to
dpu_hw_sspp_setup_sourceaddress
  drm/msm/dpu: remove dpu_hw_fmt_layout from struct dpu_hw_sspp_cfg
  drm/msm/dpu: rename dpu_hw_sspp_cfg to dpu_sw_pipe_cfg
  drm/msm/dpu: drop src_split and multirect check from
dpu_crtc_atomic_check
  drm/msm/dpu: don't use unsupported blend stages
  drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
  drm/msm/dpu: drop redundant plane dst check from
dpu_crtc_atomic_check()
  drm/msm/dpu: rewrite plane's QoS-related functions to take dpu_sw_pipe
and dpu_format
  drm/msm/dpu: make _dpu_plane_calc_clk accept mode directly
  drm/msm/dpu: add dpu_hw_sspp_cfg to dpu_plane_state
  drm/msm/dpu: simplify dpu_plane_validate_src()
  drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
  drm/msm/dpu: rework dpu_plane_atomic_check()
  drm/msm/dpu: rework plane CSC setting
  drm/msm/dpu: rework static color fill code
  drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
  drm/msm/dpu: add support for wide planes
  drm/msm/dpu: populate SmartDMA features in hw catalog
  drm/msm/dpu: enable SmartDMA for the rest of the platforms
  drm/msm/dpu: drop smart_dma_rev from dpu_caps

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 289 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c   |  10 +-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c|  23 +-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h|   2 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c   | 169 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h   | 111 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |  18 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 756 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  23 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c|  22 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h|  12 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  19 +-
 12 files changed, 719 insertions(+), 735 deletions(-)

-- 
2.39.2



[Freedreno] [PATCH v4 03/30] drm/msm/dpu: move SSPP debugfs creation to dpu_kms.c

2023-03-03 Thread Dmitry Baryshkov
As SSPP blocks are now visible through dpu_kms->rm.sspp_blocks, move
SSPP debugfs creation from dpu_plane to dpu_kms. We are going to break
the 1:1 correspondence between planes and SSPPs, so it makes no sense
anymore to create SSPP debugfs entries in dpu_plane.c

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |  1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 18 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 16 
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index bbff908e6dbe..c30f168b6c0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -388,7 +388,6 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(enum dpu_sspp idx,
  */
 void dpu_hw_sspp_destroy(struct dpu_hw_sspp *ctx);
 
-void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry 
*debugfs_root);
 int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
  struct dentry *entry);
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index a683bd9b5a04..0d2ef83c38ea 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -250,6 +250,24 @@ void dpu_debugfs_create_regset32(const char *name, umode_t 
mode,
debugfs_create_file(name, mode, parent, regset, _regset32_fops);
 }
 
+static void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry 
*debugfs_root)
+{
+   struct dentry *entry = debugfs_create_dir("sspp", debugfs_root);
+   int i;
+
+   if (IS_ERR(entry))
+   return;
+
+   for (i = SSPP_NONE; i < SSPP_MAX; i++) {
+   struct dpu_hw_sspp *hw = dpu_rm_get_sspp(_kms->rm, i);
+
+   if (!hw)
+   continue;
+
+   _dpu_hw_sspp_init_debugfs(hw, dpu_kms, entry);
+   }
+}
+
 static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
 {
struct dpu_kms *dpu_kms = to_dpu_kms(kms);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index b054055f120b..2b0ebdd4c207 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1399,22 +1399,6 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane 
*plane, bool enable)
_dpu_plane_set_qos_ctrl(plane, enable, DPU_PLANE_QOS_PANIC_CTRL);
pm_runtime_put_sync(_kms->pdev->dev);
 }
-
-/* SSPP live inside dpu_plane private data only. Enumerate them here. */
-void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry 
*debugfs_root)
-{
-   struct drm_plane *plane;
-   struct dentry *entry = debugfs_create_dir("sspp", debugfs_root);
-
-   if (IS_ERR(entry))
-   return;
-
-   drm_for_each_plane(plane, dpu_kms->dev) {
-   struct dpu_plane *pdpu = to_dpu_plane(plane);
-
-   _dpu_hw_sspp_init_debugfs(pdpu->pipe_hw, dpu_kms, entry);
-   }
-}
 #endif
 
 static bool dpu_plane_format_mod_supported(struct drm_plane *plane,
-- 
2.39.2



[Freedreno] [PATCH v4 02/30] drm/msm/dpu: move SSPP allocation to the RM

2023-03-03 Thread Dmitry Baryshkov
Follow the example of all other hw blocks and initialize SSPP blocks in
Resource Manager.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 17 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 22 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h| 12 
 3 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 5a4578ab62a6..b054055f120b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1275,8 +1275,6 @@ static void dpu_plane_destroy(struct drm_plane *plane)
/* this will destroy the states as well */
drm_plane_cleanup(plane);
 
-   dpu_hw_sspp_destroy(pdpu->pipe_hw);
-
kfree(pdpu);
}
 }
@@ -1482,14 +1480,10 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
pdpu->pipe = pipe;
 
/* initialize underlying h/w driver */
-   pdpu->pipe_hw = dpu_hw_sspp_init(pipe, kms->mmio, kms->catalog);
-   if (IS_ERR(pdpu->pipe_hw)) {
-   DPU_ERROR("[%u]SSPP init failed\n", pipe);
-   ret = PTR_ERR(pdpu->pipe_hw);
+   pdpu->pipe_hw = dpu_rm_get_sspp(>rm, pipe);
+   if (!pdpu->pipe_hw || !pdpu->pipe_hw->cap || !pdpu->pipe_hw->cap->sblk) 
{
+   DPU_ERROR("[%u]SSPP is invalid\n", pipe);
goto clean_plane;
-   } else if (!pdpu->pipe_hw->cap || !pdpu->pipe_hw->cap->sblk) {
-   DPU_ERROR("[%u]SSPP init returned invalid cfg\n", pipe);
-   goto clean_sspp;
}
 
format_list = pdpu->pipe_hw->cap->sblk->format_list;
@@ -1499,7 +1493,7 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
format_list, num_formats,
supported_format_modifiers, type, NULL);
if (ret)
-   goto clean_sspp;
+   goto clean_plane;
 
pdpu->catalog = kms->catalog;
 
@@ -1532,9 +1526,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
pipe, plane->base.id);
return plane;
 
-clean_sspp:
-   if (pdpu && pdpu->pipe_hw)
-   dpu_hw_sspp_destroy(pdpu->pipe_hw);
 clean_plane:
kfree(pdpu);
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 396429e63756..53c644ca52ef 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -8,6 +8,7 @@
 #include "dpu_hw_lm.h"
 #include "dpu_hw_ctl.h"
 #include "dpu_hw_pingpong.h"
+#include "dpu_hw_sspp.h"
 #include "dpu_hw_intf.h"
 #include "dpu_hw_wb.h"
 #include "dpu_hw_dspp.h"
@@ -91,6 +92,9 @@ int dpu_rm_destroy(struct dpu_rm *rm)
for (i = 0; i < ARRAY_SIZE(rm->hw_wb); i++)
dpu_hw_wb_destroy(rm->hw_wb[i]);
 
+   for (i = 0; i < ARRAY_SIZE(rm->hw_sspp); i++)
+   dpu_hw_sspp_destroy(rm->hw_sspp[i]);
+
return 0;
 }
 
@@ -255,6 +259,24 @@ int dpu_rm_init(struct dpu_rm *rm,
rm->dsc_blks[dsc->id - DSC_0] = >base;
}
 
+   for (i = 0; i < cat->sspp_count; i++) {
+   struct dpu_hw_sspp *hw;
+   const struct dpu_sspp_cfg *sspp = >sspp[i];
+
+   if (sspp->id < SSPP_NONE || sspp->id >= SSPP_MAX) {
+   DPU_ERROR("skip intf %d with invalid id\n", sspp->id);
+   continue;
+   }
+
+   hw = dpu_hw_sspp_init(sspp->id, mmio, cat);
+   if (IS_ERR(hw)) {
+   rc = PTR_ERR(hw);
+   DPU_ERROR("failed sspp object creation: err %d\n", rc);
+   goto fail;
+   }
+   rm->hw_sspp[sspp->id - SSPP_NONE] = hw;
+   }
+
return 0;
 
 fail:
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 59de72b381f9..d62c2edb2460 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -21,6 +21,7 @@ struct dpu_global_state;
  * @hw_intf: array of intf hardware resources
  * @hw_wb: array of wb hardware resources
  * @dspp_blks: array of dspp hardware resources
+ * @hw_sspp: array of sspp hardware resources
  */
 struct dpu_rm {
struct dpu_hw_blk *pingpong_blks[PINGPONG_MAX - PINGPONG_0];
@@ -31,6 +32,7 @@ struct dpu_rm {
struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
+   struct dpu_hw_sspp *hw_sspp[SSPP_MAX - SSPP_NONE];
 };
 
 /**
@@ -108,5 +110,15 @@ static inline struct dpu_hw_wb *dpu_rm_get_wb(struct 
dpu_rm *rm, enum dpu_wb wb_
return rm->hw_wb[wb_idx - WB_0];
 }
 
+/**
+ * dpu_rm_get_sspp - Return a 

[Freedreno] [PATCH v4 01/30] drm/msm/dpu: rename struct dpu_hw_pipe(_cfg) to dpu_hw_sspp(_cfg)

2023-03-03 Thread Dmitry Baryshkov
For all hardware blocks except SSPP the corresponding struct is named
after the block. Rename dpu_hw_pipe (SSPP structure) to dpu_hw_sspp.
Also rename struct dpu_hw_pipe_cfg to dpu_hw_sspp_cfg to follow this
change.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 49 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 53 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 20 
 3 files changed, 62 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index 4246ab0b3bee..3e65bfd65b62 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -136,7 +136,7 @@
 #define TS_CLK 1920
 
 
-static int _sspp_subblk_offset(struct dpu_hw_pipe *ctx,
+static int _sspp_subblk_offset(struct dpu_hw_sspp *ctx,
int s_id,
u32 *idx)
 {
@@ -168,7 +168,7 @@ static int _sspp_subblk_offset(struct dpu_hw_pipe *ctx,
return rc;
 }
 
-static void dpu_hw_sspp_setup_multirect(struct dpu_hw_pipe *ctx,
+static void dpu_hw_sspp_setup_multirect(struct dpu_hw_sspp *ctx,
enum dpu_sspp_multirect_index index,
enum dpu_sspp_multirect_mode mode)
 {
@@ -197,7 +197,7 @@ static void dpu_hw_sspp_setup_multirect(struct dpu_hw_pipe 
*ctx,
DPU_REG_WRITE(>hw, SSPP_MULTIRECT_OPMODE + idx, mode_mask);
 }
 
-static void _sspp_setup_opmode(struct dpu_hw_pipe *ctx,
+static void _sspp_setup_opmode(struct dpu_hw_sspp *ctx,
u32 mask, u8 en)
 {
u32 idx;
@@ -218,7 +218,7 @@ static void _sspp_setup_opmode(struct dpu_hw_pipe *ctx,
DPU_REG_WRITE(>hw, SSPP_VIG_OP_MODE + idx, opmode);
 }
 
-static void _sspp_setup_csc10_opmode(struct dpu_hw_pipe *ctx,
+static void _sspp_setup_csc10_opmode(struct dpu_hw_sspp *ctx,
u32 mask, u8 en)
 {
u32 idx;
@@ -239,7 +239,7 @@ static void _sspp_setup_csc10_opmode(struct dpu_hw_pipe 
*ctx,
 /*
  * Setup source pixel format, flip,
  */
-static void dpu_hw_sspp_setup_format(struct dpu_hw_pipe *ctx,
+static void dpu_hw_sspp_setup_format(struct dpu_hw_sspp *ctx,
const struct dpu_format *fmt, u32 flags,
enum dpu_sspp_multirect_index rect_mode)
 {
@@ -360,7 +360,7 @@ static void dpu_hw_sspp_setup_format(struct dpu_hw_pipe 
*ctx,
DPU_REG_WRITE(c, SSPP_UBWC_ERROR_STATUS + idx, BIT(31));
 }
 
-static void dpu_hw_sspp_setup_pe_config(struct dpu_hw_pipe *ctx,
+static void dpu_hw_sspp_setup_pe_config(struct dpu_hw_sspp *ctx,
struct dpu_hw_pixel_ext *pe_ext)
 {
struct dpu_hw_blk_reg_map *c;
@@ -418,8 +418,8 @@ static void dpu_hw_sspp_setup_pe_config(struct dpu_hw_pipe 
*ctx,
tot_req_pixels[3]);
 }
 
-static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_pipe *ctx,
-   struct dpu_hw_pipe_cfg *sspp,
+static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_sspp *ctx,
+   struct dpu_hw_sspp_cfg *sspp,
void *scaler_cfg)
 {
u32 idx;
@@ -434,7 +434,7 @@ static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_pipe 
*ctx,
sspp->layout.format);
 }
 
-static u32 _dpu_hw_sspp_get_scaler3_ver(struct dpu_hw_pipe *ctx)
+static u32 _dpu_hw_sspp_get_scaler3_ver(struct dpu_hw_sspp *ctx)
 {
u32 idx;
 
@@ -447,8 +447,8 @@ static u32 _dpu_hw_sspp_get_scaler3_ver(struct dpu_hw_pipe 
*ctx)
 /*
  * dpu_hw_sspp_setup_rects()
  */
-static void dpu_hw_sspp_setup_rects(struct dpu_hw_pipe *ctx,
-   struct dpu_hw_pipe_cfg *cfg,
+static void dpu_hw_sspp_setup_rects(struct dpu_hw_sspp *ctx,
+   struct dpu_hw_sspp_cfg *cfg,
enum dpu_sspp_multirect_index rect_index)
 {
struct dpu_hw_blk_reg_map *c;
@@ -516,8 +516,8 @@ static void dpu_hw_sspp_setup_rects(struct dpu_hw_pipe *ctx,
DPU_REG_WRITE(c, SSPP_SRC_YSTRIDE1 + idx, ystride1);
 }
 
-static void dpu_hw_sspp_setup_sourceaddress(struct dpu_hw_pipe *ctx,
-   struct dpu_hw_pipe_cfg *cfg,
+static void dpu_hw_sspp_setup_sourceaddress(struct dpu_hw_sspp *ctx,
+   struct dpu_hw_sspp_cfg *cfg,
enum dpu_sspp_multirect_index rect_mode)
 {
int i;
@@ -543,7 +543,7 @@ static void dpu_hw_sspp_setup_sourceaddress(struct 
dpu_hw_pipe *ctx,
}
 }
 
-static void dpu_hw_sspp_setup_csc(struct dpu_hw_pipe *ctx,
+static void dpu_hw_sspp_setup_csc(struct dpu_hw_sspp *ctx,
const struct dpu_csc_cfg *data)
 {
u32 idx;
@@ -560,7 +560,7 @@ static void dpu_hw_sspp_setup_csc(struct dpu_hw_pipe *ctx,
dpu_hw_csc_setup(>hw, idx, data, csc10);
 }
 
-static void dpu_hw_sspp_setup_solidfill(struct dpu_hw_pipe *ctx, u32 color, 
enum
+static void dpu_hw_sspp_setup_solidfill(struct dpu_hw_sspp *ctx, u32 color, 
enum
dpu_sspp_multirect_index rect_index)
 {
u32 

Re: [Freedreno] [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer

2023-03-03 Thread Dmitry Baryshkov

On 09/02/2023 01:44, Abhinav Kumar wrote:



On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:

Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
separate functon. This is a preparation for the r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 86 ---
  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++-
  2 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

index 73e1a8c69ef0..0ca3bc38ff7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -400,6 +400,47 @@ static void 
_dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)

  }
  }
+static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
+   struct drm_plane *plane,
+   struct dpu_crtc_mixer *mixer,
+   u32 num_mixers,
+   struct dpu_hw_stage_cfg *stage_cfg,
+   enum dpu_stage stage,
+   unsigned int stage_idx,
+   unsigned long *fetch_active,
+   struct dpu_sw_pipe *pipe
+  )
+{
+    uint32_t lm_idx;
+    enum dpu_sspp sspp_idx;
+    struct drm_plane_state *state;
+
+    if (!pipe->sspp)
+    return;
+
+    sspp_idx = pipe->sspp->idx;
+
+    state = plane->state;
+
+    DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+ crtc->base.id,
+ stage,
+ plane->base.id,
+ sspp_idx - SSPP_NONE,
+ state->fb ? state->fb->base.id : -1);
+
+    set_bit(sspp_idx, fetch_active);
+
+    stage_cfg->stage[stage][stage_idx] = sspp_idx;
+    stage_cfg->multirect_index[stage][stage_idx] =
+    pipe->multirect_index;
+
+    /* blend config update */
+    for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,

+    sspp_idx);


If you just pass the format to this function you can move rest of the 
for loop also to this function.


As a second thought, this would defeat the purpose of the split. We 
don't have to call _dpu_crtc_setup_blend_cfg() or setup mixer_op_mode 
for the second pipe separately. So, I'd leave the loops as is.




Also, you will be able to add the trace_dpu_crtc_setup_mixer() with 
complete information.


trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is 
important to debug blend issues.



+}
+
  static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
  struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
  struct dpu_hw_stage_cfg *stage_cfg)
@@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct 
drm_crtc *crtc,

  struct dpu_format *format;
  struct dpu_hw_ctl *ctl = mixer->lm_ctl;
-    uint32_t stage_idx, lm_idx;
-    int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
+    uint32_t lm_idx;
  bool bg_alpha_enable = false;
  DECLARE_BITMAP(fetch_active, SSPP_MAX);
  memset(fetch_active, 0, sizeof(fetch_active));
  drm_atomic_crtc_for_each_plane(plane, crtc) {
-    enum dpu_sspp sspp_idx;
-
  state = plane->state;
  if (!state)
  continue;
@@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct 
drm_crtc *crtc,

  pstate = to_dpu_plane_state(state);
  fb = state->fb;
-    sspp_idx = pstate->pipe.sspp->idx;
-    set_bit(sspp_idx, fetch_active);
-
-    DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
-    crtc->base.id,
-    pstate->stage,
-    plane->base.id,
-    sspp_idx - SSPP_VIG0,
-    state->fb ? state->fb->base.id : -1);
-
  format = 
to_dpu_format(msm_framebuffer_format(pstate->base.fb));

  if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
  bg_alpha_enable = true;
-    stage_idx = zpos_cnt[pstate->stage]++;
-    stage_cfg->stage[pstate->stage][stage_idx] =
-    sspp_idx;
-    stage_cfg->multirect_index[pstate->stage][stage_idx] =
-    pstate->pipe.multirect_index;
-
  trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
-   state, pstate, stage_idx,
+   state, pstate,
 format->base.pixel_format,
 fb ? fb->modifier : 0);
+    _dpu_crtc_blend_setup_pipe(crtc, plane,
+   mixer, cstate->num_mixers,
+   stage_cfg, pstate->stage, 0,
+   fetch_active,
+   >pipe);
+
  /* blend config update */
  for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
-    _dpu_crtc_setup_blend_cfg(mixer + lm_idx,
-    pstate, format);
-
- 

Re: [Freedreno] [PATCH v3 15/27] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()

2023-03-03 Thread Dmitry Baryshkov

On 15/02/2023 02:08, Dmitry Baryshkov wrote:

On 15/02/2023 01:25, Abhinav Kumar wrote:

Hi Dmitry

Sorry for the late response on this one.

On 2/3/2023 2:55 PM, Dmitry Baryshkov wrote:

On 04/02/2023 00:44, Abhinav Kumar wrote:



On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:

Move plane state updates from dpu_crtc_atomic_check() to the function
where they belong: to dpu_plane_atomic_check().

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 18 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  6 --
  3 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

index b485234eefb2..bd09bb319a58 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1129,7 +1129,6 @@ static int dpu_crtc_atomic_check(struct 
drm_crtc *crtc,

    crtc);
  struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
  struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
-    struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
  const struct drm_plane_state *pstate;
  struct drm_plane *plane;
@@ -1161,11 +1160,10 @@ static int dpu_crtc_atomic_check(struct 
drm_crtc *crtc,

  crtc_rect.x2 = mode->hdisplay;
  crtc_rect.y2 = mode->vdisplay;
- /* get plane state for all drm planes associated with crtc 
state */

+    /* FIXME: move this to dpu_plane_atomic_check? */
  drm_atomic_crtc_state_for_each_plane_state(plane, pstate, 
crtc_state) {
  struct dpu_plane_state *dpu_pstate = 
to_dpu_plane_state(pstate);

  struct drm_rect dst, clip = crtc_rect;
-    int stage;
  if (IS_ERR_OR_NULL(pstate)) {
  rc = PTR_ERR(pstate);
@@ -1179,8 +1177,6 @@ static int dpu_crtc_atomic_check(struct 
drm_crtc *crtc,

  dpu_pstate->needs_dirtyfb = needs_dirtyfb;
-    dpu_plane_clear_multirect(pstate);
-
  dst = drm_plane_state_dest(pstate);
  if (!drm_rect_intersect(, )) {
  DPU_ERROR("invalid vertical/horizontal destination\n");
@@ -1189,18 +1185,6 @@ static int dpu_crtc_atomic_check(struct 
drm_crtc *crtc,

    DRM_RECT_ARG());
  return -E2BIG;
  }
-
-    /* verify stage setting before using it */
-    stage = DPU_STAGE_0 + pstate->normalized_zpos;
-    if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
-    DPU_ERROR("> %d plane stages assigned\n",
-    dpu_kms->catalog->caps->max_mixer_blendstages 
- DPU_STAGE_0);

-    return -EINVAL;
-    }
-
-    to_dpu_plane_state(pstate)->stage = stage;
-    DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
-
  }
  atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c

index 1b3033b15bfa..5aabf9694a53 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -733,14 +733,6 @@ static int _dpu_plane_color_fill(struct 
dpu_plane *pdpu,

  return 0;
  }
-void dpu_plane_clear_multirect(const struct drm_plane_state 
*drm_state)

-{
-    struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
-
-    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
-    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-}
-
  int dpu_plane_validate_multirect_v2(struct 
dpu_multirect_plane_states *plane)

  {
  struct dpu_plane_state *pstate[R_MAX];
@@ -994,6 +986,16 @@ static int dpu_plane_atomic_check(struct 
drm_plane *plane,

  if (!new_plane_state->visible)
  return 0;
+    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
+    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+


But I am not sure if clearing the multirect belongs here and now I 
want to clarify one thing about 
https://patchwork.freedesktop.org/patch/521354/?series=99909=4 
which was R-bed in the v1 and carried fwd since then.


So prior to that change, we were only clearing the multirects of the 
planes that were staged to the crtc and we were getting those from 
the crtc state. But now we are clearing the multirect of all the 
planes.


Dont we have to keep that in the crtc_atomic_check() since we do 
that on all the planes attached to a certain CRTC.


In that case shouldnt we keep this in the crtc_atomic_check() and 
bring back pipe_staged[] without the multirect and source split 
cases ofcourse.


What for? In other words, what would be the difference?



So, please correct my understanding here. drm_plane's atomic_check() 
will be called for all the planes which are getting updated in this 
atomic commit using for_each_oldnew_plane_in_state() and drm_crtc's 
atomic_check() will be called for all the CRTC's in this atomic update 
using 

Re: [Freedreno] [PATCH 3/4] drm/msm/adreno: drop redundant pm_runtime_disable()

2023-03-03 Thread Johan Hovold
Hi Rob,

Sorry about the late follow-up on this. Went down a bit of a DRM rabbit
hole this week.

On Wed, Feb 22, 2023 at 11:09:16AM -0800, Rob Clark wrote:
> On Tue, Feb 21, 2023 at 2:16 AM Johan Hovold  wrote:
> >
> > Since commit 4b18299b3365 ("drm/msm/adreno: Defer enabling runpm until
> > hw_init()") runtime PM is no longer enabled at adreno_gpu_init(), which
> > means that there are no longer any bind() error paths for which
> > adreno_gpu_cleanup() is called with runtime PM enabled.
> >
> > As the runtime PM enable on first open() is balanced by the
> > pm_runtime_force_suspend() call at unbind(), adreno_gpu_cleanup() is now
> > always called with runtime PM disabled so that its pm_runtime_disable()
> > call can be removed.
> >
> > Signed-off-by: Johan Hovold 
> > ---
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 5 -
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index ce6b76c45b6f..1101b8234b49 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -1082,15 +1082,10 @@ int adreno_gpu_init(struct drm_device *drm, struct 
> > platform_device *pdev,
> >
> >  void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
> >  {
> > -   struct msm_gpu *gpu = _gpu->base;
> > -   struct msm_drm_private *priv = gpu->dev ? gpu->dev->dev_private : 
> > NULL;
> > unsigned int i;
> >
> > for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++)
> > release_firmware(adreno_gpu->fw[i]);
> >
> > -   if (priv && pm_runtime_enabled(>gpu_pdev->dev))
> > -   pm_runtime_disable(>gpu_pdev->dev);
> > -
> 
> Maybe WARN_ON(priv && pm_runtime_enabled(>gpu_pdev->dev))?

I'd rather not add warnings for something that can not happen, but it
turns out there is indeed one corner case were this function could still
end up being called with runtime PM enabled, namely if suspending the
scheduler fails in adreno_system_suspend() during unbind:

adreno_bind()
 info->init()   // e.g. a6xx_gpu_init()
   adreno_gpu_init()

msm_open()
  load_gpu()
adreno_load_gpu()
  pm_runtime_enable()

adreno_unbind()
  adreno_system_suspend()
err = suspend_scheduler(gpu)
if (!err)
  pm_runtime_force_suspend()
pm_runtime_disable()
  gpu->funcs->destroy() // e.g. a6xx_destroy()
adreno_gpu_cleanup()

I assume we'd be in bigger troubles than just having an unbalanced
disable count if that ever happens, but we should probably just keep the
conditional disable in adreno_gpu_cleanup() in place for now.

> > msm_gpu_cleanup(_gpu->base);
> >  }
> > --
> > 2.39.2

I've found another related runtime PM issue so I'll send a v2 anyway.

Johan


Re: [Freedreno] [PATCH v9 12/15] drm/msm: Add deadline based boost support

2023-03-03 Thread Dmitry Baryshkov

On 03/03/2023 01:53, Rob Clark wrote:

From: Rob Clark 

Track the nearest deadline on a fence timeline and set a timer to expire
shortly before to trigger boost if the fence has not yet been signaled.

v2: rebase

Signed-off-by: Rob Clark 
---
  drivers/gpu/drm/msm/msm_fence.c | 74 +
  drivers/gpu/drm/msm/msm_fence.h | 20 +
  2 files changed, 94 insertions(+)


Reviewed-by: Dmitry Baryshkov 

A small question: do we boost to fit into the deadline or to miss the 
deadline for as little as possible? If the former is the case, we might 
need to adjust 3ms depending on the workload.




diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 56641408ea74..51b461f32103 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -8,6 +8,35 @@
  
  #include "msm_drv.h"

  #include "msm_fence.h"
+#include "msm_gpu.h"
+
+static struct msm_gpu *fctx2gpu(struct msm_fence_context *fctx)
+{
+   struct msm_drm_private *priv = fctx->dev->dev_private;
+   return priv->gpu;
+}
+
+static enum hrtimer_restart deadline_timer(struct hrtimer *t)
+{
+   struct msm_fence_context *fctx = container_of(t,
+   struct msm_fence_context, deadline_timer);
+
+   kthread_queue_work(fctx2gpu(fctx)->worker, >deadline_work);
+
+   return HRTIMER_NORESTART;
+}
+
+static void deadline_work(struct kthread_work *work)
+{
+   struct msm_fence_context *fctx = container_of(work,
+   struct msm_fence_context, deadline_work);
+
+   /* If deadline fence has already passed, nothing to do: */
+   if (msm_fence_completed(fctx, fctx->next_deadline_fence))
+   return;
+
+   msm_devfreq_boost(fctx2gpu(fctx), 2);
+}
  
  
  struct msm_fence_context *

@@ -36,6 +65,13 @@ msm_fence_context_alloc(struct drm_device *dev, volatile 
uint32_t *fenceptr,
fctx->completed_fence = fctx->last_fence;
*fctx->fenceptr = fctx->last_fence;
  
+	hrtimer_init(>deadline_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);

+   fctx->deadline_timer.function = deadline_timer;
+
+   kthread_init_work(>deadline_work, deadline_work);
+
+   fctx->next_deadline = ktime_get();
+
return fctx;
  }
  
@@ -62,6 +98,8 @@ void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence)

spin_lock_irqsave(>spinlock, flags);
if (fence_after(fence, fctx->completed_fence))
fctx->completed_fence = fence;
+   if (msm_fence_completed(fctx, fctx->next_deadline_fence))
+   hrtimer_cancel(>deadline_timer);
spin_unlock_irqrestore(>spinlock, flags);
  }
  
@@ -92,10 +130,46 @@ static bool msm_fence_signaled(struct dma_fence *fence)

return msm_fence_completed(f->fctx, f->base.seqno);
  }
  
+static void msm_fence_set_deadline(struct dma_fence *fence, ktime_t deadline)

+{
+   struct msm_fence *f = to_msm_fence(fence);
+   struct msm_fence_context *fctx = f->fctx;
+   unsigned long flags;
+   ktime_t now;
+
+   spin_lock_irqsave(>spinlock, flags);
+   now = ktime_get();
+
+   if (ktime_after(now, fctx->next_deadline) ||
+   ktime_before(deadline, fctx->next_deadline)) {
+   fctx->next_deadline = deadline;
+   fctx->next_deadline_fence =
+   max(fctx->next_deadline_fence, (uint32_t)fence->seqno);
+
+   /*
+* Set timer to trigger boost 3ms before deadline, or
+* if we are already less than 3ms before the deadline
+* schedule boost work immediately.
+*/
+   deadline = ktime_sub(deadline, ms_to_ktime(3));
+
+   if (ktime_after(now, deadline)) {
+   kthread_queue_work(fctx2gpu(fctx)->worker,
+   >deadline_work);
+   } else {
+   hrtimer_start(>deadline_timer, deadline,
+   HRTIMER_MODE_ABS);
+   }
+   }
+
+   spin_unlock_irqrestore(>spinlock, flags);
+}
+
  static const struct dma_fence_ops msm_fence_ops = {
.get_driver_name = msm_fence_get_driver_name,
.get_timeline_name = msm_fence_get_timeline_name,
.signaled = msm_fence_signaled,
+   .set_deadline = msm_fence_set_deadline,
  };
  
  struct dma_fence *

diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h
index 7f1798c54cd1..cdaebfb94f5c 100644
--- a/drivers/gpu/drm/msm/msm_fence.h
+++ b/drivers/gpu/drm/msm/msm_fence.h
@@ -52,6 +52,26 @@ struct msm_fence_context {
volatile uint32_t *fenceptr;
  
  	spinlock_t spinlock;

+
+   /*
+* TODO this doesn't really deal with multiple deadlines, like
+* if userspace got multiple frames ahead.. OTOH atomic updates
+* don't queue, so maybe that is ok
+*/
+
+   /** next_deadline: Time of next deadline */
+   

Re: [Freedreno] [PATCH v9 13/15] drm/msm: Add wait-boost support

2023-03-03 Thread Dmitry Baryshkov

On 03/03/2023 01:53, Rob Clark wrote:

From: Rob Clark 

Add a way for various userspace waits to signal urgency.

Signed-off-by: Rob Clark 
---
  drivers/gpu/drm/msm/msm_drv.c | 12 
  drivers/gpu/drm/msm/msm_gem.c |  5 +
  include/uapi/drm/msm_drm.h| 14 --
  3 files changed, 25 insertions(+), 6 deletions(-)


Reviewed-by: Dmitry Baryshkov 

--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v9 14/15] drm/msm/atomic: Switch to vblank_start helper

2023-03-03 Thread Dmitry Baryshkov

On 03/03/2023 01:53, Rob Clark wrote:

From: Rob Clark 

Drop our custom thing and switch to drm_crtc_next_vblank_start() for
calculating the time of the start of the next vblank period.

Signed-off-by: Rob Clark 


It took me a while to dig into the differences between old and proposed 
paths. Looks correct to me.


Reviewed-by: Dmitry Baryshkov 


---
  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 15 ---
  drivers/gpu/drm/msm/msm_atomic.c|  8 +---
  drivers/gpu/drm/msm/msm_kms.h   |  8 
  3 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index a683bd9b5a04..43996aecaf8c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -411,20 +411,6 @@ static void dpu_kms_disable_commit(struct msm_kms *kms)
pm_runtime_put_sync(_kms->pdev->dev);
  }
  
-static ktime_t dpu_kms_vsync_time(struct msm_kms *kms, struct drm_crtc *crtc)

-{
-   struct drm_encoder *encoder;
-
-   drm_for_each_encoder_mask(encoder, crtc->dev, 
crtc->state->encoder_mask) {
-   ktime_t vsync_time;
-
-   if (dpu_encoder_vsync_time(encoder, _time) == 0)
-   return vsync_time;
-   }
-
-   return ktime_get();
-}
-
  static void dpu_kms_prepare_commit(struct msm_kms *kms,
struct drm_atomic_state *state)
  {
@@ -953,7 +939,6 @@ static const struct msm_kms_funcs kms_funcs = {
.irq = dpu_core_irq,
.enable_commit   = dpu_kms_enable_commit,
.disable_commit  = dpu_kms_disable_commit,
-   .vsync_time  = dpu_kms_vsync_time,
.prepare_commit  = dpu_kms_prepare_commit,
.flush_commit= dpu_kms_flush_commit,
.wait_flush  = dpu_kms_wait_flush,
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 1686fbb611fd..c5e71c05f038 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -186,8 +186,7 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state)
struct msm_kms *kms = priv->kms;
struct drm_crtc *async_crtc = NULL;
unsigned crtc_mask = get_crtc_mask(state);
-   bool async = kms->funcs->vsync_time &&
-   can_do_async(state, _crtc);
+   bool async = can_do_async(state, _crtc);
  
  	trace_msm_atomic_commit_tail_start(async, crtc_mask);
  
@@ -231,7 +230,9 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state)
  
  			kms->pending_crtc_mask |= crtc_mask;
  
-			vsync_time = kms->funcs->vsync_time(kms, async_crtc);

+   if (drm_crtc_next_vblank_start(async_crtc, _time))
+   goto fallback;
+
wakeup_time = ktime_sub(vsync_time, ms_to_ktime(1));
  
  			msm_hrtimer_queue_work(>work, wakeup_time,

@@ -253,6 +254,7 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state)
return;
}
  
+fallback:

/*
 * If there is any async flush pending on updated crtcs, fold
 * them into the current flush.
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index f8ed7588928c..086a3f1ff956 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -59,14 +59,6 @@ struct msm_kms_funcs {
void (*enable_commit)(struct msm_kms *kms);
void (*disable_commit)(struct msm_kms *kms);
  
-	/**

-* If the kms backend supports async commit, it should implement
-* this method to return the time of the next vsync.  This is
-* used to determine a time slightly before vsync, for the async
-* commit timer to run and complete an async commit.
-*/
-   ktime_t (*vsync_time)(struct msm_kms *kms, struct drm_crtc *crtc);
-
/**
 * Prepare for atomic commit.  This is called after any previous
 * (async or otherwise) commit has completed.


--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support

2023-03-03 Thread Tvrtko Ursulin



On 03/03/2023 03:21, Rodrigo Vivi wrote:

On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:

From: Rob Clark 



missing some wording here...


v2: rebase

Signed-off-by: Rob Clark 
---
  drivers/gpu/drm/i915/i915_request.c | 20 
  1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_request.c 
b/drivers/gpu/drm/i915/i915_request.c
index 7503dcb9043b..44491e7e214c 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct dma_fence 
*fence)
return i915_request_enable_breadcrumb(to_request(fence));
  }
  
+static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t deadline)

+{
+   struct i915_request *rq = to_request(fence);
+
+   if (i915_request_completed(rq))
+   return;
+
+   if (i915_request_started(rq))
+   return;


why do we skip the boost if already started?
don't we want to boost the freq anyway?


I'd wager Rob is just copying the current i915 wait boost logic.


+
+   /*
+* TODO something more clever for deadlines that are in the
+* future.  I think probably track the nearest deadline in
+* rq->timeline and set timer to trigger boost accordingly?
+*/


I'm afraid it will be very hard to find some heuristics of what's
late enough for the boost no?
I mean, how early to boost the freq on an upcoming deadline for the
timer?


We can off load this patch from Rob and deal with it separately, or 
after the fact?


It's a half solution without a smarter scheduler too. Like 
https://lore.kernel.org/all/20210208105236.28498-10-ch...@chris-wilson.co.uk/, 
or if GuC plans to do something like that at any point.


Or bump the priority too if deadline is looming?

IMO it is not very effective to fiddle with the heuristic on an ad-hoc 
basis. For instance I have a new heuristics which improves the 
problematic OpenCL cases for further 5% (relative to the current 
waitboost improvement from adding missing syncobj waitboost). But I 
can't really test properly for regressions over platforms, stacks, 
workloads.. :(


Regards,

Tvrtko




+
+   intel_rps_boost(rq);
+}
+
  static signed long i915_fence_wait(struct dma_fence *fence,
   bool interruptible,
   signed long timeout)
@@ -182,6 +201,7 @@ const struct dma_fence_ops i915_fence_ops = {
.signaled = i915_fence_signaled,
.wait = i915_fence_wait,
.release = i915_fence_release,
+   .set_deadline = i915_fence_set_deadline,
  };
  
  static void irq_execute_cb(struct irq_work *wrk)

--
2.39.1