On 5/3/2023 12:51 PM, Dmitry Baryshkov wrote:
On 03/05/2023 22:04, Jessica Zhang wrote:
On 5/3/2023 12:28 AM, Marijn Suijten wrote:
On 2023-05-02 18:19:15, Jessica Zhang wrote:
Add a dpu_hw_intf op to enable data compression.
Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 ++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 7 +++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 ++
3 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 74470d068622..4321a1aba17f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
Can we have INTF DCE on video-mode encoders as well?
Hi Marijn,
Currently, there's no way to validate DSC for video mode as I've only
made changes to support DSI for command mode. We are planning to post
changes to support DSC over DP, which will include changes for video
mode.
If I remember correctly, HDK8350 panel should support DSC for both
command and video modes.
Hi Dmitry,
Correct, however we are planning to submit the video mode changes with
the DP DSC v1.2 changes.
My current panel driver/dt changes are for command mode, so we would
have to spent time to also add video mode support. It would be faster to
land the video mode changes with DP support as that's already a work in
progress.
@@ -72,6 +72,10 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
phys_enc->hw_intf,
true,
phys_enc->hw_pp->idx);
+
+ if (phys_enc->dpu_kms->catalog->caps->has_data_compress &&
As per my suggestion on patch 3/4, drop the flag and check above and
only check if the function is NULL (below).
Acked.
+ phys_enc->hw_intf->ops.enable_compression)
+ phys_enc->hw_intf->ops.enable_compression(phys_enc->hw_intf);
}
static void dpu_encoder_phys_cmd_pp_tx_done_irq(void *arg, int
irq_idx)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 671048a78801..4ce7ffdd7a05 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -64,10 +64,16 @@
#define INTF_CFG2_DATABUS_WIDEN BIT(0)
#define INTF_CFG2_DATA_HCTL_EN BIT(4)
These should probably be reindented to match the below... And the rest
of the defines use spaces instead of tabs.
Fair point, though I think fixing the whitespace for these 2 macros
specifically might be better in a more relevant series.
With that being said, I'll change the spacing of the DATA_COMPRESS bit
to spaces instead of tabs.
+#define INTF_CFG2_DCE_DATA_COMPRESS BIT(12)
#define INTF_MISR_CTRL 0x180
#define INTF_MISR_SIGNATURE 0x184
This does not seem to apply on top of:
https://lore.kernel.org/linux-arm-msm/20230411-dpu-intf-te-v4-10-27ce1a5ab5c6@xxxxxxxxxxxxxx/
Seems like I'm missing some patches from that series on my working
branch. Will rebase on top of the full series for the v2.
+static inline void dpu_hw_intf_enable_compression(struct
dpu_hw_intf *ctx)
Why inline? This is used as a pointer callback.
Acked, will remove the inline.
+{
+ DPU_REG_WRITE(&ctx->hw, INTF_CONFIG2,
INTF_CFG2_DCE_DATA_COMPRESS);
dpu_hw_intf_setup_timing_engine() also programs INTF_CONFIG2. Is it
double-buffered, or is that config **always** unused when DSI CMD mode
is used in conjunction with DSC/DCE? Otherwise this should perhaps OR
the bitflag into the register, or write the whole thing at once in
dpu_hw_intf_setup_timing_engine()?
For command mode, INTF_CONFIG2 is unused aside from setting
DATA_COMPRESS for DSC.
Since setup_timing_engine() is only used for video mode, the
corresponding changes will be made in the DSC v1.2 for DP changes.
So, for command mode panels is this the only bit that should be set in
INTF_CFG2?
Yep, outside of the changes in this patch, INTF_CONFIG2 is only used in
the video mode setup_timing_engine() method.
Thanks,
Jessica Zhang
--
With best wishes
Dmitry
