On 27/07/2023 18:24, Abhinav Kumar wrote:
On 7/27/2023 1:39 AM, Dmitry Baryshkov wrote:
On Thu, 27 Jul 2023 at 02:20, Abhinav Kumar
<quic_abhinavk@xxxxxxxxxxx> wrote:
On 5/21/2023 10:10 AM, Dmitry Baryshkov wrote:
Switch to using data from MDSS driver to program the SSPP fetch and
UBWC
configuration.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 18 +++++++++++-------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 7 +++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 +++++++++++++++-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 3 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 ++
6 files changed, 36 insertions(+), 11 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 cf70a9bd1034..bfd82c2921af 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -8,6 +8,8 @@
#include "dpu_hw_sspp.h"
#include "dpu_kms.h"
+#include "msm_mdss.h"
+
#include <drm/drm_file.h>
#define DPU_FETCH_CONFIG_RESET_VALUE 0x00000087
@@ -308,26 +310,26 @@ static void dpu_hw_sspp_setup_format(struct
dpu_sw_pipe *pipe,
DPU_REG_WRITE(c, SSPP_FETCH_CONFIG,
DPU_FETCH_CONFIG_RESET_VALUE |
ctx->ubwc->highest_bank_bit << 18);
Does this needs to be protected with if ctx->ubwc check?
Yes... And it should have been already.
- switch (ctx->ubwc->ubwc_version) {
- case DPU_HW_UBWC_VER_10:
+ switch (ctx->ubwc->ubwc_enc_version) {
+ case UBWC_1_0:
The values of UBWC_x_x dont match the values of DPU_HW_UBWC_VER_xx.
What was the reason for the catalog to go with DPU_HW_UBWC_VER_xx in the
catalog for the encoder version in the first place? Because looking at
the registers UBWC_x_x is the correct value.
If we cannot find the reason why, it should be noted in the commit text
that the values we are using did change.
Huh? This is just an enum. It isn't a part of uABI, nor it is written
to the hardware.
The reason is that, this switch case is moving from comparing one set of
values to totally different ones. So atleast that should be noted.
First thing that struck me was this point because the enums UBWC_x_x and
DPU_HW_UBWC_VER_xx dont match.
Do you have any proposed text in mind?
--
With best wishes
Dmitry
