On 5/8/2023 4:17 PM, Jessica Zhang wrote:
On 5/7/2023 9:06 AM, Marijn Suijten wrote:
On 2023-05-05 14:23:51, Jessica Zhang wrote:
Add a DPU INTF op to set DATA_COMPRESS register for command mode
panels if
the DPU_INTF_DATA_COMPRESS feature flag is set. This flag needs to be
enabled in order for DSC v1.2 to work.
Note: These changes are for command mode only. Video mode changes will
be posted along with the DSC v1.2 support for DP.
Nit: the "command mode" parts of both paragraphs only apply to the call
in dpu_encoder_phys_cmd, right? If so, and the INTF op remains the same
for video mode (but only the call needs to be added to the
dpu_encoder_phy_vid), make this a bit more clear in your commit message.
(Sorry, forgot to address this comment in my initial reply)
The op will be available for video mode to use, but most likely video
mode will set DATA_COMPRESS (or call dpu_hw_intf_enable_compression())
directly in dpu_hw_intf_setup_timing_engine().
Thanks,
Jessica Zhang
Changes in v2:
- Fixed whitespace issue in macro definition
- Read INTF_CONFIG2 before writing to DATA_COMPRESS bit
- Only set dpu_hw_intf_ops.data_compress if DATA_COMPRESS feature is set
- Removed `inline` from dpu_hw_intf_enable_compression declaration
Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 3 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 11 +++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 ++
3 files changed, 16 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 d8ed85a238af..1a4c20f02312 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
@@ -68,6 +68,9 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
phys_enc->hw_intf,
true,
phys_enc->hw_pp->idx);
+
+ if (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 6485500eedb8..322c55a5042c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -91,6 +91,14 @@
#define INTF_CFG2_DATABUS_WIDEN BIT(0)
#define INTF_CFG2_DATA_HCTL_EN BIT(4)
+#define INTF_CFG2_DCE_DATA_COMPRESS BIT(12)
+
+static void dpu_hw_intf_enable_compression(struct dpu_hw_intf *ctx)
+{
+ u32 intf_cfg2 = DPU_REG_READ(&ctx->hw, INTF_CONFIG2);
+
+ DPU_REG_WRITE(&ctx->hw, INTF_CONFIG2, intf_cfg2 |
INTF_CFG2_DCE_DATA_COMPRESS);
I'm not sure if it's more idiomatic to write:
intf_cfg2 |= INTF_CFG2_DCE_DATA_COMPRESS;
On a separate line.
Hi Marijn,
Sounds good.
+}
Move the function close to the bottom of this file. Right now all the
functions are defined approximately in the same order as they're listed
in the header and assigned in _setup_intf_ops().
Acked.
static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
const struct intf_timing_params *p,
@@ -542,6 +550,9 @@ static void _setup_intf_ops(struct
dpu_hw_intf_ops *ops,
ops->vsync_sel = dpu_hw_intf_vsync_sel;
ops->disable_autorefresh = dpu_hw_intf_disable_autorefresh;
}
+
+ if (cap & BIT(DPU_INTF_DATA_COMPRESS))
+ ops->enable_compression = dpu_hw_intf_enable_compression;
}
struct dpu_hw_intf *dpu_hw_intf_init(const struct dpu_intf_cfg *cfg,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index 73b0885918f8..a8def68a5ec2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -70,6 +70,7 @@ struct intf_status {
* @get_autorefresh: Retrieve autorefresh config from
hardware
* Return: 0 on success, -ETIMEDOUT on
timeout
* @vsync_sel: Select vsync signal for tear-effect
configuration
+ * @enable_compression: Enable data compression
Indent to match above.
Sure, is the plan to correct the whitespace in the first half of the
comment block in the future?
Thanks,
Jessica Zhang
- Marijn
*/
struct dpu_hw_intf_ops {
void (*setup_timing_gen)(struct dpu_hw_intf *intf,
@@ -107,6 +108,7 @@ struct dpu_hw_intf_ops {
* Disable autorefresh if enabled
*/
void (*disable_autorefresh)(struct dpu_hw_intf *intf, uint32_t
encoder_id, u16 vdisplay);
+ void (*enable_compression)(struct dpu_hw_intf *intf);
};
struct dpu_hw_intf {
--
2.40.1
