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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 24 Jan 2023 00:17:46 +0200
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Kuogee Hsieh <quic_khsieh@...cinc.com>,
        dri-devel@...ts.freedesktop.org, robdclark@...il.com,
        sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
        vkoul@...nel.org, daniel@...ll.ch, airlied@...il.com,
        agross@...nel.org, andersson@...nel.org
Cc:     quic_abhinavk@...cinc.com, quic_sbillaka@...cinc.com,
        freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 01/14] drm/msm/dp: add dpcd read of both dsc and fec
 capability

On 23/01/2023 20:24, Kuogee Hsieh wrote:
> FEC is pre-requirement of DSC. Therefore FEC has to be enabled
> before DSC enabled. This patch add functions to read sink's DSC
> and FEC related DPCD and decode them and set enable flags
> accordingly.

Please split this to FEC and DSC patches.

> 
> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 91 ++++++++++++++++++++++++++++++++++++++-
>   drivers/gpu/drm/msm/dp/dp_panel.h | 20 ++++++++-
>   2 files changed, 109 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 1800d89..5078247 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -1,14 +1,18 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /*
> - * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2012-2023, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023. Qualcomm Innovation Center, Inc. All rights reserved
>    */
>   
>   #include "dp_panel.h"
>   
> +#include <drm/drm_fixed.h>
>   #include <drm/drm_connector.h>
>   #include <drm/drm_edid.h>
>   #include <drm/drm_print.h>
>   
> +#define DSC_TGT_BPP 8
> +
>   struct dp_panel_private {
>   	struct device *dev;
>   	struct drm_device *drm_dev;
> @@ -68,6 +72,9 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>   		goto end;
>   	}
>   
> +	print_hex_dump_debug("[drm-dp] SINK DPCD: ",
> +		DUMP_PREFIX_NONE, 8, 1, dp_panel->dpcd, rlen, false);

I think this can go away.

