lkml.org 
[lkml]   [2023]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 4/4] drm/msm/dpu: Enable compression for command mode
From
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

\
 
 \ /
  Last update: 2023-05-03 21:51    [W:0.106 / U:4.904 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site