| 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
| ||
|
Date: Wed, 9 Feb 2022 17:22:27 -0800
From: Stephen Boyd <swboyd@...omium.org>
To: Sankeerth Billakanti <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@...cinc.com, quic_abhinavk@...cinc.com,
quic_khsieh@...cinc.com, quic_mkrishn@...cinc.com,
quic_vproddut@...cinc.com,
Dmitry Baryshkov <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