On 2024/12/6 16:51, Dmitry Baryshkov wrote:
> On Thu, Dec 05, 2024 at 08:31:35PM -0800, Abhinav Kumar wrote:
>> In preparation of DP MST where link caps are read for the
>> immediate downstream device and the edid is read through
>> sideband messaging, split the msm_dp_panel_read_sink_caps() into
>> two parts which read the link parameters and the edid parts
>> respectively.
>
> As you are touching this part, could you please refactor the code
> instead by dropping the msm_dp_panel->drm_edid? There should be no need
> to store EDID in the panel structure.
>
Hi, Dmitry, Abhinav will be leaving the company and will no longer be
responsible for updating and address the comments. I will take over
handling MST patch series. Regarding this comments, I don't got that
where the drm_edid should be stored. In MST cases, where multiple panels
exist, i think that there should be a separate drm_edid saved for each
panel.
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> ---
>> drivers/gpu/drm/msm/dp/dp_display.c | 6 +++++-
>> drivers/gpu/drm/msm/dp/dp_panel.c | 25 +++++++++++++++++--------
>> drivers/gpu/drm/msm/dp/dp_panel.h | 5 ++++-
>> 3 files changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index be26064af9febf4f4761e21ea7db85ab1ac66081..052db80c6a365f53c2c0a37d3b69ea2b627aea1f 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -372,7 +372,11 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
>> const struct drm_display_info *info = &connector->display_info;
>> int rc = 0;
>>
>> - rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
>> + rc = msm_dp_panel_read_link_caps(dp->panel, connector);
>> + if (rc)
>> + goto end;
>> +
>> + rc = msm_dp_panel_read_edid(dp->panel, connector);
>> if (rc)
>> goto end;
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
>> index 5d7eaa31bf3176566f40f01ff636bee64e81c64f..d277e9b2cbc03688976b6aa481ee724b186bab51 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>> @@ -108,8 +108,8 @@ static u32 msm_dp_panel_get_supported_bpp(struct msm_dp_panel *msm_dp_panel,
>> return min_supported_bpp;
>> }
>>
>> -int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
>> - struct drm_connector *connector)
>> +int msm_dp_panel_read_link_caps(struct msm_dp_panel *msm_dp_panel,
>> + struct drm_connector *connector)
>
> This function doesn't require connector anymore.
Right, will remove connector in function.
>
>> {
>> int rc, bw_code;
>> int count;
>> @@ -150,8 +150,19 @@ int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
>>
>> rc = drm_dp_read_downstream_info(panel->aux, msm_dp_panel->dpcd,
>> msm_dp_panel->downstream_ports);
>> - if (rc)
>> - return rc;
>> + return rc;
>> +}
>> +
>> +int msm_dp_panel_read_edid(struct msm_dp_panel *msm_dp_panel, struct drm_connector *connector)
>> +{
>> + struct msm_dp_panel_private *panel;
>> +
>> + if (!msm_dp_panel || !connector) {
>> + DRM_ERROR("invalid input\n");
>> + return -EINVAL;
>> + }
>
> Neither panel nor connector can be NULL here, please drop.
>
>> +
>> + panel = container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
>>
>> drm_edid_free(msm_dp_panel->drm_edid);
>>
>> @@ -163,13 +174,11 @@ int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
>> DRM_ERROR("panel edid read failed\n");
>> /* check edid read fail is due to unplug */
>> if (!msm_dp_catalog_link_is_connected(panel->catalog)) {
>> - rc = -ETIMEDOUT;
>> - goto end;
>> + return -ETIMEDOUT;
>> }
>> }
>>
>> -end:
>> - return rc;
>> + return 0;
>> }
>>
>> u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel,
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
>> index 0e944db3adf2f187f313664fe80cf540ec7a19f2..7a38655c443af597c84fb78c6702b2a3ef9822ed 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
>> @@ -59,7 +59,10 @@ void msm_dp_panel_dump_regs(struct msm_dp_panel *msm_dp_panel);
>> int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
>> struct drm_connector *connector);
>> u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel, u32 mode_max_bpp,
>> - u32 mode_pclk_khz);
>> + u32 mode_pclk_khz);
>> +int msm_dp_panel_read_link_caps(struct msm_dp_panel *dp_panel,
>> + struct drm_connector *connector);
>> +int msm_dp_panel_read_edid(struct msm_dp_panel *dp_panel, struct drm_connector *connector);
>> int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
>> struct drm_connector *connector);
>> void msm_dp_panel_handle_sink_request(struct msm_dp_panel *msm_dp_panel);
>>
>> --
>> 2.34.1
>>
>