Messages in this thread | | | Date | Wed, 3 May 2023 22:51:07 +0300 | Subject | Re: [PATCH 4/4] drm/msm/dpu: Enable compression for command mode | From | Dmitry Baryshkov <> |
| |
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@quicinc.com> >>> --- >>> 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.
> >> >>> @@ -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@somainline.org/ > > 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? -- With best wishes Dmitry
| |