| 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
| ||
|
Message-ID: <MW4PR02MB7186BB7DE94099078930E282E12F9@MW4PR02MB7186.namprd02.prod.outlook.com>
Date: Thu, 10 Feb 2022 11:57:34 +0000
From: "Sankeerth Billakanti (QUIC)" <quic_sbillaka@...cinc.com>
To: Stephen Boyd <swboyd@...omium.org>,
"Sankeerth Billakanti (QUIC)" <quic_sbillaka@...cinc.com>,
"agross@...nel.org" <agross@...nel.org>,
"airlied@...ux.ie" <airlied@...ux.ie>,
"bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
"daniel@...ll.ch" <daniel@...ll.ch>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"dianders@...omium.org" <dianders@...omium.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"freedreno@...ts.freedesktop.org" <freedreno@...ts.freedesktop.org>,
"krzysztof.kozlowski@...onical.com"
<krzysztof.kozlowski@...onical.com>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"robdclark@...il.com" <robdclark@...il.com>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"sam@...nborg.org" <sam@...nborg.org>,
"seanpaul@...omium.org" <seanpaul@...omium.org>,
"thierry.reding@...il.com" <thierry.reding@...il.com>
CC: quic_kalyant <quic_kalyant@...cinc.com>,
"Abhinav Kumar (QUIC)" <quic_abhinavk@...cinc.com>,
"Kuogee Hsieh (QUIC)" <quic_khsieh@...cinc.com>,
quic_mkrishn <quic_mkrishn@...cinc.com>,
quic_vproddut <quic_vproddut@...cinc.com>,
"dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>
Subject: RE: [PATCH v3 4/4] drm/msm/dp: Add driver support to utilize drm
panel
Hi Stephen,
Will make the changes.
-----Original Message-----
From: Stephen Boyd <swboyd@...omium.org>
Sent: Thursday, February 10, 2022 6:52 AM
To: Sankeerth Billakanti (QUIC) <quic_sbillaka@...cinc.com>; agross@...nel.org; airlied@...ux.ie; bjorn.andersson@...aro.org; daniel@...ll.ch; devicetree@...r.kernel.org; dianders@...omium.org; dri-devel@...ts.freedesktop.org; freedreno@...ts.freedesktop.org; krzysztof.kozlowski@...onical.com; linux-arm-msm@...r.kernel.org; linux-kernel@...r.kernel.org; robdclark@...il.com; robh+dt@...nel.org; sam@...nborg.org; seanpaul@...omium.org; thierry.reding@...il.com
Cc: quic_kalyant <quic_kalyant@...cinc.com>; Abhinav Kumar (QUIC) <quic_abhinavk@...cinc.com>; Kuogee Hsieh (QUIC) <quic_khsieh@...cinc.com>; quic_mkrishn <quic_mkrishn@...cinc.com>; quic_vproddut <quic_vproddut@...cinc.com>; dmitry.baryshkov@...aro.org
Subject: Re: [PATCH v3 4/4] drm/msm/dp: Add driver support to utilize drm panel
Quoting Sankeerth Billakanti (2022-02-09 00:55:32)
> Add support in the DP driver to utilize the custom eDP panels from
> drm/panels.
>
> An eDP panel is always connected to the platform. So, the eDP
> connector can be reported as always connected. The display mode will
> be sourced from the panel. The panel mode will be set after the link
> training is completed.
>
> Signed-off-by: Sankeerth Billakanti <quic_sbillaka@...cinc.com>
> ---
>
> Changes in v3:
> None
>
> drivers/gpu/drm/msm/dp/dp_display.c | 8 ++++++
> drivers/gpu/drm/msm/dp/dp_drm.c | 54 +++++++++++++++++++++++++++++++++----
> drivers/gpu/drm/msm/dp/dp_parser.h | 3 +++
> 3 files changed, 60 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 7cc4d21..410fda4 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1513,6 +1513,10 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
> return -EINVAL;
> }
>
> + /* handle eDP on */
This comment is obvious. Please remove.
> + if (dp->connector_type == DRM_MODE_CONNECTOR_eDP)
> + dp_hpd_plug_handle(dp_display, 0);
> +
> mutex_lock(&dp_display->event_mutex);
>
> /* stop sentinel checking */
> @@ -1577,6 +1581,10 @@ int msm_dp_display_disable(struct msm_dp *dp,
> struct drm_encoder *encoder)
>
> dp_display = container_of(dp, struct dp_display_private,
> dp_display);
>
> + /* handle edp off */
This comment is obvious. Please remove.
> + if (dp->connector_type == DRM_MODE_CONNECTOR_eDP)
> + dp_hpd_unplug_handle(dp_display, 0);
> +
> mutex_lock(&dp_display->event_mutex);
>
> /* stop sentinel checking */
> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c
> b/drivers/gpu/drm/msm/dp/dp_drm.c index d4d360d..12fa8c1 100644
> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> @@ -39,6 +39,10 @@ static enum drm_connector_status
> dp_connector_detect(struct drm_connector *conn,
>
> dp = to_dp_connector(conn)->dp_display;
>
> + /* eDP is always connected */
> + if (dp->connector_type == DRM_MODE_CONNECTOR_eDP)
> + return connector_status_connected;
Why not implement different connector ops for eDP and then not implement this function at all in that case?
> +
> DRM_DEBUG_DP("is_connected = %s\n",
> (dp->is_connected) ? "true" : "false");
>
> @@ -123,6 +127,35 @@ static enum drm_mode_status dp_connector_mode_valid(
> return dp_display_validate_mode(dp_disp, mode->clock); }
>
> +static int edp_connector_get_modes(struct drm_connector *connector) {
> + struct msm_dp *dp;
> +
> + if (!connector)
Is this check really necessary? Why doesn't drm do it in higher layers?
> + return 0;
> +
> + dp = to_dp_connector(connector)->dp_display;
> +
> + return drm_bridge_get_modes(dp->panel_bridge, connector); }
> +
> +static enum drm_mode_status edp_connector_mode_valid(
> + struct drm_connector *connector,
> + struct drm_display_mode *mode) {
> + struct msm_dp *dp;
> +
> + if (!connector)
Is this check really necessary? Why doesn't drm do it in higher layers?
> + return 0;
> +
> + dp = to_dp_connector(connector)->dp_display;
> +
> + if (mode->clock > EDP_MAX_PIXEL_CLK_KHZ)
> + return MODE_BAD;
Why not return MODE_CLOCK_HIGH?
> +
> + return MODE_OK;
> +}
> +
> static const struct drm_connector_funcs dp_connector_funcs = {
> .detect = dp_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> @@ -137,6 +170,12 @@ static const struct drm_connector_helper_funcs dp_connector_helper_funcs = {
> .mode_valid = dp_connector_mode_valid, };
>
> +static const struct drm_connector_helper_funcs edp_connector_helper_funcs = {
> + .get_modes = edp_connector_get_modes,
> + .mode_valid = edp_connector_mode_valid,
> +
Why the extra newline?
> +};
> +
> /* connector initialization */
> struct drm_connector *dp_drm_connector_init(struct msm_dp
> *dp_display) { @@ -160,12 +199,17 @@ struct drm_connector
> *dp_drm_connector_init(struct msm_dp *dp_display)
> if (ret)
> return ERR_PTR(ret);
>
> - drm_connector_helper_add(connector, &dp_connector_helper_funcs);
> + if (dp_display->connector_type == DRM_MODE_CONNECTOR_eDP) {
> + drm_connector_helper_add(connector,
> + &edp_connector_helper_funcs);
> + } else {
> + drm_connector_helper_add(connector,
> + &dp_connector_helper_funcs);
>
> - /*
> - * Enable HPD to let hpd event is handled when cable is connected.
> - */
> - connector->polled = DRM_CONNECTOR_POLL_HPD;
> + /*
> + * Enable HPD to let hpd event is handled when cable is connected.
> + */
> + connector->polled = DRM_CONNECTOR_POLL_HPD;
> + }
>
> drm_connector_attach_encoder(connector, dp_display->encoder);
>
Powered by blists - more mailing lists