> +
>   	link_info->revision = dpcd[DP_DPCD_REV];
>   	major = (link_info->revision >> 4) & 0x0f;
>   	minor = link_info->revision & 0x0f;
> @@ -154,6 +161,83 @@ static int dp_panel_update_modes(struct drm_connector *connector,
>   	return rc;
>   }
>   
> +static void dp_panel_decode_dsc_dpcd(struct dp_panel *dp_panel)
> +{
> +	if (dp_panel->dsc_dpcd[0]) {
> +		dp_panel->sink_dsc_caps.dsc_capable = true;
> +		dp_panel->sink_dsc_caps.version = dp_panel->dsc_dpcd[1];
> +		dp_panel->sink_dsc_caps.block_pred_en =
> +				dp_panel->dsc_dpcd[6] ? true : false;
> +		dp_panel->sink_dsc_caps.color_depth =
> +				dp_panel->dsc_dpcd[10];
> +
> +		if (dp_panel->sink_dsc_caps.version >= 0x11)
> +			dp_panel->dsc_en = true;
> +	} else {
> +		dp_panel->sink_dsc_caps.dsc_capable = false;
> +		dp_panel->dsc_en = false;
> +	}
> +}
> +
> +static void dp_panel_read_sink_dsc_caps(struct dp_panel *dp_panel)
> +{
> +	int rlen;
> +	struct dp_panel_private *panel;
> +	int dpcd_rev;
> +
> +	if (!dp_panel) {
> +		DRM_ERROR("invalid input\n");
> +		return;
> +	}
> +
> +	dpcd_rev = dp_panel->dpcd[DP_DPCD_REV];
> +
> +	panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
> +	if (dpcd_rev >= 0x14) {
> +		rlen = drm_dp_dpcd_read(panel->aux, DP_DSC_SUPPORT,
> +			dp_panel->dsc_dpcd, (DP_DSC_RECEIVER_CAP_SIZE + 1));
> +		if (rlen < (DP_DSC_RECEIVER_CAP_SIZE + 1)) {
> +			drm_dbg_dp(panel->drm_dev, "dsc dpcd read failed, rlen=%d\n", rlen);
> +			return;
> +		}
> +
> +		print_hex_dump_debug("[drm-dp] SINK DSC DPCD: ",
> +			DUMP_PREFIX_NONE, 8, 1, dp_panel->dsc_dpcd, rlen, false);

Oh. An extra dump again. Can we get rid of them please? Or can we use 
the standard drm debug levels? So that it would be possible to turn them 
on and off with standard params.

> +
> +		dp_panel_decode_dsc_dpcd(dp_panel);
> +	}
> +}
> +
> +static void dp_panel_read_sink_fec_caps(struct dp_panel *dp_panel)
> +{
> +	int rlen;
> +	struct dp_panel_private *panel;
> +	s64 fec_overhead_fp = drm_fixp_from_fraction(1, 1);
> +
> +	if (!dp_panel) {
> +		DRM_ERROR("invalid input\n");
> +		return;
> +	}
> +
> +	panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
> +	rlen = drm_dp_dpcd_readb(panel->aux, DP_FEC_CAPABILITY,
> +			&dp_panel->fec_dpcd);
> +	if (rlen < 1) {
> +		DRM_ERROR("fec capability read failed, rlen=%d\n", rlen);
> +		return;
> +	}
> +
> +	print_hex_dump_debug("[drm-dp] SINK FEC DPCD: ",
> +		DUMP_PREFIX_NONE, 8, 1, &dp_panel->fec_dpcd, rlen, false);
> +
> +	dp_panel->fec_en = dp_panel->fec_dpcd & DP_FEC_CAPABLE;
> +
> +	if (dp_panel->fec_en)
> +		fec_overhead_fp = drm_fixp_from_fraction(100000, 97582);
> +
> +	dp_panel->fec_overhead_fp = fec_overhead_fp;
> +}
> +
>   int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
>   	struct drm_connector *connector)
>   {
> @@ -224,6 +308,11 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
>   		}
>   		panel->aux_cfg_update_done = false;
>   	}
> +
> +	dp_panel_read_sink_fec_caps(dp_panel);
> +
> +	if (dp_panel->fec_en)
> +		dp_panel_read_sink_dsc_caps(dp_panel);
>   end:
>   	return rc;
>   }
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
> index f04d021..fb30b92 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -1,6 +1,7 @@
>   /* SPDX-License-Identifier: GPL-2.0-only */
>   /*
> - * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2012-2023, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023. Qualcomm Innovation Center, Inc. All rights reserved
>    */
>   
>   #ifndef _DP_PANEL_H_
> @@ -11,6 +12,7 @@
>   #include "dp_aux.h"
>   #include "dp_link.h"
>   #include "dp_hpd.h"
> +#include "msm_drv.h"
>   
>   struct edid;
>   
> @@ -34,12 +36,21 @@ struct dp_panel_in {
>   	struct dp_catalog *catalog;
>   };
>   
> +struct dp_dsc_caps {
> +	bool dsc_capable;
> +	u8 version;
> +	bool block_pred_en;
> +	u8 color_depth;
> +};
> +
>   struct dp_panel {
>   	/* dpcd raw data */
>   	u8 dpcd[DP_RECEIVER_CAP_SIZE + 1];
>   	u8 ds_cap_info[DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE];
>   	u32 ds_port_cnt;
>   	u32 dfp_present;
> +	u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE + 1];
> +	u8 fec_dpcd;
>   
>   	struct dp_link_info link_info;
>   	struct drm_dp_desc desc;
> @@ -53,6 +64,13 @@ struct dp_panel {
>   	u32 max_dp_link_rate;
>   
>   	u32 max_bw_code;
> +
> +	struct dp_dsc_caps sink_dsc_caps;

Why do you need both decoded and raw DSC caps?

> +	bool dsc_feature_enable;
> +	bool fec_feature_enable;
> +	bool dsc_en;
> +	bool fec_en;
> +	s64 fec_overhead_fp;
>   };
>   
>   int dp_panel_init_panel_info(struct dp_panel *dp_panel);

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