linux-kernel - Re: [PATCH v4 1/2] drm/msm/dpu: retrieve DSI DSC struct through priv->dsi[0]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <add6f941-284e-7c38-b536-14541e518bcc@quicinc.com>
Date:   Thu, 22 Jun 2023 11:32:50 -0700
From:   Abhinav Kumar <quic_abhinavk@...cinc.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Kuogee Hsieh <quic_khsieh@...cinc.com>,
        <dri-devel@...ts.freedesktop.org>, <robdclark@...il.com>,
        <sean@...rly.run>, <swboyd@...omium.org>, <dianders@...omium.org>,
        <vkoul@...nel.org>, <daniel@...ll.ch>, <airlied@...il.com>,
        <agross@...nel.org>, <andersson@...nel.org>
CC:     <quic_jesszhan@...cinc.com>, <quic_sbillaka@...cinc.com>,
        <marijn.suijten@...ainline.org>, <freedreno@...ts.freedesktop.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/2] drm/msm/dpu: retrieve DSI DSC struct through
 priv->dsi[0]



On 6/22/2023 7:00 AM, Dmitry Baryshkov wrote:
> On 21/06/2023 19:18, Kuogee Hsieh wrote:
>> Currently DSI DSC struct is populated at display setup during
>> system bootup. This mechanism works fine with embedded display
>> but not for pluggable displays as the DSC struct will become
>> stale once external display unplugged.
>>
>> Move storing of DSI DSC struct to both atomic_mode_set() and
>> atomic_enable() so that same mechanism will work for both
>> embedded display and pluggable displays.
>>
>> Changes in v4:
>> -- fix checkpatch.pl warning
>>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 30 
>> +++++++++++++++++++++++++----
>>   1 file changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 2e1873d..367d374 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -543,11 +543,24 @@ bool dpu_encoder_use_dsc_merge(struct 
>> drm_encoder *drm_enc)
>>       return (num_dsc > 0) && (num_dsc > intf_count);
>>   }
>> +static struct drm_dsc_config *dpu_encoder_get_dsc_config(struct 
>> drm_encoder *drm_enc)
>> +{
>> +    struct msm_drm_private *priv = drm_enc->dev->dev_private;
>> +    struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
>> +    int index = dpu_enc->disp_info.h_tile_instance[0];
>> +
>> +    if (dpu_enc->disp_info.intf_type == INTF_DSI)
>> +        return msm_dsi_get_dsc_config(priv->dsi[index]);
>> +
>> +    return NULL;
>> +}
>> +
>>   static struct msm_display_topology dpu_encoder_get_topology(
>>               struct dpu_encoder_virt *dpu_enc,
>>               struct dpu_kms *dpu_kms,
>>               struct drm_display_mode *mode,
>> -            struct drm_crtc_state *crtc_state)
>> +            struct drm_crtc_state *crtc_state,
>> +            struct drm_dsc_config *dsc)
>>   {
>>       struct msm_display_topology topology = {0};
>>       int i, intf_count = 0;
>> @@ -579,7 +592,7 @@ static struct msm_display_topology 
>> dpu_encoder_get_topology(
>>       topology.num_intf = intf_count;
>> -    if (dpu_enc->dsc) {
>> +    if (dsc) {
>>           /*
>>            * In case of Display Stream Compression (DSC), we would use
>>            * 2 DSC encoders, 2 layer mixers and 1 interface
>> @@ -605,6 +618,7 @@ static int dpu_encoder_virt_atomic_check(
>>       struct drm_display_mode *adj_mode;
>>       struct msm_display_topology topology;
>>       struct dpu_global_state *global_state;
>> +    struct drm_dsc_config *dsc;
>>       int i = 0;
>>       int ret = 0;
>> @@ -640,7 +654,9 @@ static int dpu_encoder_virt_atomic_check(
>>           }
>>       }
>> -    topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, 
>> crtc_state);
>> +    dsc = dpu_encoder_get_dsc_config(drm_enc);
>> +
>> +    topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, 
>> crtc_state, dsc);
>>       /*
>>        * Release and Allocate resources on every modeset
>> @@ -1056,6 +1072,8 @@ static void 
>> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>>       trace_dpu_enc_mode_set(DRMID(drm_enc));
>> +    dpu_enc->dsc = dpu_encoder_get_dsc_config(drm_enc);
>> +
>>       /* Query resource that have been reserved in atomic check step. */
>>       num_pp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>>           drm_enc->base.id, DPU_HW_BLK_PINGPONG, hw_pp,
>> @@ -1187,6 +1205,8 @@ static void 
>> dpu_encoder_virt_atomic_enable(struct drm_encoder *drm_enc,
>>       dpu_enc = to_dpu_encoder_virt(drm_enc);
>> +    dpu_enc->dsc = dpu_encoder_get_dsc_config(drm_enc);
> 
> Let's have a single place where dpu_enc->dsc is set. I have slight 
> preference for atomic_enable, but atomic_mode_set is fine too.
> 

Yes, it was in a single place in v3. Then some mistake happened and it 
got dropped in v4. Now, it has been added back for v5.

Sorry for the confusion.