Re: [PATCH v3 04/11] drm/msm/dpu: drop separate dpu_core_perf_tune overrides

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

 





On 7/10/2023 7:34 PM, Dmitry Baryshkov wrote:
On 11/07/2023 05:31, Abhinav Kumar wrote:


On 7/7/2023 12:39 PM, Dmitry Baryshkov wrote:
The values in struct dpu_core_perf_tune are fixed per the core perf
mode. Drop the 'tune' values and substitute them with known values when
performing perf management.

Note: min_bus_vote was not used at all, so it is just silently dropped.


Interesting ..... should bring this back properly. Will take it up.

Ack, thanks.


Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 29 ++++++++-----------
  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  4 ---
  2 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 05d340aa18c5..348550ac7e51 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -235,7 +235,7 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
  {
      struct dpu_core_perf_params perf = { 0 };
      int i, ret = 0;
-    u64 avg_bw;
+    u32 avg_bw;

avg_bw seems unused in this patch, so unrelated change?

      if (!kms->num_paths)
          return 0;
@@ -291,10 +291,16 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
  static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
  {
-    u64 clk_rate = kms->perf.perf_tune.min_core_clk;
+    u64 clk_rate;
      struct drm_crtc *crtc;
      struct dpu_crtc_state *dpu_cstate;
+    if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED)
+        return kms->perf.fix_core_clk_rate;
+
+    if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM)
+        return kms->perf.max_core_clk_rate;
+
      drm_for_each_crtc(crtc, kms->dev) {
          if (crtc->enabled) {
              dpu_cstate = to_dpu_crtc_state(crtc->state);
@@ -305,11 +311,6 @@ static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
          }
      }
-    if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED)
-        clk_rate = kms->perf.fix_core_clk_rate;
-
-    DRM_DEBUG_ATOMIC("clk:%llu\n", clk_rate);
-

Why dont you move both FIXED and MINIMUM handling below instead of above.

So that they will just override the clk_rate and you can keep this useful log here and it matches where the function is.

I can keep the log in the next version. The logic was quite simple: there is no need to loop over CRTCs if we know that we are overriding the value.


Yes I understood that part. I wanted to keep the log and the function together that way we are logging whats the value its going to return.

This patch is logging it at the caller. Thats the only difference.

I am fine either way. Once the avg_bw change is removed, I can ack this.


This chunk looks better that way.

<skipping the rest as it LGTM>





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux