[PATCH 2/2] drm/msm/dsi: implement opp table based check for dsi_mgr_bridge_mode_valid()

[Dmitry Baryshkov]

On 27/10/2022 20:36, Dmitry Baryshkov wrote:
> On 22/09/2022 03:49, Abhinav Kumar wrote:
>> Currently there is no protection against a user trying to set
>> an unsupported mode on DSI. Implement a check based on the opp
>> table whether the byte clock for the mode can be supported by
>> validating whether an opp table entry exists.
>>
>> For devices which have not added opp table support yet, skip
>> this check otherwise it will break bootup on those devices.
>>
>> Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/15
>> Reported-by: Rob Clark <robdclark at gmail.com>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/dsi/dsi_manager.c | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c 
>> b/drivers/gpu/drm/msm/dsi/dsi_manager.c
>> index 3a1417397283..87b518c42965 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
>> @@ -450,6 +450,29 @@ static enum drm_mode_status 
>> dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
>>       int id = dsi_mgr_bridge_get_id(bridge);
>>       struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
>>       struct mipi_dsi_host *host = msm_dsi->host;
>> +    struct platform_device *pdev = msm_dsi->pdev;
>> +    struct dev_pm_opp *opp;
>> +    struct opp_table *opp_tbl;
>> +    unsigned long byte_clk_rate;
>> +
>> +    byte_clk_rate = dsi_byte_clk_get_rate(host, IS_BONDED_DSI(), mode);
>> +
>> +    /*
>> +     * first check if there is an opp table available for the calculated
>> +     * byte clock and then check DSC related info. Some devices have not
>> +     * added support for OPP table. Skip the check for those.
>> +     */
>> +    opp_tbl = dev_pm_opp_get_opp_table(&pdev->dev);
> 
> Can we store the table inside the msm_dsi during the init? Then we won't 
> have to get it again and again during each mode_valid call.

I checked other drivers. I think we can skip the get_opp_table 
completely, can we not? Just handle ENODEV returned from 
dev_pm_opp_find_freq_ceil().

> 
>> +    if (opp_tbl) {
>> +        opp = dev_pm_opp_find_freq_ceil(&pdev->dev, &byte_clk_rate);
>> +        if (IS_ERR(opp)) {
>> +            pr_err("opp table not found for freq %lu err: %ld\n",
>> +                    byte_clk_rate, PTR_ERR(opp));
>> +            return PTR_ERR(opp);
>> +        }
>> +        dev_pm_opp_put(opp);
>> +        dev_pm_opp_put_opp_table(opp_tbl);
>> +    }
>>       return msm_dsi_host_check_dsc(host, mode);
>>   }
> 

-- 
With best wishes
Dmitry