[PATCH] drm/amdgpu: add dummy read by engines for some GCVM status registers
From: changzhu
The GRBM register interface is now capable of bursting 1 cycle per
register wr->wr, wr->rd much faster than previous muticycle per
transaction done interface. This has caused a problem where
status registers requiring HW to update have a 1 cycle delay, due
to the register update having to go through GRBM.
For cp ucode, it has realized dummy read in cp firmware.It covers
the use of WAIT_REG_MEM operation 1 case only.So it needs to call
gfx_v10_0_wait_reg_mem in gfx10. Besides it also needs to add warning to
update firmware in case firmware is too old to have function to realize
dummy read in cp firmware.
For sdma ucode, it hasn't realized dummy read in sdma firmware. sdma is
moved to gfxhub in gfx10. So it needs to add dummy read in driver
between amdgpu_ring_emit_wreg and amdgpu_ring_emit_reg_wait for sdma_v5_0.
Change-Id: Ie028f37eb789966d4593984bd661b248ebeb1ac3
Signed-off-by: changzhu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 1 +
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 50 +
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 7
drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 8 ++--
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 13 ++-
5 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 459aa9059542..a74ecd449775 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -267,6 +267,7 @@ struct amdgpu_gfx {
uint32_tmec2_feature_version;
boolmec_fw_write_wait;
boolme_fw_write_wait;
+ boolcp_fw_write_wait;
struct amdgpu_ring gfx_ring[AMDGPU_MAX_GFX_RINGS];
unsignednum_gfx_rings;
struct amdgpu_ring compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 17a5cbfd0024..814764723c26 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -561,6 +561,32 @@ static void gfx_v10_0_free_microcode(struct amdgpu_device
*adev)
kfree(adev->gfx.rlc.register_list_format);
}
+static void gfx_v10_0_check_fw_write_wait(struct amdgpu_device *adev)
+{
+ adev->gfx.cp_fw_write_wait = false;
+
+ switch (adev->asic_type) {
+ case CHIP_NAVI10:
+ case CHIP_NAVI12:
+ case CHIP_NAVI14:
+ if ((adev->gfx.me_fw_version >= 0x0046) &&
+ (adev->gfx.me_feature_version >= 27) &&
+ (adev->gfx.pfp_fw_version >= 0x0068) &&
+ (adev->gfx.pfp_feature_version >= 27) &&
+ (adev->gfx.mec_fw_version >= 0x005b) &&
+ (adev->gfx.mec_feature_version >= 27))
+ adev->gfx.cp_fw_write_wait = true;
+ break;
+ default:
+ break;
+ }
+
+ if (adev->gfx.cp_fw_write_wait == false)
+ DRM_WARN_ONCE("Warning: check cp_fw_version and update it to
realize \
+ GRBM requires 1-cycle delay in cp
firmware\n");
+}
+
+
static void gfx_v10_0_init_rlc_ext_microcode(struct amdgpu_device *adev)
{
const struct rlc_firmware_header_v2_1 *rlc_hdr;
@@ -4768,6 +4794,28 @@ static void gfx_v10_0_ring_emit_reg_wait(struct
amdgpu_ring *ring, uint32_t reg,
gfx_v10_0_wait_reg_mem(ring, 0, 0, 0, reg, 0, val, mask, 0x20);
}
+static void gfx_v10_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,
+ uint32_t reg0, uint32_t reg1,
+ uint32_t ref, uint32_t mask)
+{
+ int usepfp = (ring->funcs->type == AMDGPU_RING_TYPE_GFX);
+ struct amdgpu_device *adev = ring->adev;
+ bool fw_version_ok = false;
+
+ gfx_v10_0_check_fw_write_wait(adev);
+
+ if (ring->funcs->type == AMDGPU_RING_TYPE_GFX ||
+ ring->funcs->type == AMDGPU_RING_TYPE_COMPUTE)
+ fw_version_ok = adev->gfx.cp_fw_write_wait;
+
+ if (fw_version_ok)
+ gfx_v10_0_wait_reg_mem(ring, usepfp, 0, 1, reg0, reg1,
+ ref, mask, 0x20);
+ else
+ amdgpu_ring_emit_reg_write_reg_wait_helper(ring, reg0, reg1,
+ ref, mask);
+}
+
static void
gfx_v10_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
uint32_t me, uint32_t pipe,
@@ -5158,6 +5206,7 @@ static const struct amdgpu_ring_funcs
gfx_v10_0_ring_funcs_gfx = {
.emit_tmz = gfx_v10_0_ring_emit_tmz,
.emit_wreg = gfx_v10_0_ring_emit_wreg,
.emit_reg_wait = gfx_v10_0_ring_emit_reg_wait,
+ .emit_reg_write_reg_wait =
[PATCH libdrm] tests/amdgpu: enable dispatch/draw tests for Renoir
From: changzhu
It can run dispatch/draw tests on new renoir chips. So it needs to
enable dispatch/draw tests for Renoir again.
Change-Id: I3a72a4bbfe0fc663ee0e3e58d8e9c304f513e568
Signed-off-by: changzhu
Reviewed-by: Flora Cui
Reviewed-by: Marek Olšák
Reviewed-by: Huang Rui
---
tests/amdgpu/basic_tests.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
index e75b9d0d..a57dcbb4 100644
--- a/tests/amdgpu/basic_tests.c
+++ b/tests/amdgpu/basic_tests.c
@@ -592,20 +592,6 @@ int suite_basic_tests_init(void)
family_id = gpu_info.family_id;
- if (gpu_info.asic_id == 0x1636) {
- if (amdgpu_set_test_active("Basic Tests",
- "Dispatch Test",
- CU_FALSE))
- fprintf(stderr, "test deactivation failed - %s\n",
- CU_get_error_msg());
-
- if (amdgpu_set_test_active("Basic Tests",
- "Draw Test",
- CU_FALSE))
- fprintf(stderr, "test deactivation failed - %s\n",
- CU_get_error_msg());
- }
-
return CUE_SUCCESS;
}
@@ -2992,7 +2978,7 @@ void amdgpu_memset_draw(amdgpu_device_handle
device_handle,
resources[1] = bo_shader_ps;
resources[2] = bo_shader_vs;
resources[3] = bo_cmd;
- r = amdgpu_bo_list_create(device_handle, 3, resources, NULL, _list);
+ r = amdgpu_bo_list_create(device_handle, 4, resources, NULL, _list);
CU_ASSERT_EQUAL(r, 0);
ib_info.ib_mc_address = mc_address_cmd;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: disallow direct upload save restore list from gfx driver
Reviewed-by: Candice Li
Thanks,
Candice Li
-Original Message-
From: Zhang, Hawking
Sent: Monday, November 04, 2019 6:18 PM
To: amd-gfx@lists.freedesktop.org; Clements John ; Li,
Candice
Cc: Zhang, Hawking
Subject: [PATCH] drm/amdgpu: disallow direct upload save restore list from gfx
driver
Direct uploading save/restore list via mmio register writes breaks the security
policy. Instead, the driver should pass s list to psp.
For all the ASICs that use rlc v2_1 headers, the driver actually upload s
list twice, in non-psp ucode front door loading phase and gfx pg initialization
phase.
The latter is not allowed.
VG12 is the only exception where the driver still keeps legacy approach for S
list uploading. In theory, this can be elimnated if we have valid srcntl ucode
for VG12.
Change-Id: I8cc8e0126f746aae43b9114e05bc111ee7b23531
Signed-off-by: Hawking Zhang
---
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 0525fc6..d14c4d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -2725,7 +2725,8 @@ static void gfx_v9_0_init_pg(struct amdgpu_device *adev)
* And it's needed by gfxoff feature.
*/
if (adev->gfx.rlc.is_rlc_v2_1) {
- gfx_v9_1_init_rlc_save_restore_list(adev);
+ if (adev->asic_type == CHIP_VEGA12)
+ gfx_v9_1_init_rlc_save_restore_list(adev);
gfx_v9_0_enable_save_restore_machine(adev);
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 12/13] drm/dp_mst: Add DSC enablement helpers to DRM
Hey! Great start so far, some comments down below:
On Wed, 2019-10-30 at 15:24 -0400, mikita.lip...@amd.com wrote:
> From: Mikita Lipski
>
> Adding the following elements to add MST DSC support to DRM:
>
> - dsc_enable boolean flag to drm_dp_vcpi_allocation structure to signal,
> which port got DSC enabled
>
> - function drm_dp_helper_update_vcpi_slots_for_dsc allows reallocation
> of newly recalculated VCPI slots and raises dsc_enable flag on the port.
>
> - function drm_dp_mst_update_dsc_crtcs is called in drm_dp_mst_atomic_check,
> its purpose is to iterate through all the ports in the topology and set
> mode_changed flag on crtc if DSC has been enabled.
>
> Cc: Harry Wentland
> Cc: Lyude Paul
> Signed-off-by: Mikita Lipski
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 103 +-
> include/drm/drm_dp_mst_helper.h | 4 +
> 2 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index d5df02315e14..4f2f09fe32f8 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -73,6 +73,7 @@ static bool drm_dp_validate_guid(struct
> drm_dp_mst_topology_mgr *mgr,
> static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux);
> static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux);
> static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
> +static void drm_dp_mst_update_dsc_crtcs(struct drm_dp_mst_topology_state
> *mst_state);
>
> #define DP_STR(x) [DP_ ## x] = #x
>
> @@ -3293,6 +3294,65 @@ int drm_dp_atomic_find_vcpi_slots(struct
> drm_atomic_state *state,
> }
> EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots);
>
> +/**
> + * drm_dp_helper_update_vcpi_slots_for_dsc() - Update VCPI slots with new
> on the state
> + *
> + * @state: global atomic state
> + * @port: port to find vcpi slots
> + * @pbn: updated bandwidth required for the mode in PBN
> + *
> + * Function reallocates VCPI slots to the @port by calling
> + * drm_dp_atomic_find_vcpi_slots. The assumption is that VCPI slots
> + * have already been allocated and this is second call overwritting
> + * initial values. After the VCPI is allocated dsc_enable flag is set to
> + * true for atomic check.
> + *
> + * It is driver's responsibility to call this function after it decides
> + * to enable DSC.
> + *
> + * See also:
> + * drm_dp_mst_update_dsc_crtcs()
> + *
> + * Returns:
> + * Total slots in the atomic state assigned for this port, or a negative
> error
> + * code if the port no longer exists or vcpi slots haven't been assigned.
> + */
> +int drm_dp_helper_update_vcpi_slots_for_dsc(struct drm_atomic_state *state,
> + struct drm_dp_mst_port *port,
> + int pbn)
> +{
> + struct drm_dp_mst_topology_state *topology_state;
> + struct drm_dp_vcpi_allocation *pos;
> + bool found = false;
> + int vcpi = 0;
> +
> + topology_state = drm_atomic_get_mst_topology_state(state, port->mgr);
> +
> + if (IS_ERR(topology_state))
> + return PTR_ERR(topology_state);
> +
> + list_for_each_entry(pos, _state->vcpis, next) {
> + if (pos->port == port) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found || !pos->vcpi)
> + return -EINVAL;
> +
> + vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr,
> + port, pbn);
> +
> + if (vcpi < 0)
> + return -EINVAL;
> +
> + pos->dsc_enable = true;
> +
> + return vcpi;
> +}
> +
This helper I think we can simplify a bit by dropping it and merging it with
drm_dp_mst_update_dsc_crtcs(). I've got a more in-depth explanation of what I
mean down below:
> +EXPORT_SYMBOL(drm_dp_helper_update_vcpi_slots_for_dsc);
> /**
> * drm_dp_atomic_release_vcpi_slots() - Release allocated vcpi slots
> * @state: global atomic state
> @@ -3871,6 +3931,46 @@ drm_dp_mst_atomic_check_topology_state(struct
> drm_dp_mst_topology_mgr *mgr,
> return 0;
> }
>
> +/**
> + * drm_dp_mst_update_dsc_crtcs - Set mode change flag on CRTCs which
> + * just got DSC enabled
> + * @state: Pointer to the new drm_dp_mst_topology_state
> + *
> + * Itearate through all the ports in MST topology to check if DSC
> + * has been enabled on any of them. Set mode_changed to true on
> + * crtc state that just got DSC enabled.
> + *
> + * See also:
> + * drm_dp_helper_update_vcpi_slots_for_dsc()
> + */
> +static void
> +drm_dp_mst_update_dsc_crtcs(struct drm_dp_mst_topology_state *mst_state)
> +{
Just grab the atomic state from within this function, not really much point in
making the caller pull in the mst topoloy state since we're the only ones
using it
> + struct drm_dp_vcpi_allocation *pos;
> + struct drm_dp_mst_port *port;
> + struct drm_connector_state *conn_state;
> +
Re: [PATCH 11/13] drm/amd/display: MST DSC compute fair share
So: one comment down below:
On Wed, 2019-10-30 at 15:24 -0400, mikita.lip...@amd.com wrote:
> From: David Francis
>
> If there is limited link bandwidth on a MST network,
> it must be divided fairly between the streams on that network
>
> Implement an algorithm to determine the correct DSC config
> for each stream
>
> The algorithm:
> This
> [ ] ( )
> represents the range of bandwidths possible for a given stream.
> The [] area represents the range of DSC configs, and the ()
> represents no DSC. The bandwidth used increases from left to right.
>
> First, try disabling DSC on all streams
> [ ] (|)
> [ ](|)
> Check this against the bandwidth limits of the link and each branch
> (including each endpoint). If it passes, the job is done
>
> Second, try maximum DSC compression on all streams
> that support DSC
> [| ]( )
> [|] ( )
> If this does not pass, then enabling this combination of streams
> is impossible
>
> Otherwise, divide the remaining bandwidth evenly amongst the streams
> [| ] ( )
> [| ]( )
>
> If one or more of the streams reach minimum compression, evenly
> divide the reamining bandwidth amongst the remaining streams
> [|] ( )
> [ |] ( )
> [ | ] ( )
> [ | ] ( )
>
> If all streams can reach minimum compression, disable compression
> greedily
> [ |] ( )
> [|]( )
> [ ](|)
>
> Perform this algorithm on each full update, on each MST link
> with at least one DSC stream on it
>
> After the configs are computed, call
> dcn20_add_dsc_to_stream_resource on each stream with DSC enabled.
> It is only after all streams are created that we can know which
> of them will need DSC.
>
> Do all of this at the end of amdgpu atomic check. If it fails,
> fail check; This combination of timings cannot be supported.
>
> Reviewed-by: Wenjing Liu
> Signed-off-by: David Francis
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +
> .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 386 ++
> .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 4 +
> .../drm/amd/display/dc/dcn20/dcn20_resource.c | 7 +-
> .../drm/amd/display/dc/dcn20/dcn20_resource.h | 1 +
> 5 files changed, 400 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 1309e9cfdea3..1bbdf29f3c7d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7780,6 +7780,10 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
> if (ret)
> goto fail;
>
> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
> + if (!compute_mst_dsc_configs_for_state(dm_state->context))
> + goto fail;
> +#endif
> if (dc_validate_global_state(dc, dm_state->context, false) !=
> DC_OK) {
> ret = -EINVAL;
> goto fail;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 804a00082bee..c58cf41f3086 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -38,6 +38,8 @@
>
> #include "i2caux_interface.h"
>
> +#include "dc/dcn20/dcn20_resource.h"
> +
> /* #define TRACE_DPCD */
>
> #ifdef TRACE_DPCD
> @@ -491,3 +493,387 @@ void amdgpu_dm_initialize_dp_connector(struct
> amdgpu_display_manager *dm,
> aconnector->connector_id);
> }
>
> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
> +struct dsc_mst_fairness_params {
> + struct dc_crtc_timing *timing;
> + struct dc_sink *sink;
> + struct dc_dsc_bw_range bw_range;
> + bool compression_possible;
> + struct drm_dp_mst_port *port;
> +};
> +
> +struct dsc_mst_fairness_vars {
> + int pbn;
> + bool dsc_enabled;
> + int bpp_x16;
> +};
> +
> +static bool port_downstream_of_branch(struct drm_dp_mst_port *port,
> + struct drm_dp_mst_branch *branch)
> +{
> + while (port->parent) {
> + if (port->parent == branch)
> + return true;
> +
> + if (port->parent->port_parent)
> + port = port->parent->port_parent;
> + else
> + break;
> + }
> + return false;
> +}
> +
> +static bool check_pbn_limit_on_branch(struct drm_dp_mst_branch *branch,
> + struct dsc_mst_fairness_params *params,
> + struct dsc_mst_fairness_vars *vars, int count)
> +{
> + struct drm_dp_mst_port *port;
> + int
Power consumption of Navi and Polaris
Hello Alex et al., can you please have a look at this bug report and findings? Sapphire Pulse RX 5700 XT power consumption https://bugs.freedesktop.org/show_bug.cgi?id=111482 e.g. https://bugs.freedesktop.org/show_bug.cgi?id=111482#c30 Thanks, Dieter ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[amd-staging-drm-nex] Planned move to 5.4-rc6+ soon?
Hello Alex, like AMD...;-) [Why] 5.3-rcX (-rc3) - has broken (regressed) block layer (CD/DVD, games...) - fixed in 5.3 final - ACPI (power management) regression - fixed in 5.3 final - USB 3.1/3.2 regression (5.000 vs 10.000 Mbit/s mode, in use heavily here) even with AMD's partner ASMedia (ASM 2142+ chips) - fixed in 5.4-rc (!!!) - Navi problems (e.g. SDMA) some progress/mitigation with 5.4-rc [How] Switch to 5.4-rc4 (or better -rc6) as soon as possible Back to pp power consumption testing then... Greetings, Dieter ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 02/13] drm/dp_mst: Add PBN calculation for DSC modes
hey-sorry this took a little bit to get back to. I reviewed this before, but
this patch doesn't seem to be rebased against drm-tip otherwise it would
reflect the changes that happened with drm_dp_calc_pbn_mode() upstream:
https://patchwork.freedesktop.org/patch/332935/
can you rebase these patches against drm-tip and also update the
igt_dp_mst_calc_pbn_mode() test here?
https://cgit.freedesktop.org/drm-tip/tree/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c#n14
On Wed, 2019-10-30 at 15:24 -0400, mikita.lip...@amd.com wrote:
> From: David Francis
>
> With DSC, bpp can be fractional in multiples of 1/16.
>
> Change drm_dp_calc_pbn_mode to reflect this, adding a new
> parameter bool dsc. When this parameter is true, treat the
> bpp parameter as having units not of bits per pixel, but
> 1/16 of a bit per pixel
>
> v2: Don't add separate function for this
>
> Reviewed-by: Manasi Navare
> Reviewed-by: Lyude Paul
> Reviewed-by: Harry Wentland
> Signed-off-by: David Francis
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 2 +-
> drivers/gpu/drm/drm_dp_mst_topology.c| 16
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++-
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 ++-
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
> include/drm/drm_dp_mst_helper.h | 3 +--
> 6 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index d75726013436..1309e9cfdea3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4656,7 +4656,7 @@ static int dm_encoder_helper_atomic_check(struct
> drm_encoder *encoder,
> color_depth = convert_color_depth_from_display_info(connector,
> conn_state);
> bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
> clock = adjusted_mode->clock;
> - dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock,
> bpp);
> + dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp,
> false);
> }
> dm_new_connector_state->vcpi_slots =
> drm_dp_atomic_find_vcpi_slots(state,
> mst
> _mgr,
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 82add736e17d..3e7b7553cf4d 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3534,10 +3534,11 @@ EXPORT_SYMBOL(drm_dp_check_act_status);
> * drm_dp_calc_pbn_mode() - Calculate the PBN for a mode.
> * @clock: dot clock for the mode
> * @bpp: bpp for the mode.
> + * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel
> *
> * This uses the formula in the spec to calculate the PBN value for a mode.
> */
> -int drm_dp_calc_pbn_mode(int clock, int bpp)
> +int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
> {
> u64 kbps;
> s64 peak_kbps;
> @@ -3555,11 +3556,18 @@ int drm_dp_calc_pbn_mode(int clock, int bpp)
>* peak_kbps *= (1006/1000)
>* peak_kbps *= (64/54)
>* peak_kbps *= 8convert to bytes
> + *
> + * If the bpp is in units of 1/16, further divide by 16. Put this
> + * factor in the numerator rather than the denominator to avoid
> + * integer overflow
>*/
>
> numerator = 64 * 1006;
> denominator = 54 * 8 * 1000 * 1000;
>
> + if (dsc)
> + numerator /= 16;
> +
> kbps *= numerator;
> peak_kbps = drm_fixp_from_fraction(kbps, denominator);
>
> @@ -3570,19 +3578,19 @@ EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
> static int test_calc_pbn_mode(void)
> {
> int ret;
> - ret = drm_dp_calc_pbn_mode(154000, 30);
> + ret = drm_dp_calc_pbn_mode(154000, 30, false);
> if (ret != 689) {
> DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
> expected PBN %d, actual PBN %d.\n",
> 154000, 30, 689, ret);
> return -EINVAL;
> }
> - ret = drm_dp_calc_pbn_mode(234000, 30);
> + ret = drm_dp_calc_pbn_mode(234000, 30, false);
> if (ret != 1047) {
> DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
> expected PBN %d, actual PBN %d.\n",
> 234000, 30, 1047, ret);
> return -EINVAL;
> }
> - ret = drm_dp_calc_pbn_mode(297000, 24);
> + ret = drm_dp_calc_pbn_mode(297000, 24, false);
> if (ret != 1063) {
> DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
> expected PBN %d, actual PBN %d.\n",
> 297000, 24, 1063, ret);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 2c5ac3dd647f..dfac450841df 100644
> ---
[PATCH v2] drm/amd/display: Hook up drm interface for DP 1.4 edid corruption test
-v3: Rename to avoid confusion
Signed-off-by: Jerry (Fangzhi) Zuo
---
.../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 35 +-
1 file changed, 7 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 11e5784aa62a..927fdac77b6f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -575,6 +575,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
struct dc_sink *sink)
{
struct amdgpu_dm_connector *aconnector = link->priv;
+ struct drm_connector *connector = >base;
struct i2c_adapter *ddc;
int retry = 3;
enum dc_edid_status edid_status;
@@ -592,6 +593,12 @@ enum dc_edid_status dm_helpers_read_local_edid(
edid = drm_get_edid(>base, ddc);
+ if (link->aux_mode && connector->edid_corrupt)
+
drm_dp_send_real_edid_checksum(>dm_dp_aux.aux,
connector->real_edid_checksum);
+
+ if (!edid && connector->edid_corrupt)
+ return EDID_BAD_CHECKSUM;
+
if (!edid)
return EDID_NO_RESPONSE;
@@ -612,34 +619,6 @@ enum dc_edid_status dm_helpers_read_local_edid(
DRM_ERROR("EDID err: %d, on connector: %s",
edid_status,
aconnector->base.name);
- if (link->aux_mode) {
- union test_request test_request = { {0} };
- union test_response test_response = { {0} };
-
- dm_helpers_dp_read_dpcd(ctx,
- link,
- DP_TEST_REQUEST,
- _request.raw,
- sizeof(union test_request));
-
- if (!test_request.bits.EDID_READ)
- return edid_status;
-
- test_response.bits.EDID_CHECKSUM_WRITE = 1;
-
- dm_helpers_dp_write_dpcd(ctx,
- link,
- DP_TEST_EDID_CHECKSUM,
-
>dc_edid.raw_edid[sink->dc_edid.length-1],
- 1);
-
- dm_helpers_dp_write_dpcd(ctx,
- link,
- DP_TEST_RESPONSE,
- _response.raw,
- sizeof(test_response));
-
- }
return edid_status;
}
--
2.14.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6
DP 1.4 edid corruption test requires source DUT to write calculated
CRC, not the corrupted CRC from reference sink.
Return the calculated CRC back, and initiate the required sequence.
-v2: Have separate routine for returning real CRC
-v3: Rewrite checksum computation routine to avoid duplicated code.
Rename to avoid confusion
Signed-off-by: Jerry (Fangzhi) Zuo
---
drivers/gpu/drm/drm_dp_helper.c | 36
drivers/gpu/drm/drm_edid.c | 18 +++---
include/drm/drm_connector.h | 7 +++
include/drm/drm_dp_helper.h | 3 +++
4 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffc68d305afe..75bdffd840c6 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
}
EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
+/**
+ * drm_dp_send_real_edid_checksum() - send back real edid checksum value
+ * @aux: DisplayPort AUX channel
+ * @bad_edid_checksum: real edid checksum for the last block
+ *
+ * Returns true on success
+ */
+bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
+u8 real_edid_checksum)
+{
+u8 link_edid_read = 0, auto_test_req = 0;
+u8 test_resp = 0;
+
+drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req, 1);
+auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
+
+drm_dp_dpcd_read(aux, DP_TEST_REQUEST, _edid_read, 1);
+link_edid_read &= DP_TEST_LINK_EDID_READ;
+
+if (!auto_test_req || !link_edid_read) {
+DRM_DEBUG_KMS("Source DUT does not support TEST_EDID_READ\n");
+return false;
+}
+
+drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req,
1);
+
+/* send back checksum for the last edid extension block data */
+drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, _edid_checksum, 1);
+
+test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
+drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, _resp, 1);
+
+return true;
+}
+EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
+
/**
* drm_dp_link_probe() - probe a DisplayPort link for capabilities
* @aux: DisplayPort AUX channel
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 82a4ceed3fcf..ff64e5f1feb6 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1348,10 +1348,19 @@ static int drm_edid_block_checksum(const u8 *raw_edid)
{
int i;
u8 csum = 0;
- for (i = 0; i < EDID_LENGTH; i++)
+
+ for (i = 0; i < EDID_LENGTH - 1; i++)
csum += raw_edid[i];
- return csum;
+ return (0x100 - csum);
+}
+
+static bool drm_edid_block_checksum_diff(const u8 *raw_edid, u8 real_checksum)
+{
+ if (raw_edid[EDID_LENGTH - 1] != real_checksum)
+ return true;
+ else
+ return false;
}
static bool drm_edid_is_zero(const u8 *in_edid, int length)
@@ -1409,7 +1418,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid,
}
csum = drm_edid_block_checksum(raw_edid);
- if (csum) {
+ if (drm_edid_block_checksum_diff(raw_edid, csum)) {
if (edid_corrupt)
*edid_corrupt = true;
@@ -1572,6 +1581,9 @@ static void connector_bad_edid(struct drm_connector
*connector,
prefix, DUMP_PREFIX_NONE, 16, 1,
block, EDID_LENGTH, false);
}
+
+ /* Calculate real checksum for the last edid extension block data */
+ connector->real_edid_checksum = drm_edid_block_checksum(edid +
edid[0x7e] * EDID_LENGTH);
}
/* Get override or firmware EDID */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 681cb590f952..eb0d8c7b35fd 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1345,6 +1345,13 @@ struct drm_connector {
* rev1.1 4.2.2.6
*/
bool edid_corrupt;
+ /**
+ * @real_edid_checksum: real edid checksum value for corrupted edid
block.
+ * Required in Displayport 1.4 compliance testing
+ * rev1.1 4.2.2.6
+ */
+uint8_t real_edid_checksum;
+
/** @debugfs_entry: debugfs directory for this connector */
struct dentry *debugfs_entry;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5a795075d5da..84709d7810f8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1383,6 +1383,9 @@ static inline ssize_t drm_dp_dpcd_writeb(struct
drm_dp_aux *aux,
int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
u8 status[DP_LINK_STATUS_SIZE]);
+bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
+ u8
Re: [PATCH] drm/amd/display: remove "setting DIG_MODE twice" workaround
On 2019-11-04 2:49 p.m., Liu, Zhan wrote:
> Thank you Nick for the advice. I just reverted the original commit.
>
> Zhan
The revert still needs code review.
Nicholas Kazlauskas
>
>> -Original Message-
>> From: Kazlauskas, Nicholas
>> Sent: 2019/November/04, Monday 11:53 AM
>> To: Liu, Zhan ; amd-gfx@lists.freedesktop.org; Wu,
>> Hersen
>> Subject: Re: [PATCH] drm/amd/display: remove "setting DIG_MODE twice"
>> workaround
>>
>> On 2019-11-04 11:06 a.m., Liu, Zhan wrote:
>>> [Why]
>>> The root cause of Navi14 HDMI pink screen issue has been found.
>>> There is no need to set DIG_MODE twice anymore.
>>>
>>> [How]
>>> Remove "setting DIG_MODE" twice workaround.
>>>
>>> Signed-off-by: Zhan Liu
>>
>> Please use git to revert the commit instead:
>>
>> eg.
>>
>> git revert
>>
>> Then add the reasoning to the revert commit message.
>>
>> Thanks,
>>
>> Nicholas Kazlauskas
>>
>>> ---
>>>drivers/gpu/drm/amd/display/dc/core/dc_link.c | 9 -
>>>1 file changed, 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>>> index cc94c1a73daa..12ba6fdf89b7 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>>> @@ -3027,15 +3027,6 @@ void core_link_enable_stream(
>>>
>>> CONTROLLER_DP_TEST_PATTERN_VIDEOMODE,
>>> COLOR_DEPTH_UNDEFINED);
>>>
>>> - /* This second call is needed to reconfigure the DIG
>>> -* as a workaround for the incorrect value being applied
>>> -* from transmitter control.
>>> -*/
>>> - if (!dc_is_virtual_signal(pipe_ctx->stream->signal))
>>> - stream->link->link_enc->funcs->setup(
>>> - stream->link->link_enc,
>>> - pipe_ctx->stream->signal);
>>> -
>>>#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
>>> if (pipe_ctx->stream->timing.flags.DSC) {
>>> if (dc_is_dp_signal(pipe_ctx->stream->signal) ||
>>> --
>>> 2.21.0
>>> ___
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/display: remove "setting DIG_MODE twice" workaround
Thank you Nick for the advice. I just reverted the original commit.
Zhan
> -Original Message-
> From: Kazlauskas, Nicholas
> Sent: 2019/November/04, Monday 11:53 AM
> To: Liu, Zhan ; amd-gfx@lists.freedesktop.org; Wu,
> Hersen
> Subject: Re: [PATCH] drm/amd/display: remove "setting DIG_MODE twice"
> workaround
>
> On 2019-11-04 11:06 a.m., Liu, Zhan wrote:
> > [Why]
> > The root cause of Navi14 HDMI pink screen issue has been found.
> > There is no need to set DIG_MODE twice anymore.
> >
> > [How]
> > Remove "setting DIG_MODE" twice workaround.
> >
> > Signed-off-by: Zhan Liu
>
> Please use git to revert the commit instead:
>
> eg.
>
> git revert
>
> Then add the reasoning to the revert commit message.
>
> Thanks,
>
> Nicholas Kazlauskas
>
> > ---
> > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 9 -
> > 1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index cc94c1a73daa..12ba6fdf89b7 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -3027,15 +3027,6 @@ void core_link_enable_stream(
> >
> > CONTROLLER_DP_TEST_PATTERN_VIDEOMODE,
> > COLOR_DEPTH_UNDEFINED);
> >
> > - /* This second call is needed to reconfigure the DIG
> > -* as a workaround for the incorrect value being applied
> > -* from transmitter control.
> > -*/
> > - if (!dc_is_virtual_signal(pipe_ctx->stream->signal))
> > - stream->link->link_enc->funcs->setup(
> > - stream->link->link_enc,
> > - pipe_ctx->stream->signal);
> > -
> > #ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
> > if (pipe_ctx->stream->timing.flags.DSC) {
> > if (dc_is_dp_signal(pipe_ctx->stream->signal) ||
> > --
> > 2.21.0
> > ___
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Mon, Nov 04, 2019 at 12:05:40PM -0500, Alex Deucher wrote: > On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote: > > > > On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote: > > > On 2019-11-04 5:53 a.m., Daniel Vetter wrote: > > > > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote: > > > >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet > > > >> wrote: > > > >>> > > > >>> I misunderstood and was talking about the ksv validation specifically > > > >>> (usage of drm_hdcp_check_ksvs_revoked()). > > > >> > > > >> Hm for that specifically I think you want to do both, i.e. both > > > >> consult your psp, but also check for revoked ksvs with the core > > > >> helper. At least on some platforms only the core helper might have the > > > >> updated revoke list. > > > >> > > > > > > I think it's an either/or. Either we use an HDCP implementation that's > > > fully running in x86 kernel space (still not sure how that's compliant) > > > or we fully rely on our PSP FW to do what it's designed to do. I don't > > > think it makes sense to mix and match here. > > > > Then you need to somehow tie the revoke list that's in the psp to the > > revoke list update logic we have. That's what we've done for hdcp2 (which > > is similarly to yours implemented in hw). The point is that on linux we > > now have a standard way to get these revoke lists updated/handled. > > > > I guess it wasn't clear how exactly I think you're supposed to combine > > them? > > There's no driver sw required at all for our implementation and as far > as I know, HDCP 2.x requires that all of the key revoke handling be > handled in a secure processor rather than than on the host processor, > so I'm not sure how we make use if it. All the driver sw is > responsible for doing is saving/restoring the potentially updated srm > at suspend/resume/etc. Uh, you don't have a permanent store on the chip? I thought another requirement is that you can't downgrade. And for hw solutions all you do with the updated revoke cert is stuff it into the hw, it's purely for updating it. And those updates need to come from somewhere else (usually in the media you play), the kernel can't fetch them over the internet itself. I thought we already had the function to give you the srm directly so you can stuff it into the hw, but looks like that part isn't there (yet). -Daniel > > Alex > > > -Daniel > > > > > > > > > > >>> For the defines I will create patches to use drm_hdcp where it is > > > >>> usable. > > > >> > > > >> Thanks a lot. Ime once we have shared definitions it's much easier to > > > >> also share some helpers, where it makes sense. > > > >> > > > >> Aside I think the hdcp code could also use a bit of demidlayering. At > > > >> least I'm not understanding why you add a 2nd abstraction layer for > > > >> i2c/dpcd, dm_helper already has that. That seems like one abstraction > > > >> layer too much. > > > > > > > > I haven't seen anything fly by or in the latest pull request ... you > > > > folks still working on this or more put on the "maybe, probably never" > > > > pile? > > > > > > > > > > Following up with Bhawan. > > > > > > Harry > > > > > > > -Daniel > > > > > > > > > > > >> -Daniel > > > >> > > > >>> > > > >>> > > > >>> Bhawan > > > >>> > > > >>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote: > > > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet > > > wrote: > > > > Hi, > > > > > > > > The reason we don't use drm_hdcp is because our policy is to do hdcp > > > > verification using PSP/HW (onboard secure processor). > > > i915 also uses hw to auth, we still use the parts from drm_hdcp ... > > > Did you actually look at what's in there? It's essentially just > > > shared > > > defines and data structures from the standard, plus a few minimal > > > helpers to en/decode some bits. Just from a quick read the entire > > > patch very much looks like midlayer everywhere design that we > > > discussed back when DC landed ... > > > -Daniel > > > > > > > Bhawan > > > > > > > > On 2019-10-09 12:32 p.m., Daniel Vetter wrote: > > > >> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote: > > > >>> Hi, > > > >>> > > > >>> Static analysis with Coverity has detected a potential issue with > > > >>> function validate_bksv in > > > >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with > > > >>> recent > > > >>> commit: > > > >>> > > > >>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec > > > >>> Author: Bhawanpreet Lakha > > > >>> Date: Tue Aug 6 17:52:01 2019 -0400 > > > >>> > > > >>> drm/amd/display: Add HDCP module > > > >> I think the real question here is ... why is this not using > > > >> drm_hdcp? > > > >> -Daniel > > > >> > > > >>> The analysis is as follows: > > > >>> > > > >>>28 static inline enum mod_hdcp_status validate_bksv(struct
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Mon, Nov 4, 2019 at 11:55 AM Daniel Vetter wrote:
>
> On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote:
> > On 2019-11-04 5:53 a.m., Daniel Vetter wrote:
> > > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
> > >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
> > >> wrote:
> > >>>
> > >>> I misunderstood and was talking about the ksv validation specifically
> > >>> (usage of drm_hdcp_check_ksvs_revoked()).
> > >>
> > >> Hm for that specifically I think you want to do both, i.e. both
> > >> consult your psp, but also check for revoked ksvs with the core
> > >> helper. At least on some platforms only the core helper might have the
> > >> updated revoke list.
> > >>
> >
> > I think it's an either/or. Either we use an HDCP implementation that's
> > fully running in x86 kernel space (still not sure how that's compliant)
> > or we fully rely on our PSP FW to do what it's designed to do. I don't
> > think it makes sense to mix and match here.
>
> Then you need to somehow tie the revoke list that's in the psp to the
> revoke list update logic we have. That's what we've done for hdcp2 (which
> is similarly to yours implemented in hw). The point is that on linux we
> now have a standard way to get these revoke lists updated/handled.
>
> I guess it wasn't clear how exactly I think you're supposed to combine
> them?
There's no driver sw required at all for our implementation and as far
as I know, HDCP 2.x requires that all of the key revoke handling be
handled in a secure processor rather than than on the host processor,
so I'm not sure how we make use if it. All the driver sw is
responsible for doing is saving/restoring the potentially updated srm
at suspend/resume/etc.
Alex
> -Daniel
>
>
> >
> > >>> For the defines I will create patches to use drm_hdcp where it is
> > >>> usable.
> > >>
> > >> Thanks a lot. Ime once we have shared definitions it's much easier to
> > >> also share some helpers, where it makes sense.
> > >>
> > >> Aside I think the hdcp code could also use a bit of demidlayering. At
> > >> least I'm not understanding why you add a 2nd abstraction layer for
> > >> i2c/dpcd, dm_helper already has that. That seems like one abstraction
> > >> layer too much.
> > >
> > > I haven't seen anything fly by or in the latest pull request ... you
> > > folks still working on this or more put on the "maybe, probably never"
> > > pile?
> > >
> >
> > Following up with Bhawan.
> >
> > Harry
> >
> > > -Daniel
> > >
> > >
> > >> -Daniel
> > >>
> > >>>
> > >>>
> > >>> Bhawan
> > >>>
> > >>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> > wrote:
> > > Hi,
> > >
> > > The reason we don't use drm_hdcp is because our policy is to do hdcp
> > > verification using PSP/HW (onboard secure processor).
> > i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> > Did you actually look at what's in there? It's essentially just shared
> > defines and data structures from the standard, plus a few minimal
> > helpers to en/decode some bits. Just from a quick read the entire
> > patch very much looks like midlayer everywhere design that we
> > discussed back when DC landed ...
> > -Daniel
> >
> > > Bhawan
> > >
> > > On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> > >> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> > >>> Hi,
> > >>>
> > >>> Static analysis with Coverity has detected a potential issue with
> > >>> function validate_bksv in
> > >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with
> > >>> recent
> > >>> commit:
> > >>>
> > >>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> > >>> Author: Bhawanpreet Lakha
> > >>> Date: Tue Aug 6 17:52:01 2019 -0400
> > >>>
> > >>> drm/amd/display: Add HDCP module
> > >> I think the real question here is ... why is this not using drm_hdcp?
> > >> -Daniel
> > >>
> > >>> The analysis is as follows:
> > >>>
> > >>>28 static inline enum mod_hdcp_status validate_bksv(struct
> > >>> mod_hdcp *hdcp)
> > >>>29 {
> > >>>
> > >>> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
> > >>>
> > >>> 1. overrun-local:
> > >>> Overrunning array of 5 bytes at byte offset 7 by dereferencing
> > >>> pointer
> > >>> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
> > >>>
> > >>>30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> > >>>31uint8_t count = 0;
> > >>>32
> > >>>33while (n) {
> > >>>34count++;
> > >>>35n &= (n - 1);
> > >>>36}
> > >>>
> > >>> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> > >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
> > >>>
> > >>> struct
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Mon, Nov 04, 2019 at 03:23:09PM +, Harry Wentland wrote:
> On 2019-11-04 5:53 a.m., Daniel Vetter wrote:
> > On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
> >> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
> >> wrote:
> >>>
> >>> I misunderstood and was talking about the ksv validation specifically
> >>> (usage of drm_hdcp_check_ksvs_revoked()).
> >>
> >> Hm for that specifically I think you want to do both, i.e. both
> >> consult your psp, but also check for revoked ksvs with the core
> >> helper. At least on some platforms only the core helper might have the
> >> updated revoke list.
> >>
>
> I think it's an either/or. Either we use an HDCP implementation that's
> fully running in x86 kernel space (still not sure how that's compliant)
> or we fully rely on our PSP FW to do what it's designed to do. I don't
> think it makes sense to mix and match here.
Then you need to somehow tie the revoke list that's in the psp to the
revoke list update logic we have. That's what we've done for hdcp2 (which
is similarly to yours implemented in hw). The point is that on linux we
now have a standard way to get these revoke lists updated/handled.
I guess it wasn't clear how exactly I think you're supposed to combine
them?
-Daniel
>
> >>> For the defines I will create patches to use drm_hdcp where it is usable.
> >>
> >> Thanks a lot. Ime once we have shared definitions it's much easier to
> >> also share some helpers, where it makes sense.
> >>
> >> Aside I think the hdcp code could also use a bit of demidlayering. At
> >> least I'm not understanding why you add a 2nd abstraction layer for
> >> i2c/dpcd, dm_helper already has that. That seems like one abstraction
> >> layer too much.
> >
> > I haven't seen anything fly by or in the latest pull request ... you
> > folks still working on this or more put on the "maybe, probably never"
> > pile?
> >
>
> Following up with Bhawan.
>
> Harry
>
> > -Daniel
> >
> >
> >> -Daniel
> >>
> >>>
> >>>
> >>> Bhawan
> >>>
> >>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> wrote:
> > Hi,
> >
> > The reason we don't use drm_hdcp is because our policy is to do hdcp
> > verification using PSP/HW (onboard secure processor).
> i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> Did you actually look at what's in there? It's essentially just shared
> defines and data structures from the standard, plus a few minimal
> helpers to en/decode some bits. Just from a quick read the entire
> patch very much looks like midlayer everywhere design that we
> discussed back when DC landed ...
> -Daniel
>
> > Bhawan
> >
> > On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> >> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> >>> Hi,
> >>>
> >>> Static analysis with Coverity has detected a potential issue with
> >>> function validate_bksv in
> >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
> >>> commit:
> >>>
> >>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> >>> Author: Bhawanpreet Lakha
> >>> Date: Tue Aug 6 17:52:01 2019 -0400
> >>>
> >>> drm/amd/display: Add HDCP module
> >> I think the real question here is ... why is this not using drm_hdcp?
> >> -Daniel
> >>
> >>> The analysis is as follows:
> >>>
> >>>28 static inline enum mod_hdcp_status validate_bksv(struct
> >>> mod_hdcp *hdcp)
> >>>29 {
> >>>
> >>> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
> >>>
> >>> 1. overrun-local:
> >>> Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
> >>> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
> >>>
> >>>30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> >>>31uint8_t count = 0;
> >>>32
> >>>33while (n) {
> >>>34count++;
> >>>35n &= (n - 1);
> >>>36}
> >>>
> >>> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
> >>>
> >>> struct mod_hdcp_message_hdcp1 {
> >>> uint8_t an[8];
> >>> uint8_t aksv[5];
> >>> uint8_t ainfo;
> >>> uint8_t bksv[5];
> >>> uint16_tr0p;
> >>> uint8_t bcaps;
> >>> uint16_tbstatus;
> >>> uint8_t ksvlist[635];
> >>> uint16_tksvlist_size;
> >>> uint8_t vp[20];
> >>>
> >>> uint16_tbinfo_dp;
> >>> };
> >>>
> >>> variable n is going to contain the contains of r0p and bcaps. I'm not
> >>> sure if that is intentional. If not, then the count is
Re: [PATCH] drm/amd/display: remove "setting DIG_MODE twice" workaround
On 2019-11-04 11:06 a.m., Liu, Zhan wrote:
> [Why]
> The root cause of Navi14 HDMI pink screen issue has been found.
> There is no need to set DIG_MODE twice anymore.
>
> [How]
> Remove "setting DIG_MODE" twice workaround.
>
> Signed-off-by: Zhan Liu
Please use git to revert the commit instead:
eg.
git revert
Then add the reasoning to the revert commit message.
Thanks,
Nicholas Kazlauskas
> ---
> drivers/gpu/drm/amd/display/dc/core/dc_link.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index cc94c1a73daa..12ba6fdf89b7 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -3027,15 +3027,6 @@ void core_link_enable_stream(
> CONTROLLER_DP_TEST_PATTERN_VIDEOMODE,
> COLOR_DEPTH_UNDEFINED);
>
> - /* This second call is needed to reconfigure the DIG
> -* as a workaround for the incorrect value being applied
> -* from transmitter control.
> -*/
> - if (!dc_is_virtual_signal(pipe_ctx->stream->signal))
> - stream->link->link_enc->funcs->setup(
> - stream->link->link_enc,
> - pipe_ctx->stream->signal);
> -
> #ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
> if (pipe_ctx->stream->timing.flags.DSC) {
> if (dc_is_dp_signal(pipe_ctx->stream->signal) ||
> --
> 2.21.0
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: remove "setting DIG_MODE twice" workaround
[Why]
The root cause of Navi14 HDMI pink screen issue has been found.
There is no need to set DIG_MODE twice anymore.
[How]
Remove "setting DIG_MODE" twice workaround.
Signed-off-by: Zhan Liu
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index cc94c1a73daa..12ba6fdf89b7 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -3027,15 +3027,6 @@ void core_link_enable_stream(
CONTROLLER_DP_TEST_PATTERN_VIDEOMODE,
COLOR_DEPTH_UNDEFINED);
- /* This second call is needed to reconfigure the DIG
-* as a workaround for the incorrect value being applied
-* from transmitter control.
-*/
- if (!dc_is_virtual_signal(pipe_ctx->stream->signal))
- stream->link->link_enc->funcs->setup(
- stream->link->link_enc,
- pipe_ctx->stream->signal);
-
#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
if (pipe_ctx->stream->timing.flags.DSC) {
if (dc_is_dp_signal(pipe_ctx->stream->signal) ||
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: drm/amd/display: Add HDCP module - static analysis bug report
Hi Daniel,
I have the patches prepared but they needed some testing before I send them
(code needed a slight refactor to use the drm_hdcp.h), I should be able to send
the patches this week.
Thanks,
Bhawan
On 2019-11-04 10:23 a.m., Wentland, Harry wrote:
> On 2019-11-04 5:53 a.m., Daniel Vetter wrote:
>> On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
>>> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
>>> wrote:
I misunderstood and was talking about the ksv validation specifically
(usage of drm_hdcp_check_ksvs_revoked()).
>>> Hm for that specifically I think you want to do both, i.e. both
>>> consult your psp, but also check for revoked ksvs with the core
>>> helper. At least on some platforms only the core helper might have the
>>> updated revoke list.
>>>
> I think it's an either/or. Either we use an HDCP implementation that's
> fully running in x86 kernel space (still not sure how that's compliant)
> or we fully rely on our PSP FW to do what it's designed to do. I don't
> think it makes sense to mix and match here.
>
For the defines I will create patches to use drm_hdcp where it is usable.
>>> Thanks a lot. Ime once we have shared definitions it's much easier to
>>> also share some helpers, where it makes sense.
>>>
>>> Aside I think the hdcp code could also use a bit of demidlayering. At
>>> least I'm not understanding why you add a 2nd abstraction layer for
>>> i2c/dpcd, dm_helper already has that. That seems like one abstraction
>>> layer too much.
>> I haven't seen anything fly by or in the latest pull request ... you
>> folks still working on this or more put on the "maybe, probably never"
>> pile?
>>
> Following up with Bhawan.
>
> Harry
>
>> -Daniel
>>
>>
>>> -Daniel
>>>
Bhawan
On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> wrote:
>> Hi,
>>
>> The reason we don't use drm_hdcp is because our policy is to do hdcp
>> verification using PSP/HW (onboard secure processor).
> i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> Did you actually look at what's in there? It's essentially just shared
> defines and data structures from the standard, plus a few minimal
> helpers to en/decode some bits. Just from a quick read the entire
> patch very much looks like midlayer everywhere design that we
> discussed back when DC landed ...
> -Daniel
>
>> Bhawan
>>
>> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
>>> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
Hi,
Static analysis with Coverity has detected a potential issue with
function validate_bksv in
drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
commit:
commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
Author: Bhawanpreet Lakha
Date: Tue Aug 6 17:52:01 2019 -0400
drm/amd/display: Add HDCP module
>>> I think the real question here is ... why is this not using drm_hdcp?
>>> -Daniel
>>>
The analysis is as follows:
28 static inline enum mod_hdcp_status validate_bksv(struct
mod_hdcp *hdcp)
29 {
CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
1. overrun-local:
Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
(uint64_t *)hdcp->auth.msg.hdcp1.bksv.
30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
31uint8_t count = 0;
32
33while (n) {
34count++;
35n &= (n - 1);
36}
hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
struct mod_hdcp_message_hdcp1 {
uint8_t an[8];
uint8_t aksv[5];
uint8_t ainfo;
uint8_t bksv[5];
uint16_tr0p;
uint8_t bcaps;
uint16_tbstatus;
uint8_t ksvlist[635];
uint16_tksvlist_size;
uint8_t vp[20];
uint16_tbinfo_dp;
};
variable n is going to contain the contains of r0p and bcaps. I'm not
sure if that is intentional. If not, then the count is going to be
incorrect if these are non-zero.
Colin
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>>>
>>>
>>> --
>>> Daniel
Re: drm/amd/display: Add HDCP module - static analysis bug report
On 2019-11-04 5:53 a.m., Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
>> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
>> wrote:
>>>
>>> I misunderstood and was talking about the ksv validation specifically
>>> (usage of drm_hdcp_check_ksvs_revoked()).
>>
>> Hm for that specifically I think you want to do both, i.e. both
>> consult your psp, but also check for revoked ksvs with the core
>> helper. At least on some platforms only the core helper might have the
>> updated revoke list.
>>
I think it's an either/or. Either we use an HDCP implementation that's
fully running in x86 kernel space (still not sure how that's compliant)
or we fully rely on our PSP FW to do what it's designed to do. I don't
think it makes sense to mix and match here.
>>> For the defines I will create patches to use drm_hdcp where it is usable.
>>
>> Thanks a lot. Ime once we have shared definitions it's much easier to
>> also share some helpers, where it makes sense.
>>
>> Aside I think the hdcp code could also use a bit of demidlayering. At
>> least I'm not understanding why you add a 2nd abstraction layer for
>> i2c/dpcd, dm_helper already has that. That seems like one abstraction
>> layer too much.
>
> I haven't seen anything fly by or in the latest pull request ... you
> folks still working on this or more put on the "maybe, probably never"
> pile?
>
Following up with Bhawan.
Harry
> -Daniel
>
>
>> -Daniel
>>
>>>
>>>
>>> Bhawan
>>>
>>> On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
wrote:
> Hi,
>
> The reason we don't use drm_hdcp is because our policy is to do hdcp
> verification using PSP/HW (onboard secure processor).
i915 also uses hw to auth, we still use the parts from drm_hdcp ...
Did you actually look at what's in there? It's essentially just shared
defines and data structures from the standard, plus a few minimal
helpers to en/decode some bits. Just from a quick read the entire
patch very much looks like midlayer everywhere design that we
discussed back when DC landed ...
-Daniel
> Bhawan
>
> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
>> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
>>> Hi,
>>>
>>> Static analysis with Coverity has detected a potential issue with
>>> function validate_bksv in
>>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
>>> commit:
>>>
>>> commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
>>> Author: Bhawanpreet Lakha
>>> Date: Tue Aug 6 17:52:01 2019 -0400
>>>
>>> drm/amd/display: Add HDCP module
>> I think the real question here is ... why is this not using drm_hdcp?
>> -Daniel
>>
>>> The analysis is as follows:
>>>
>>>28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp
>>> *hdcp)
>>>29 {
>>>
>>> CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
>>>
>>> 1. overrun-local:
>>> Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
>>> (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
>>>
>>>30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
>>>31uint8_t count = 0;
>>>32
>>>33while (n) {
>>>34count++;
>>>35n &= (n - 1);
>>>36}
>>>
>>> hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
>>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
>>>
>>> struct mod_hdcp_message_hdcp1 {
>>> uint8_t an[8];
>>> uint8_t aksv[5];
>>> uint8_t ainfo;
>>> uint8_t bksv[5];
>>> uint16_tr0p;
>>> uint8_t bcaps;
>>> uint16_tbstatus;
>>> uint8_t ksvlist[635];
>>> uint16_tksvlist_size;
>>> uint8_t vp[20];
>>>
>>> uint16_tbinfo_dp;
>>> };
>>>
>>> variable n is going to contain the contains of r0p and bcaps. I'm not
>>> sure if that is intentional. If not, then the count is going to be
>>> incorrect if these are non-zero.
>>>
>>> Colin
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14
On 2019-11-01 9:37 p.m., Wu, Hersen wrote:
>
> Reviewed-by: Hersen Wu
>
>
>
> -Original Message-
> From: Liu, Zhan
> Sent: Friday, November 1, 2019 9:35 PM
> To: Wu, Hersen ; amd-gfx@lists.freedesktop.org;
> Kazlauskas, Nicholas ; Lakha, Bhawanpreet
> ; Li, Roman ; Siqueira, Rodrigo
> ; Wentland, Harry ; Zuo,
> Jerry
> Cc: Yeh, Eagle ; Lazare, Jordan
> Subject: RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for
> Navi14
>
> Thank you Hersen. Please check the updated patch:
>
> From: Liu, Zhan
> Sent: Friday, November 1, 2019 9:18 PM
> To: amd-gfx@lists.freedesktop.org; Kazlauskas, Nicholas
> ; Lakha, Bhawanpreet
> ; Li, Roman ; Liu, Zhan
> ; Siqueira, Rodrigo ; Wentland,
> Harry ; Wu, Hersen ; Zuo, Jerry
>
> Cc: Yeh, Eagle ; Lazare, Jordan
> Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for
> Navi14
>
> From: Zhan liu
> Date: Fri, 1 Nov 2019 21:10:17 -0400
> Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for
> Navi14
>
> [Why]
> Navi10 has 6 PHY, but Navi14 only has 5 PHY, that is because there is no
> ENGINE_ID_DIGD in Navi14. Without this patch, many HDMI related issues (e.g.
> HDMI S3 resume failure, HDMI pink screen on boot) will be observed.
Are we sure it's always DIGD that's missing on Navi14? It just seems odd
that it's not the last one or that it's not harvested (i.e. potentially
being any one that's missing).
Harry
>
> [How]
> If eng_id is larger than ENGINE_ID_DIGD, then add eng_id by 1.
>
> Signed-off-by: Zhan liu
> ---
> drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 3 +++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> index 924c2e303588..cf886483e380 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> @@ -1152,6 +1152,11 @@ struct stream_encoder *dcn20_stream_encoder_create(
> if (!enc1)
> return NULL;
>
> + if (ASIC_REV_IS_NAVI14_M(ctx->asic_id.hw_internal_rev)) {
> + if (eng_id >= ENGINE_ID_DIGD)
> + eng_id++;
> + }
> +
> dcn20_stream_encoder_construct(enc1, ctx, ctx->dc_bios, eng_id,
> _enc_regs[eng_id],
> _shift, _mask);
> --
> 2.21.0
>
>> -Original Message-
>> From: Wu, Hersen
>> Sent: 2019/November/01, Friday 9:23 PM
>> To: Liu, Zhan ; amd-gfx@lists.freedesktop.org;
>> Kazlauskas, Nicholas ; Lakha, Bhawanpreet
>> ; Li, Roman ; Siqueira,
>> Rodrigo ; Wentland, Harry
>> ; Zuo, Jerry
>> Cc: Yeh, Eagle ; Lazare, Jordan
>>
>> Subject: RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition
>> check for Navi14
>>
>> Hi Zhan,
>>
>> The function is shared by NV10,12,14.
>>
>> Please add ASIC ID check for the DIG D skip.
>>
>> Thanks!
>> Hersen
>>
>>
>> -Original Message-
>> From: Liu, Zhan
>> Sent: Friday, November 1, 2019 9:18 PM
>> To: amd-gfx@lists.freedesktop.org; Kazlauskas, Nicholas
>> ; Lakha, Bhawanpreet
>> ; Li, Roman ; Liu, Zhan
>> ; Siqueira, Rodrigo ;
>> Wentland, Harry ; Wu, Hersen
>> ; Zuo, Jerry
>> Cc: Yeh, Eagle ; Lazare, Jordan
>>
>> Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check
>> for Navi14
>>
>> From: Zhan liu
>> Date: Fri, 1 Nov 2019 21:10:17 -0400
>> Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check
>> for Navi14
>>
>> [Why]
>> Navi10 has 6 PHY, but Navi14 only has 5 PHY, that is because there is
>> no ENGINE_ID_DIGD in Navi14. Without this patch, many HDMI related
>> issues (e.g. HDMI S3 resume failure, HDMI pink screen on boot) will be
>> observed.
>>
>> [How]
>> If eng_id is larger than ENGINE_ID_DIGD, then add eng_id by 1.
>>
>> Signed-off-by: Zhan liu
>> ---
>> drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
>> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
>> index 924c2e303588..cf886483e380 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
>> @@ -1152,6 +1152,9 @@ struct stream_encoder
>> *dcn20_stream_encoder_create(
>> if (!enc1)
>> return NULL;
>>
>> + if (eng_id >= ENGINE_ID_DIGD)
>> + eng_id++;
>> +
>> dcn20_stream_encoder_construct(enc1, ctx, ctx->dc_bios, eng_id,
>> _enc_regs[eng_id],
>> _shift, _mask);
>> --
>> 2.21.0
>>
>> ___
>>
>> amd-gfx mailing list
>>
>> amd-gfx@lists.freedesktop.org
>>
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
Re: [PATCH v2 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6
On Fri, 01 Nov 2019, "Jerry (Fangzhi) Zuo" wrote:
> DP 1.4 edid corruption test requires source DUT to write calculated
> CRC, not the corrupted CRC from reference sink.
>
> Return the calculated CRC back, and initiate the required sequence.
>
> -v2: Have separate routine for returning real CRC
>
> Signed-off-by: Jerry (Fangzhi) Zuo
> ---
> drivers/gpu/drm/drm_dp_helper.c | 36
> drivers/gpu/drm/drm_edid.c | 14 ++
> include/drm/drm_connector.h | 7 +++
> include/drm/drm_dp_helper.h | 3 +++
> 4 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index ffc68d305afe..75dbd30c62a7 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
> }
> EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>
> +/**
> + * drm_dp_send_bad_edid_checksum() - send back real edid checksum value
> + * @aux: DisplayPort AUX channel
> + * @bad_edid_checksum: real edid checksum for the last block
> + *
> + * Returns true on success
> + */
> +bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
> +u8 bad_edid_checksum)
> +{
> +u8 link_edid_read = 0, auto_test_req = 0;
> +u8 test_resp = 0;
> +
> +drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req,
> 1);
> +auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
> +
> +drm_dp_dpcd_read(aux, DP_TEST_REQUEST, _edid_read, 1);
> +link_edid_read &= DP_TEST_LINK_EDID_READ;
> +
> +if (!auto_test_req || !link_edid_read) {
> +DRM_DEBUG_KMS("Source DUT does not support
> TEST_EDID_READ\n");
> +return false;
> +}
> +
> +drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req,
> 1);
> +
> +/* send back checksum for the last edid extension block data */
> +drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, _edid_checksum, 1);
> +
> +test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
> +drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, _resp, 1);
> +
> +return true;
> +}
> +EXPORT_SYMBOL(drm_dp_send_bad_edid_checksum);
> +
> /**
> * drm_dp_link_probe() - probe a DisplayPort link for capabilities
> * @aux: DisplayPort AUX channel
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 82a4ceed3fcf..0598314e3f46 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1354,6 +1354,17 @@ static int drm_edid_block_checksum(const u8 *raw_edid)
> return csum;
> }
>
> +static int drm_edid_block_real_checksum(const u8 *raw_edid)
> +{
> + int i;
> + u8 csum = 0;
> +
> + for (i = 0; i < EDID_LENGTH - 1; i++)
> + csum += raw_edid[i];
> +
> + return (0x100 - csum);
Now you have two functions that have the loop to calculate checksums,
which is not at all what I tried to tell you to do.
I tried to suggest something like this:
static int drm_edid_block_checksum(const u8 *raw_edid)
{
int i;
u8 csum = 0;
for (i = 0; i < EDID_LENGTH - 1; i++)
csum += raw_edid[i];
return 0x100 - csum;
}
static int drm_edid_block_checksum_diff(const u8 *raw_edid)
{
u8 csum = drm_edid_block_checksum(raw_edid) + raw_edid[EDID_LENGTH - 1];
return csum;
}
Alternatively, you could have just the function to calculate the
checksum, and then the check is comparing the calculated checksum
against the checksum in the EDID.
BR,
Jani.
> +}
> +
> static bool drm_edid_is_zero(const u8 *in_edid, int length)
> {
> if (memchr_inv(in_edid, 0, length))
> @@ -1572,6 +1583,9 @@ static void connector_bad_edid(struct drm_connector
> *connector,
> prefix, DUMP_PREFIX_NONE, 16, 1,
> block, EDID_LENGTH, false);
> }
> +
> + /* Calculate real checksum for the last edid extension block data */
> + connector->bad_edid_checksum = drm_edid_block_real_checksum(edid +
> edid[0x7e] * EDID_LENGTH);
> }
>
> /* Get override or firmware EDID */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..8442461542b9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1345,6 +1345,13 @@ struct drm_connector {
>* rev1.1 4.2.2.6
>*/
> bool edid_corrupt;
> + /**
> + * @bad_edid_checksum: real edid checksum value for corrupted edid
> block.
> + * Required in Displayport 1.4 compliance testing
> + * rev1.1 4.2.2.6
> + */
> +uint8_t bad_edid_checksum;
> +
>
> /** @debugfs_entry: debugfs directory for this connector */
> struct dentry *debugfs_entry;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 5a795075d5da..2a7e54bebb18 100644
> ---
[PATCH 0/7] fix various gcc warnings
This patch set fixes various gcc warnings. yu kuai (7): drm/amdgpu: remove 4 set but not used variable in amdgpu_atombios_get_connector_info_from_object_table drm/amdgpu: add function parameter description in 'amdgpu_device_set_cg_state' drm/amdgpu: add function parameter description in 'amdgpu_gart_bind' drm/amdgpu: remove set but not used variable 'dig_connector' drm/amdgpu: remove set but not used variable 'dig' drm/amdgpu: remove always false comparison in 'amdgpu_atombios_i2c_process_i2c_ch' drm/amdgpu: remove set but not used variable 'mc_shared_chmap' drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 20 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 1 + drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 5 - drivers/gpu/drm/amd/amdgpu/atombios_i2c.c| 5 - drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 3 +-- 6 files changed, 5 insertions(+), 30 deletions(-) -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 7/7] drm/amdgpu: remove set but not used variable 'mc_shared_chmap'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c: In function
‘gfx_v8_0_gpu_early_init’:
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:1713:6: warning: variable
‘mc_shared_chmap’ set but not used [-Wunused-but-set-variable]
Fixes: 0bde3a95eaa9 ("drm/amdgpu: split gfx8 gpu init into sw and hw parts")
Signed-off-by: yu kuai
---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index e4c645d..80b7958 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -1710,7 +1710,7 @@ static int gfx_v8_0_do_edc_gpr_workarounds(struct
amdgpu_device *adev)
static int gfx_v8_0_gpu_early_init(struct amdgpu_device *adev)
{
u32 gb_addr_config;
- u32 mc_shared_chmap, mc_arb_ramcfg;
+ u32 mc_arb_ramcfg;
u32 dimm00_addr_map, dimm01_addr_map, dimm10_addr_map, dimm11_addr_map;
u32 tmp;
int ret;
@@ -1850,7 +1850,6 @@ static int gfx_v8_0_gpu_early_init(struct amdgpu_device
*adev)
break;
}
- mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);
adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/7] drm/amdgpu: add function parameter description in 'amdgpu_device_set_cg_state'
Fixes gcc warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:1954: warning: Function
parameter or member 'state' not described in 'amdgpu_device_set_cg_state'
Fixes: e3ecdffac9cc ("drm/amdgpu: add documentation for amdgpu_device.c")
Signed-off-by: yu kuai
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b4fb6d8..26c43e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1941,6 +1941,7 @@ static bool amdgpu_device_check_vram_lost(struct
amdgpu_device *adev)
* amdgpu_device_set_cg_state - set clockgating for amdgpu device
*
* @adev: amdgpu_device pointer
+ * @state: clockgating state (gate or ungate)
*
* The list of all the hardware IPs that make up the asic is walked and the
* set_clockgating_state callbacks are run.
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 6/7] drm/amdgpu: remove always false comparison in 'amdgpu_atombios_i2c_process_i2c_ch'
Fixes gcc '-Wtype-limits' warning:
drivers/gpu/drm/amd/amdgpu/atombios_i2c.c: In function
‘amdgpu_atombios_i2c_process_i2c_ch’:
drivers/gpu/drm/amd/amdgpu/atombios_i2c.c:79:11: warning: comparison is
always false due to limited range of data type [-Wtype-limits]
'num' is 'u8', so it will never be greater than 'TOM_MAX_HW_I2C_READ',
which is defined as 255. Therefore, the comparison can be removed.
Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Signed-off-by: yu kuai
---
drivers/gpu/drm/amd/amdgpu/atombios_i2c.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
b/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
index 980c363..b4cc7c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
@@ -76,11 +76,6 @@ static int amdgpu_atombios_i2c_process_i2c_ch(struct
amdgpu_i2c_chan *chan,
}
args.lpI2CDataOut = cpu_to_le16(out);
} else {
- if (num > ATOM_MAX_HW_I2C_READ) {
- DRM_ERROR("hw i2c: tried to read too many bytes (%d vs
255)\n", num);
- r = -EINVAL;
- goto done;
- }
args.ucRegIndex = 0;
args.lpI2CDataOut = 0;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/7] drm/amdgpu: add function parameter description in 'amdgpu_gart_bind'
Fixes gcc warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c:313: warning: Function
parameter or member 'flags' not described in 'amdgpu_gart_bind'
Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Signed-off-by: yu kuai
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 19705e3..e01e681 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -302,6 +302,7 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t
offset,
* @pages: number of pages to bind
* @pagelist: pages to bind
* @dma_addr: DMA addresses of pages
+ * @flags: page table entry flags
*
* Binds the requested pages to the gart page table
* (all asics).
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 5/7] drm/amdgpu: remove set but not used variable 'dig'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/amd/amdgpu/atombios_dp.c: In function
‘amdgpu_atombios_dp_link_train’:
drivers/gpu/drm/amd/amdgpu/atombios_dp.c:716:34: warning: variable ‘dig’
set but not used [-Wunused-but-set-variable]
Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Signed-off-by: yu kuai
---
drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
index 9426530..ea702a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
@@ -710,7 +710,6 @@ void amdgpu_atombios_dp_link_train(struct drm_encoder
*encoder,
struct drm_device *dev = encoder->dev;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder);
- struct amdgpu_encoder_atom_dig *dig;
struct amdgpu_connector *amdgpu_connector;
struct amdgpu_connector_atom_dig *dig_connector;
struct amdgpu_atombios_dp_link_train_info dp_info;
@@ -718,7 +717,6 @@ void amdgpu_atombios_dp_link_train(struct drm_encoder
*encoder,
if (!amdgpu_encoder->enc_priv)
return;
- dig = amdgpu_encoder->enc_priv;
amdgpu_connector = to_amdgpu_connector(connector);
if (!amdgpu_connector->con_priv)
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/7] drm/amdgpu: remove set but not used variable 'dig_connector'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/amd/amdgpu/atombios_dp.c: In function
‘amdgpu_atombios_dp_get_panel_mode’:
drivers/gpu/drm/amd/amdgpu/atombios_dp.c:364:36: warning: variable
‘dig_connector’ set but not used [-Wunused-but-set-variable]
It is never used, so can be removed.
Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Signed-off-by: yu kuai
---
drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
index 6858cde..9426530 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
@@ -361,7 +361,6 @@ int amdgpu_atombios_dp_get_panel_mode(struct drm_encoder
*encoder,
struct drm_connector *connector)
{
struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(connector);
- struct amdgpu_connector_atom_dig *dig_connector;
int panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
u16 dp_bridge =
amdgpu_connector_encoder_get_dp_bridge_encoder_id(connector);
u8 tmp;
@@ -369,8 +368,6 @@ int amdgpu_atombios_dp_get_panel_mode(struct drm_encoder
*encoder,
if (!amdgpu_connector->con_priv)
return panel_mode;
- dig_connector = amdgpu_connector->con_priv;
-
if (dp_bridge != ENCODER_OBJECT_ID_NONE) {
/* DP bridge chips */
if (drm_dp_dpcd_readb(_connector->ddc_bus->aux,
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/7] drm/amdgpu: remove 4 set but not used variable in amdgpu_atombios_get_connector_info_from_object_table
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c: In function
'amdgpu_atombios_get_connector_info_from_object_table':
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:376:26: warning: variable
'grph_obj_num' set but not used [-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:376:13: warning: variable
'grph_obj_id' set but not used [-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:341:37: warning: variable
'con_obj_type' set but not used [-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:341:24: warning: variable
'con_obj_num' set but not used [-Wunused-but-set-variable]
They are never used, so can be removed.
Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Signed-off-by: yu kuai
---
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 19 ++-
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index 72232fc..be6d0cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -338,17 +338,9 @@ bool
amdgpu_atombios_get_connector_info_from_object_table(struct amdgpu_device *
path_size += le16_to_cpu(path->usSize);
if (device_support & le16_to_cpu(path->usDeviceTag)) {
- uint8_t con_obj_id, con_obj_num, con_obj_type;
-
- con_obj_id =
+ uint8_t con_obj_id =
(le16_to_cpu(path->usConnObjectId) & OBJECT_ID_MASK)
>> OBJECT_ID_SHIFT;
- con_obj_num =
- (le16_to_cpu(path->usConnObjectId) & ENUM_ID_MASK)
- >> ENUM_ID_SHIFT;
- con_obj_type =
- (le16_to_cpu(path->usConnObjectId) &
-OBJECT_TYPE_MASK) >> OBJECT_TYPE_SHIFT;
/* Skip TV/CV support */
if ((le16_to_cpu(path->usDeviceTag) ==
@@ -373,14 +365,7 @@ bool
amdgpu_atombios_get_connector_info_from_object_table(struct amdgpu_device *
router.ddc_valid = false;
router.cd_valid = false;
for (j = 0; j < ((le16_to_cpu(path->usSize) - 8) / 2);
j++) {
- uint8_t grph_obj_id, grph_obj_num,
grph_obj_type;
-
- grph_obj_id =
- (le16_to_cpu(path->usGraphicObjIds[j]) &
-OBJECT_ID_MASK) >> OBJECT_ID_SHIFT;
- grph_obj_num =
- (le16_to_cpu(path->usGraphicObjIds[j]) &
-ENUM_ID_MASK) >> ENUM_ID_SHIFT;
+ uint8_t grph_obj_type=
grph_obj_type =
(le16_to_cpu(path->usGraphicObjIds[j]) &
OBJECT_TYPE_MASK) >> OBJECT_TYPE_SHIFT;
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm/sched: Fix passing zero to 'PTR_ERR' warning
Reviewed-by: Christian König
Am 04.11.19 um 04:37 schrieb Deng, Emily:
Reviewed-by: Emily Deng
-Original Message-
From: amd-gfx On Behalf Of
Andrey Grodzovsky
Sent: Wednesday, October 30, 2019 2:08 AM
To: dan.carpen...@oracle.com
Cc: Grodzovsky, Andrey ; amd-
g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH v2] drm/sched: Fix passing zero to 'PTR_ERR' warning
Fix a static code checker warning.
v2: Drop PTR_ERR_OR_ZERO.
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/scheduler/sched_main.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index f39b97e..dba4390 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -496,8 +496,10 @@ void drm_sched_resubmit_jobs(struct
drm_gpu_scheduler *sched)
fence = sched->ops->run_job(s_job);
if (IS_ERR_OR_NULL(fence)) {
+ if (IS_ERR(fence))
+ dma_fence_set_error(_fence->finished,
PTR_ERR(fence));
+
s_job->s_fence->parent = NULL;
- dma_fence_set_error(_fence->finished,
PTR_ERR(fence));
} else {
s_job->s_fence->parent = fence;
}
@@ -741,8 +743,9 @@ static int drm_sched_main(void *param)
r);
dma_fence_put(fence);
} else {
+ if (IS_ERR(fence))
+ dma_fence_set_error(_fence->finished,
PTR_ERR(fence));
- dma_fence_set_error(_fence->finished,
PTR_ERR(fence));
drm_sched_process_job(NULL, _job->cb);
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm/amdgpu: Need to free discovery memory
Please use 'drm/amdgpu/discovery: ' prefix in commit message to let us easily track all discovery-releated changes. Actually please don't. The prefix is to denote which driver/code base is changed. IP discovery is just a feature and not limited to a certain code base inside the driver. Mentioning "discovery memory" in the commit message should be fine already. Regards, Christian. Am 04.11.19 um 05:45 schrieb Deng, Emily: Thanks, done. Best wishes Emily Deng -Original Message- From: Yuan, Xiaojie Sent: Monday, November 4, 2019 11:41 AM To: Deng, Emily ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH v2] drm/amdgpu: Need to free discovery memory Please use 'drm/amdgpu/discovery: ' prefix in commit message to let us easily track all discovery-releated changes. Other than this, patch is Reviewed-by: Xiaojie Yuan BR, Xiaojie From: amd-gfx on behalf of Emily Deng Sent: Monday, November 4, 2019 11:03 AM To: amd-gfx@lists.freedesktop.org Cc: Deng, Emily Subject: [PATCH v2] drm/amdgpu: Need to free discovery memory When unloading driver, need to free discovery memory. Signed-off-by: Emily Deng --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 28b09f6..7cbe6d9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -2106,9 +2106,6 @@ void amdgpu_ttm_late_init(struct amdgpu_device *adev) void *stolen_vga_buf; /* return the VGA stolen memory (if any) back to VRAM */ amdgpu_bo_free_kernel(>stolen_vga_memory, NULL, _vga_buf); - - /* return the IP Discovery TMR memory back to VRAM */ - amdgpu_bo_free_kernel(>discovery_memory, NULL, NULL); } /** @@ -2121,7 +2118,10 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev) amdgpu_ttm_debugfs_fini(adev); amdgpu_ttm_training_reserve_vram_fini(adev); + /* return the IP Discovery TMR memory back to VRAM */ + amdgpu_bo_free_kernel(>discovery_memory, NULL, NULL); amdgpu_ttm_fw_reserve_vram_fini(adev); + if (adev->mman.aper_base_kaddr) iounmap(adev->mman.aper_base_kaddr); adev->mman.aper_base_kaddr = NULL; -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: drm/amd/display: Add HDCP module - static analysis bug report
On Wed, Oct 9, 2019 at 10:58 PM Daniel Vetter wrote:
> On Wed, Oct 9, 2019 at 10:46 PM Lakha, Bhawanpreet
> wrote:
> >
> > I misunderstood and was talking about the ksv validation specifically
> > (usage of drm_hdcp_check_ksvs_revoked()).
>
> Hm for that specifically I think you want to do both, i.e. both
> consult your psp, but also check for revoked ksvs with the core
> helper. At least on some platforms only the core helper might have the
> updated revoke list.
>
> > For the defines I will create patches to use drm_hdcp where it is usable.
>
> Thanks a lot. Ime once we have shared definitions it's much easier to
> also share some helpers, where it makes sense.
>
> Aside I think the hdcp code could also use a bit of demidlayering. At
> least I'm not understanding why you add a 2nd abstraction layer for
> i2c/dpcd, dm_helper already has that. That seems like one abstraction
> layer too much.
I haven't seen anything fly by or in the latest pull request ... you
folks still working on this or more put on the "maybe, probably never"
pile?
-Daniel
> -Daniel
>
> >
> >
> > Bhawan
> >
> > On 2019-10-09 2:43 p.m., Daniel Vetter wrote:
> > > On Wed, Oct 9, 2019 at 8:23 PM Lakha, Bhawanpreet
> > > wrote:
> > >> Hi,
> > >>
> > >> The reason we don't use drm_hdcp is because our policy is to do hdcp
> > >> verification using PSP/HW (onboard secure processor).
> > > i915 also uses hw to auth, we still use the parts from drm_hdcp ...
> > > Did you actually look at what's in there? It's essentially just shared
> > > defines and data structures from the standard, plus a few minimal
> > > helpers to en/decode some bits. Just from a quick read the entire
> > > patch very much looks like midlayer everywhere design that we
> > > discussed back when DC landed ...
> > > -Daniel
> > >
> > >> Bhawan
> > >>
> > >> On 2019-10-09 12:32 p.m., Daniel Vetter wrote:
> > >>> On Thu, Oct 03, 2019 at 11:08:03PM +0100, Colin Ian King wrote:
> > Hi,
> >
> > Static analysis with Coverity has detected a potential issue with
> > function validate_bksv in
> > drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c with recent
> > commit:
> >
> > commit ed9d8e2bcb003ec94658cafe9b1bb3960e2139ec
> > Author: Bhawanpreet Lakha
> > Date: Tue Aug 6 17:52:01 2019 -0400
> >
> > drm/amd/display: Add HDCP module
> > >>> I think the real question here is ... why is this not using drm_hdcp?
> > >>> -Daniel
> > >>>
> > The analysis is as follows:
> >
> > 28 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp
> > *hdcp)
> > 29 {
> >
> > CID 89852 (#1 of 1): Out-of-bounds read (OVERRUN)
> >
> > 1. overrun-local:
> > Overrunning array of 5 bytes at byte offset 7 by dereferencing pointer
> > (uint64_t *)hdcp->auth.msg.hdcp1.bksv.
> >
> > 30uint64_t n = *(uint64_t *)hdcp->auth.msg.hdcp1.bksv;
> > 31uint8_t count = 0;
> > 32
> > 33while (n) {
> > 34count++;
> > 35n &= (n - 1);
> > 36}
> >
> > hdcp->auth.msg.hdcp1.bksv is an array of 5 uint8_t as defined in
> > drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h as follows:
> >
> > struct mod_hdcp_message_hdcp1 {
> > uint8_t an[8];
> > uint8_t aksv[5];
> > uint8_t ainfo;
> > uint8_t bksv[5];
> > uint16_tr0p;
> > uint8_t bcaps;
> > uint16_tbstatus;
> > uint8_t ksvlist[635];
> > uint16_tksvlist_size;
> > uint8_t vp[20];
> >
> > uint16_tbinfo_dp;
> > };
> >
> > variable n is going to contain the contains of r0p and bcaps. I'm not
> > sure if that is intentional. If not, then the count is going to be
> > incorrect if these are non-zero.
> >
> > Colin
> > >> ___
> > >> dri-devel mailing list
> > >> dri-de...@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > >
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: disallow direct upload save restore list from gfx driver
Direct uploading save/restore list via mmio register writes breaks the security
policy. Instead, the driver should pass s list to psp.
For all the ASICs that use rlc v2_1 headers, the driver actually upload s list
twice, in non-psp ucode front door loading phase and gfx pg initialization
phase.
The latter is not allowed.
VG12 is the only exception where the driver still keeps legacy approach for S
list uploading. In theory, this can be elimnated if we have valid srcntl ucode
for VG12.
Change-Id: I8cc8e0126f746aae43b9114e05bc111ee7b23531
Signed-off-by: Hawking Zhang
---
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 0525fc6..d14c4d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -2725,7 +2725,8 @@ static void gfx_v9_0_init_pg(struct amdgpu_device *adev)
* And it's needed by gfxoff feature.
*/
if (adev->gfx.rlc.is_rlc_v2_1) {
- gfx_v9_1_init_rlc_save_restore_list(adev);
+ if (adev->asic_type == CHIP_VEGA12)
+ gfx_v9_1_init_rlc_save_restore_list(adev);
gfx_v9_0_enable_save_restore_machine(adev);
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
