Messages in this thread | | | Date | Wed, 3 May 2023 16:24:07 -0700 | Subject | Re: [PATCH 3/4] drm/msm/dpu: Add has_data_compress to dpu_caps | From | Jessica Zhang <> |
| |
On 5/3/2023 4:03 PM, Marijn Suijten wrote: > Hi Jessica, > > On 2023-05-03 12:03:40, Jessica Zhang wrote: >> >> >> On 5/3/2023 12:07 AM, Marijn Suijten wrote: >>> On 2023-05-02 18:19:14, Jessica Zhang wrote: >>>> Add data_compress feature to DPU HW catalog. >>>> >>>> In DPU 7.x and later, there is a DATA_COMPRESS register that must be set >>>> within the DPU INTF block for DSC to work. >>>> >>>> As core_rev (and related macros) was removed from the dpu_kms struct, the >>>> most straightforward way to indicate the presence of this register would be >>>> to have a flag in dpu_caps. >>> >>> This is a very generic name to have in the global dpu_caps for a very >>> specific register on the INTF block since DPU >= 7.0.0, and I doubt any >>> new catalog contributor will know how to fill this field. After all, >>> DPU < 7.0.0 also has DCE but it is controlled via the PINGPONG block. >>> >>> Instead, how about having it as a DPU_INTF_DATA_COMPRESS (or similar) >>> feature flag on the INTF block? We do the same for other (register >>> related) features on the INTF block, and you did the same to disable DSC >>> callbacks on PP in [1]. > > (Note: I said "you" but meant Kuogee) > >> Hi Marijn, >> >> Sounds good. >> >>> >>> In fact it seems that the DSC/DCE (enablement) registers have been moved >>> from PINGPONG to INTF in DPU 7.0.0. Can you clarify in the patch >>> message for v2 that this is the case, and do the same in the linked >>> PINGPONG patch? Perhaps these patches should be part of the same series >>> as they do not seem DSI-specific. >> >> Will make a note of the PP to INTF change in the commit message. > > Thanks. > >> I would prefer to keep this patch in this series is because it is needed >> for DSI over command mode to work and the subsequent patch is >> specifically for command mode. > > That is fine, but do mention this in the commit message if it is > relevant here. Otherwise only mention it as part of patch 4/4.
Acked.
Thanks,
Jessica Zhang
> > - Marijn
| |