[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2a54ffe8-8e40-49f6-8735-96da47e1bbc6@quicinc.com>
Date: Thu, 22 May 2025 17:51:52 +0800
From: Yongxing Mou <quic_yongmou@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Abhinav Kumar
<quic_abhinavk@...cinc.com>
CC: Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
"Marijn
Suijten" <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Stephen Boyd <swboyd@...omium.org>,
"Chandan
Uddaraju" <chandanu@...eaurora.org>,
Guenter Roeck <groeck@...omium.org>,
Kuogee Hsieh <quic_khsieh@...cinc.com>,
Bjorn Andersson
<andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring
<robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Vara Reddy <quic_varar@...cinc.com>,
Rob Clark
<robdclark@...omium.org>,
Tanmay Shah <tanmay@...eaurora.org>, <linux-arm-msm@...r.kernel.org>,
<dri-devel@...ts.freedesktop.org>, <freedreno@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
Jessica Zhang <quic_jesszhan@...cinc.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH 04/45] drm/msm/dp: split msm_dp_panel_read_sink_caps()
into two parts
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@...cinc.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
>>
>
Powered by blists - more mailing lists