Re: [PATCH v2 1/2] Revert "drm/amd/display: Remove v_startup workaround for dcn3+"
[AMD Official Use Only - General]
Series is:
Reviewed-By: Fangzhi Zuo
发件人: Mahfooz, Hamza
发送时间: 星期四, 八月 31, 2023 3:39:04 下午
收件人: amd-...@lists.freedesktop.org
抄送: Zuo, Jerry ; Mahfooz, Hamza ;
sta...@vger.kernel.org ; Wentland, Harry
; Li, Sun peng (Leo) ; Siqueira,
Rodrigo ; Deucher, Alexander
; Koenig, Christian ; Pan,
Xinhui ; David Airlie ; Daniel Vetter
; Lei, Jun ; Pillai, Aurabindo
; Kazlauskas, Nicholas ;
Liu, Wenjing ; Lee, Alvin ; Kim, Sung
joon ; Miess, Daniel ; Teeger, Gabe
; dri-devel@lists.freedesktop.org
; linux-ker...@vger.kernel.org
主题: [PATCH v2 1/2] Revert "drm/amd/display: Remove v_startup workaround for
dcn3+"
This reverts commit 3a31e8b89b7240d9a17ace8a1ed050bdcb560f9e.
We still need to call dcn20_adjust_freesync_v_startup() for older DCN3+
ASICs otherwise it can cause DP to HDMI 2.1 PCONs to fail to light up.
Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2809
Signed-off-by: Hamza Mahfooz
---
.../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 24 ---
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index 0989a0152ae8..1bfdf0271fdf 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -1099,6 +1099,10 @@ void dcn20_calculate_dlg_params(struct dc *dc,
context->res_ctx.pipe_ctx[i].plane_res.bw.dppclk_khz =
pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000;
context->res_ctx.pipe_ctx[i].pipe_dlg_param =
pipes[pipe_idx].pipe.dest;
+ if
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
+ dcn20_adjust_freesync_v_startup(
+ &context->res_ctx.pipe_ctx[i].stream->timing,
+
&context->res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
pipe_idx++;
}
@@ -1927,7 +1931,6 @@ static bool dcn20_validate_bandwidth_internal(struct dc
*dc, struct dc_state *co
int vlevel = 0;
int pipe_split_from[MAX_PIPES];
int pipe_cnt = 0;
- int i = 0;
display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count *
sizeof(display_e2e_pipe_params_st), GFP_ATOMIC);
DC_LOGGER_INIT(dc->ctx->logger);
@@ -1951,15 +1954,6 @@ static bool dcn20_validate_bandwidth_internal(struct dc
*dc, struct dc_state *co
dcn20_calculate_wm(dc, context, pipes, &pipe_cnt, pipe_split_from,
vlevel, fast_validate);
dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
- for (i = 0; i < dc->res_pool->pipe_count; i++) {
- if (!context->res_ctx.pipe_ctx[i].stream)
- continue;
- if
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
- dcn20_adjust_freesync_v_startup(
- &context->res_ctx.pipe_ctx[i].stream->timing,
-
&context->res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
- }
-
BW_VAL_TRACE_END_WATERMARKS();
goto validate_out;
@@ -2232,7 +2226,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc,
int vlevel = 0;
int pipe_split_from[MAX_PIPES];
int pipe_cnt = 0;
- int i = 0;
display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count *
sizeof(display_e2e_pipe_params_st), GFP_ATOMIC);
DC_LOGGER_INIT(dc->ctx->logger);
@@ -2261,15 +2254,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc,
dcn21_calculate_wm(dc, context, pipes, &pipe_cnt, pipe_split_from,
vlevel, fast_validate);
dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
- for (i = 0; i < dc->res_pool->pipe_count; i++) {
- if (!context->res_ctx.pipe_ctx[i].stream)
- continue;
- if
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
- dcn20_adjust_freesync_v_startup(
- &context->res_ctx.pipe_ctx[i].stream->timing,
-
&context->res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
- }
-
BW_VAL_TRACE_END_WATERMARKS();
goto validate_out;
--
2.41.0
RE: [PATCH 3/3] drm/amd/display: Solve mst monitors blank out problem after resume
[AMD Official Use Only - AMD Internal Distribution Only]
Reviewed-by: Fangzhi Zuo
> -Original Message-
> From: Wayne Lin
> Sent: Wednesday, June 26, 2024 4:48 AM
> To: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: ly...@redhat.com; jani.nik...@intel.com; imre.d...@intel.com;
> dan...@ffwll.ch; Wentland, Harry ; Zuo, Jerry
> ; Lin, Wayne
> Subject: [PATCH 3/3] drm/amd/display: Solve mst monitors blank out problem
> after resume
>
> [Why]
> In dm resume, we firstly restore dc state and do the mst resume for topology
> probing thereafter. If we change dpcd DP_MSTM_CTRL value after LT in mst
> reume, it will cause light up problem on the hub.
>
> [How]
> Revert the commit 202dc359adda ("drm/amd/display: Defer handling mst up
> request in resume"). And adjust the reason to trigger dc_link_detect by
> DETECT_REASON_RESUMEFROMS3S4.
>
> Fixes: 202dc359adda ("drm/amd/display: Defer handling mst up request in
> resume")
> Signed-off-by: Wayne Lin
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> 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 c64cc2378a94..b01452eb0981 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2569,6 +2569,7 @@ static void resume_mst_branch_status(struct
> drm_dp_mst_topology_mgr *mgr)
>
> ret = drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL,
>DP_MST_EN |
> + DP_UP_REQ_EN |
>DP_UPSTREAM_IS_SRC);
> if (ret < 0) {
> drm_dbg_kms(mgr->dev, "mst write failed - undocked during
> suspend?\n"); @@ -3171,7 +3172,7 @@ static int dm_resume(void *handle)
> } else {
> mutex_lock(&dm->dc_lock);
> dc_exit_ips_for_hw_access(dm->dc);
> - dc_link_detect(aconnector->dc_link,
> DETECT_REASON_HPD);
> + dc_link_detect(aconnector->dc_link,
> DETECT_REASON_RESUMEFROMS3S4);
> mutex_unlock(&dm->dc_lock);
> }
>
> --
> 2.37.3
RE: [PATCH v2] drm/amd/display: Only define DP 2.0 symbols if not already defined
[AMD Official Use Only]
> -Original Message-
> From: Harry Wentland
> Sent: September 28, 2021 1:08 PM
> To: Deucher, Alexander ; amd-
> g...@lists.freedesktop.org; Zuo, Jerry
> Cc: jani.nik...@intel.com; Li, Sun peng (Leo) ;
> nat...@kernel.org; intel-...@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org; ville.syrj...@linux.intel.com;
> manasi.d.nav...@intel.com; Koenig, Christian ;
> Pan, Xinhui ; s...@canb.auug.org.au; linux-
> n...@vger.kernel.org; airl...@gmail.com; daniel.vet...@ffwll.ch; Wentland,
> Harry
> Subject: [PATCH v2] drm/amd/display: Only define DP 2.0 symbols if not
> already defined
>
> [Why]
> For some reason we're defining DP 2.0 definitions inside our driver. Now that
> patches to introduce relevant definitions are slated to be merged into drm-
> next this is causing conflicts.
>
> In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c:33:
> In file included
> from ./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:70:
> In file included
> from ./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu_mode.h:36:
> ./include/drm/drm_dp_helper.h:1322:9: error:
> 'DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER' macro redefined [-
> Werror,-Wmacro-redefined]
> ^
> ./drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dp_types.h:881:9: note:
> previous definition is here
> ^
> 1 error generated.
>
> v2: Add one missing endif
>
> [How]
> Guard all display driver defines with #ifndef for now. Once we pull in the new
> definitions into amd-staging-drm-next we will follow up and drop definitions
> from our driver and provide follow-up header updates for any addition DP
> 2.0 definitions required by our driver.
>
> Signed-off-by: Harry Wentland
Reviewed-by: Fangzhi Zuo
> ---
> drivers/gpu/drm/amd/display/dc/dc_dp_types.h | 54
> ++--
> 1 file changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_dp_types.h
> b/drivers/gpu/drm/amd/display/dc/dc_dp_types.h
> index a5e798b5da79..9de86ff5ef1b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_dp_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_dp_types.h
> @@ -860,28 +860,72 @@ struct psr_caps {
> };
>
> #if defined(CONFIG_DRM_AMD_DC_DCN)
> +#ifndef DP_MAIN_LINK_CHANNEL_CODING_CAP
> #define DP_MAIN_LINK_CHANNEL_CODING_CAP 0x006
> +#endif
> +#ifndef DP_SINK_VIDEO_FALLBACK_FORMATS
> #define DP_SINK_VIDEO_FALLBACK_FORMATS 0x020
> +#endif
> +#ifndef DP_FEC_CAPABILITY_1
> #define DP_FEC_CAPABILITY_1 0x091
> +#endif
> +#ifndef DP_DFP_CAPABILITY_EXTENSION_SUPPORT
> #define DP_DFP_CAPABILITY_EXTENSION_SUPPORT 0x0A3
> +#endif
> +#ifndef DP_DSC_CONFIGURATION
> #define DP_DSC_CONFIGURATION 0x161
> +#endif
> +#ifndef DP_PHY_SQUARE_PATTERN
> #define DP_PHY_SQUARE_PATTERN0x249
> +#endif
> +#ifndef DP_128b_132b_SUPPORTED_LINK_RATES
> #define DP_128b_132b_SUPPORTED_LINK_RATES0x2215
> +#endif
> +#ifndef DP_128b_132b_TRAINING_AUX_RD_INTERVAL
> #define DP_128b_132b_TRAINING_AUX_RD_INTERVAL
> 0x2216
> +#endif
> +#ifndef DP_TEST_264BIT_CUSTOM_PATTERN_7_0
> #define DP_TEST_264BIT_CUSTOM_PATTERN_7_00X2230
> +#endif
> +#ifndef DP_TEST_264BIT_CUSTOM_PATTERN_263_256
> #define DP_TEST_264BIT_CUSTOM_PATTERN_263_256
> 0X2250
> +#endif
> +#ifndef DP_DSC_SUPPORT_AND_DECODER_COUNT
> #define DP_DSC_SUPPORT_AND_DECODER_COUNT 0x2260
> +#endif
> +#ifndef DP_DSC_MAX_SLICE_COUNT_AND_AGGREGATION_0
> #define DP_DSC_MAX_SLICE_COUNT_AND_AGGREGATION_0
> 0x2270
> -# define DP_DSC_DECODER_0_MAXIMUM_SLICE_COUNT_MASK (1 <<
> 0)
> -# define DP_DSC_DECODER_0_AGGREGATION_SUPPORT_MASK
> (0b111 << 1)
> -# define DP_DSC_DECODER_0_AGGREGATION_SUPPORT_SHIFT 1
> -# define DP_DSC_DECODER_COUNT_MASK (0b111 << 5)
> -# define DP_DSC_DECODER_COUNT_SHIFT 5
> +#endif
> +#ifndef DP_DSC_DECODER_0_MAXIMUM_SLICE_COUNT_MASK
> +#define DP_DSC_DECODER_0_MAXIMUM_SLICE_COUNT_MASK(1 <<
> 0)
> +#endif
> +#ifndef DP_DSC_DECODER_0_AGGREGATION_SUPPORT_MASK
> +#define DP_DSC_DECODER_0_AGGREGATION_SUPPORT_MASK
> (0b111 << 1)
> +#endif
> +#ifndef DP_DSC_DECODER_0_AGGREGATION_SUPPORT_SHIFT
> +#define DP_DSC_DECODER_0_AGGREGATION_SUPPORT_SHIFT 1
> +#endif
> +#ifndef DP_DSC_DECODER_COUNT_MASK
> +#define DP_DSC_DECODER_COUNT_MASK(0b111 << 5)
> +#endif
> +#ifndef DP_DSC_DECODER_COUNT_SHIFT
> +#define DP_DSC_DECODER_COUNT_SHIFT 5
> +#endif
> +#ifndef DP_MAIN_L
RE: [PATCH] drm/edid: Fix crash with zero/invalid EDID
[AMD Official Use Only]
Hi Ville:
Yhea, it is pretty old change from two years ago, and it is no long valid
anymore. Please simply drop it.
Regards,
Jerry
> -Original Message-
> From: Ville Syrjälä
> Sent: October 4, 2021 3:45 PM
> To: Douglas Anderson
> Cc: dri-devel@lists.freedesktop.org; ge...@linux-m68k.org;
> oliver.s...@intel.com; Daniel Vetter ; David Airlie
> ; Jani Nikula ; Linus Walleij
> ; Maarten Lankhorst
> ; Maxime Ripard
> ; Sam Ravnborg ; Thomas
> Zimmermann ; linux-ker...@vger.kernel.org; Zuo,
> Jerry ; Wentland, Harry ;
> Siqueira, Rodrigo
> Subject: Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID
>
> On Mon, Oct 04, 2021 at 09:21:27AM -0700, Douglas Anderson wrote:
> > In the commit bac9c2948224 ("drm/edid: Break out reading block 0 of
> > the EDID") I broke out reading the base block of the EDID to its own
> > function. Unfortunately, when I did that I messed up the handling when
> > drm_edid_is_zero() indicated that we had an EDID that was all 0x00 or
> > when we went through 4 loops and didn't get a valid EDID. Specifically
> > I needed to pass the broken EDID to connector_bad_edid() but now I was
> > passing an error-pointer.
> >
> > Let's re-jigger things so we can pass the bad EDID in properly.
> >
> > Fixes: bac9c2948224 ("drm/edid: Break out reading block 0 of the
> > EDID")
> > Reported-by: kernel test robot
> > Reported-by: Geert Uytterhoeven
> > Signed-off-by: Douglas Anderson
> > ---
> >
> > drivers/gpu/drm/drm_edid.c | 27 +++
> > 1 file changed, 11 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 9b19eee0e1b4..9c9463ec5465 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -1911,13 +1911,15 @@ int drm_add_override_edid_modes(struct
> > drm_connector *connector) }
> > EXPORT_SYMBOL(drm_add_override_edid_modes);
> >
> > -static struct edid *drm_do_get_edid_base_block(
> > +static struct edid *drm_do_get_edid_base_block(struct drm_connector
> > +*connector,
> > int (*get_edid_block)(void *data, u8 *buf, unsigned int block,
> > size_t len),
> > - void *data, bool *edid_corrupt, int *null_edid_counter)
> > + void *data)
> > {
> > - int i;
> > + int *null_edid_counter = connector ? &connector-
> >null_edid_counter : NULL;
> > + bool *edid_corrupt = connector ? &connector->edid_corrupt : NULL;
> > void *edid;
> > + int i;
> >
> > edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
> > if (edid == NULL)
> > @@ -1941,9 +1943,8 @@ static struct edid
> *drm_do_get_edid_base_block(
> > return edid;
> >
> > carp:
> > - kfree(edid);
> > - return ERR_PTR(-EINVAL);
> > -
> > + if (connector)
> > + connector_bad_edid(connector, edid, 1);
>
> BTW I believe connector_bad_edid() itself is broken since commit
> e11f5bd8228f ("drm: Add support for DP 1.4 Compliance edid corruption
> test"). Before we've even allocated the memory for the extension blocks
> that code now assumes edid[0x7e] is to be 100% trusted and goes and
> calculates the checksum on a block based on that. So that's likely going to be
> pointing somewhere beyond the base block into memory we've not even
> allocated. So anyone who wanted could craft a bogus EDID and maybe get
> something interesting to happen.
>
> Would be good if someone could fix that while at it. Or just revert the
> offending commit if there is no simple solution immediately in sight.
>
> The fact that we're parsing entirely untrustworthy crap in the kernel always
> worries me. Either we need super careful review of all relevant code, and/or
> we need to think about moving the parser out of the kernel.
> I was considering playing around with the usermode helper stuff. IIRC there
> is a way to embed the userspace binary into the kernel and just fire it up
> when needed. But so far it's been the usual -ENOTIME for me...
>
> --
> Ville Syrjälä
> Intel
RE: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID)
[AMD Official Use Only]
> -Original Message-
> From: Doug Anderson
> Sent: October 5, 2021 11:14 AM
> To: Zuo, Jerry
> Cc: Ville Syrjälä ; dri-
> de...@lists.freedesktop.org; ge...@linux-m68k.org; oliver.s...@intel.com;
> Daniel Vetter ; David Airlie ; Jani Nikula
> ; Linus Walleij ; Maarten
> Lankhorst ; Maxime Ripard
> ; Sam Ravnborg ; Thomas
> Zimmermann ; linux-ker...@vger.kernel.org;
> Wentland, Harry ; Siqueira, Rodrigo
> ; Kuogee Hsieh
> Subject: Re: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid:
> Fix crash with zero/invalid EDID)
>
> Hi,
>
> On Tue, Oct 5, 2021 at 6:33 AM Zuo, Jerry wrote:
> >
> > > BTW I believe connector_bad_edid() itself is broken since commit
> > > e11f5bd8228f ("drm: Add support for DP 1.4 Compliance edid
> > > corruption test"). Before we've even allocated the memory for the
> > > extension blocks that code now assumes edid[0x7e] is to be 100%
> > > trusted and goes and calculates the checksum on a block based on
> > > that. So that's likely going to be pointing somewhere beyond the
> > > base block into memory we've not even allocated. So anyone who
> > > wanted could craft a bogus EDID and maybe get something interesting to
> happen.
> > >
> > > Would be good if someone could fix that while at it. Or just revert
> > > the offending commit if there is no simple solution immediately in sight.
> > >
> > > The fact that we're parsing entirely untrustworthy crap in the
> > > kernel always worries me. Either we need super careful review of all
> > > relevant code, and/or we need to think about moving the parser out of
> the kernel.
> > > I was considering playing around with the usermode helper stuff.
> > > IIRC there is a way to embed the userspace binary into the kernel
> > > and just fire it up when needed. But so far it's been the usual -ENOTIME
> for me...
> > >
> > [AMD Official Use Only]
> >
> > Hi Ville:
> >
> > Yhea, it is pretty old change from two years ago, and it is no long
> > valid
> anymore. Please simply drop it.
> >
> > Regards,
> > Jerry
>
> I've cut out other bits from this email and changed the subject line since I
> think this is an issue unrelated to the one my original patch was fixing.
>
> I don't actually know a ton about DP compliance testing, but I attempted to
> try to be helpful and revert commit e11f5bd8228f ("drm:
> Add support for DP 1.4 Compliance edid corruption test"). It wasn't too hard
> to deal with the conflicts in the revert itself, but then things didn't
> compile
> because there are two places that use `real_edid_checksum` and that goes
> away if I revert the patch.
>
> I've made an attempt to fix the problem by just adding a bounds check.
> Perhaps you can see if that looks good to you:
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Fr%2F20211005081022.1.Ib059f9c23c2611cb5a9d760e7d0a700c1
> 295928d%40changeid&data=04%7C01%7CJerry.Zuo%40amd.com%7C90
> b948659454400cedd308d98812c339%7C3dd8961fe4884e608e11a82d994e183d
> %7C0%7C0%7C637690436453163864%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1
> 000&sdata=OtSngWlYyDc1NbNSgAeALqN3nF%2Bnw08nJ068cpAKZJk%3
> D&reserved=0
>
> -Doug
The patch used for DP1.4 compliance edid corruption test. Let me double check
if edid corruption test could be passed without the patch.
RE: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID)
[AMD Official Use Only]
> -Original Message-
> From: Wentland, Harry
> Sent: October 5, 2021 2:04 PM
> To: Zuo, Jerry ; Doug Anderson
>
> Cc: Ville Syrjälä ; dri-
> de...@lists.freedesktop.org; ge...@linux-m68k.org; oliver.s...@intel.com;
> Daniel Vetter ; David Airlie ; Jani Nikula
> ; Linus Walleij ; Maarten
> Lankhorst ; Maxime Ripard
> ; Sam Ravnborg ; Thomas
> Zimmermann ; linux-ker...@vger.kernel.org;
> Siqueira, Rodrigo ; Kuogee Hsieh
>
> Subject: Re: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid:
> Fix crash with zero/invalid EDID)
>
>
>
> On 2021-10-05 11:25, Zuo, Jerry wrote:
> > [AMD Official Use Only]
> >
> >> -Original Message-
> >> From: Doug Anderson
> >> Sent: October 5, 2021 11:14 AM
> >> To: Zuo, Jerry
> >> Cc: Ville Syrjälä ; dri-
> >> de...@lists.freedesktop.org; ge...@linux-m68k.org;
> >> oliver.s...@intel.com; Daniel Vetter ; David Airlie
> >> ; Jani Nikula ; Linus
> >> Walleij ; Maarten Lankhorst
> >> ; Maxime Ripard
> >> ; Sam Ravnborg ; Thomas
> >> Zimmermann ; linux-ker...@vger.kernel.org;
> >> Wentland, Harry ; Siqueira, Rodrigo
> >> ; Kuogee Hsieh
> >> Subject: Re: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid:
> >> Fix crash with zero/invalid EDID)
> >>
> >> Hi,
> >>
> >> On Tue, Oct 5, 2021 at 6:33 AM Zuo, Jerry wrote:
> >>>
> >>>> BTW I believe connector_bad_edid() itself is broken since commit
> >>>> e11f5bd8228f ("drm: Add support for DP 1.4 Compliance edid
> >>>> corruption test"). Before we've even allocated the memory for the
> >>>> extension blocks that code now assumes edid[0x7e] is to be 100%
> >>>> trusted and goes and calculates the checksum on a block based on
> >>>> that. So that's likely going to be pointing somewhere beyond the
> >>>> base block into memory we've not even allocated. So anyone who
> >>>> wanted could craft a bogus EDID and maybe get something interesting
> >>>> to
> >> happen.
> >>>>
> >>>> Would be good if someone could fix that while at it. Or just revert
> >>>> the offending commit if there is no simple solution immediately in sight.
> >>>>
> >>>> The fact that we're parsing entirely untrustworthy crap in the
> >>>> kernel always worries me. Either we need super careful review of
> >>>> all relevant code, and/or we need to think about moving the parser
> >>>> out of
> >> the kernel.
> >>>> I was considering playing around with the usermode helper stuff.
> >>>> IIRC there is a way to embed the userspace binary into the kernel
> >>>> and just fire it up when needed. But so far it's been the usual
> >>>> -ENOTIME
> >> for me...
> >>>>
> >>> [AMD Official Use Only]
> >>>
> >>> Hi Ville:
> >>>
> >>> Yhea, it is pretty old change from two years ago, and it is no
> >>> long valid
> >> anymore. Please simply drop it.
> >>>
> >>> Regards,
> >>> Jerry
> >>
> >> I've cut out other bits from this email and changed the subject line
> >> since I think this is an issue unrelated to the one my original patch was
> fixing.
> >>
> >> I don't actually know a ton about DP compliance testing, but I
> >> attempted to try to be helpful and revert commit e11f5bd8228f ("drm:
> >> Add support for DP 1.4 Compliance edid corruption test"). It wasn't
> >> too hard to deal with the conflicts in the revert itself, but then
> >> things didn't compile because there are two places that use
> >> `real_edid_checksum` and that goes away if I revert the patch.
> >>
> >> I've made an attempt to fix the problem by just adding a bounds check.
> >> Perhaps you can see if that looks good to you:
> >>
> >> https://lore.
> kernel.org%2Fr%2F20211005081022.1.Ib059f9c23c2611cb5a9d760e7d0a700c1
> >>
> 295928d%40changeid&data=04%7C01%7CJerry.Zuo%40amd.com%7C90
> >>
> b948659454400cedd308d98812c339%7C3dd8961fe4884e608e11a82d994e183d
> >> %7C0%7C0%7C637690436453163864%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIj
> >>
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1
> >>
> 000&sdata=OtSngWlYyDc1NbNSgAeALqN3nF%2Bnw08nJ068cpAKZJk%3
> >> D&reserved=0
> >>
> >> -Doug
> >
> > The patch used for DP1.4 compliance edid corruption test. Let me double
> check if edid corruption test could be passed without the patch.
> >
>
> Can you try the CTS test with Doug's fix?
>
> https://patchwork.freedesktop.org/patch/457537/
>
> Harry
Yes, I'll give a try on that.
RE: [PATCH 1/2] drm: Update MST First Link Slot Information Based on Encoding Format
[AMD Official Use Only]
> -Original Message-
> From: Lyude Paul
> Sent: August 30, 2021 4:00 PM
> To: Zuo, Jerry ; dri-devel@lists.freedesktop.org
> Cc: Wentland, Harry ; Kazlauskas, Nicholas
> ; Lin, Wayne
> Subject: Re: [PATCH 1/2] drm: Update MST First Link Slot Information Based
> on Encoding Format
>
> On Fri, 2021-08-27 at 19:43 -0400, Fangzhi Zuo wrote:
> > 8b/10b encoding format requires to reserve the first slot for
> > recording metadata. Real data transmission starts from the second
> > slot, with a total of available 63 slots available.
> >
> > In 128b/132b encoding format, metadata is transmitted separately in
> > LLCP packet before MTP. Real data transmission starts from the first
> > slot, with a total of 64 slots available.
> >
> > Signed-off-by: Fangzhi Zuo
> > ---
> > drivers/gpu/drm/drm_dp_mst_topology.c | 27
> > ---
> > include/drm/drm_dp_mst_helper.h | 9 +
> > 2 files changed, 29 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 86d13d6bc463..30544801d2b8 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -3370,7 +3370,7 @@ int drm_dp_update_payload_part1(struct
> > drm_dp_mst_topology_mgr *mgr)
> > struct drm_dp_payload req_payload;
> > struct drm_dp_mst_port *port;
> > int i, j;
> > - int cur_slots = 1;
> > + int cur_slots = mgr->start_slot;
> > bool skip;
> >
> > mutex_lock(&mgr->payload_lock); @@ -4323,7 +4323,7 @@ int
> > drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
> >
> > /* max. time slots - one slot for MTP header */
> > - if (num_slots > 63)
> > + if (num_slots > mgr->total_avail_slots)
> > return -ENOSPC;
> > return num_slots;
> > }
> > @@ -4335,7 +4335,7 @@ static int drm_dp_init_vcpi(struct
> > drm_dp_mst_topology_mgr *mgr,
> > int ret;
> >
> > /* max. time slots - one slot for MTP header */
> > - if (slots > 63)
> > + if (slots > mgr->total_avail_slots)
> > return -ENOSPC;
> >
> > vcpi->pbn = pbn;
> > @@ -4509,6 +4509,17 @@ int drm_dp_atomic_release_vcpi_slots(struct
> > drm_atomic_state *state,
> > }
> > EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots);
> >
> > +void drm_dp_mst_update_encoding_cap(struct
> drm_dp_mst_topology_mgr
> > +*mgr,
> > uint8_t link_encoding_cap)
> > +{
> > + if (link_encoding_cap == DP_CAP_ANSI_128B132B) {
> > + mgr->total_avail_slots = 64;
> > + mgr->start_slot = 0;
> > + }
> > + DRM_DEBUG_KMS("%s encoding format determined\n",
> > + (link_encoding_cap == DP_CAP_ANSI_128B132B) ?
> > "128b/132b" : "8b/10b");
> > +}
> > +EXPORT_SYMBOL(drm_dp_mst_update_encoding_cap);
> > +
>
> This seems to be missing kdocs, can you fix that?
>
> Also - I'm not convinced this is all of the work we have to do, as there's no
> locking taking place here in this function. If we're changing the number of
> available VCPI slots that we have, we need to be able to factor that into the
> atomic check logic which means that we can't rely on mgr->* for any kind of
> data that isn't guaranteed to remain consistent throughout the lifetime of the
> driver or topology. (Note that some of the old MST code didn't follow this
> logic, so I wouldn't be surprised if there's still exceptions to this we need
> to
> clean up).
>
> Note that I still expect we'll have to keep some sort of track of the current
> total slot count in the topology mgr, but that should be reflecting the
> currently programmed state and not be relied on from our atomic check.
>
Thanks Lyude for your comments.
Seems I should keep existing code to keep track of current slot status in mgr.
That information is getting updated each time when topology change detected.
That slot information saved in mgr is a sort of static, and could only be used
for debug purpose to track what is the current encoding format.
> IMHO - the correct way we should go about adding support for this is to add
> something into drm_dp_mst_topology_state and integrate this into the
> atomic check helpers.
The slot information should also b
RE: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders
Reviewed-by: Jerry (Fangzhi) Zuo The change fixed MST + SST daisy chain and S3 scenarios. The issue shows huge delay in MST + SST daisy chain, and soft hang in S3 resume. The aux sequence is changed by failed iteration search in drm_connector_for_each_possible_encoder(). The failure of searching for the best encoder for the connector due to the miss of attached encoder in the process of adding MST connector. The iteration search takes time to push drm_dp_send_enum_path_resources() aux transaction after the mode probe, and causes conflict to drm_dp_mst_i2c_xfer(), leading to the aux transaction timeout. -Original Message- From: Lyude Paul Sent: November 16, 2018 6:25 PM To: amd-...@lists.freedesktop.org Cc: Zuo, Jerry ; Wentland, Harry ; Li, Sun peng (Leo) ; Deucher, Alexander ; Koenig, Christian ; Zhou, David(ChunMing) ; David Airlie ; Li, Roman ; S, Shirish ; Daniel Vetter ; dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders Drive-by fix, this is bound to cause problems somewhere. Signed-off-by: Lyude Paul Cc: Jerry Zuo --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++ 1 file changed, 2 insertions(+) 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 d02c32a1039c..0cca1809fdcd 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 @@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr, master->connector_id); aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master); + drm_connector_attach_encoder(&aconnector->base, +&aconnector->mst_encoder->base); /* * TODO: understand why this one is needed -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders
Sure, I'll do it. -Original Message- From: Lyude Paul Sent: November 19, 2018 1:52 PM To: Zuo, Jerry ; amd-...@lists.freedesktop.org Cc: Wentland, Harry ; Li, Sun peng (Leo) ; Deucher, Alexander ; Koenig, Christian ; Zhou, David(ChunMing) ; David Airlie ; Li, Roman ; S, Shirish ; Daniel Vetter ; dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders Cool! If it did actually fix those problems, would you mind making sure this gets Cc'd to stable when it gets pushed upstream? On Mon, 2018-11-19 at 15:00 +0000, Zuo, Jerry wrote: > Reviewed-by: Jerry (Fangzhi) Zuo > > The change fixed MST + SST daisy chain and S3 scenarios. The issue > shows huge delay in MST + SST daisy chain, and soft hang in S3 resume. > > The aux sequence is changed by failed iteration search in > drm_connector_for_each_possible_encoder(). > The failure of searching for the best encoder for the connector due to > the miss of attached encoder in the process of adding MST connector. > The iteration search takes time to push > drm_dp_send_enum_path_resources() aux transaction after the mode > probe, and causes conflict to drm_dp_mst_i2c_xfer(), leading to the aux > transaction timeout. > > -Original Message- > From: Lyude Paul > Sent: November 16, 2018 6:25 PM > To: amd-...@lists.freedesktop.org > Cc: Zuo, Jerry ; Wentland, Harry > ; Li, Sun peng (Leo) ; > Deucher, Alexander < alexander.deuc...@amd.com>; Koenig, Christian > ; Zhou, David(ChunMing) > ; David Airlie ; Li, Roman > ; S, Shirish ; Daniel Vetter > ; dri-devel@lists.freedesktop.org; > linux-ker...@vger.kernel.org > Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders > > Drive-by fix, this is bound to cause problems somewhere. > > Signed-off-by: Lyude Paul > Cc: Jerry Zuo > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++ > 1 file changed, 2 insertions(+) > > 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 d02c32a1039c..0cca1809fdcd 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 > @@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct > drm_dp_mst_topology_mgr *mgr, > master->connector_id); > > aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master); > + drm_connector_attach_encoder(&aconnector->base, > + &aconnector->mst_encoder->base); > > /* >* TODO: understand why this one is needed -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 2/2] drm/amd/dm: Understand why attaching path/tile properties are needed
-Original Message- From: Lyude Paul Sent: November 16, 2018 6:25 PM To: amd-...@lists.freedesktop.org Cc: Wentland, Harry ; Li, Sun peng (Leo) ; Deucher, Alexander ; Koenig, Christian ; Zhou, David(ChunMing) ; David Airlie ; Zuo, Jerry ; Li, Roman ; Cheng, Tony ; Daniel Vetter ; S, Shirish ; dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org Subject: [PATCH 2/2] drm/amd/dm: Understand why attaching path/tile properties are needed Path property is used for userspace to know what MST connector goes to what actual DRM DisplayPort connector, the tiling property is for tiling configurations. Not sure what else there is to figure out. Signed-off-by: Lyude Paul Reviewed-by: Jerry (Fangzhi) Zuo --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 --- 1 file changed, 3 deletions(-) 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 0cca1809fdcd..1b0d209d8367 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 @@ -345,9 +345,6 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr, drm_connector_attach_encoder(&aconnector->base, &aconnector->mst_encoder->base); - /* -* TODO: understand why this one is needed -*/ drm_object_attach_property( &connector->base, dev->mode_config.path_property, -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v4] drm: Add support for DP 1.4 Compliance edid corruption test
[AMD Official Use Only - Internal Distribution Only]
Hi All:
I just checked the CI report
https://patchwork.freedesktop.org/series/70530/. The failures described in
there are not quite related to my patch. Seems it is a false-positive. Does
anyone know something about the issue described in the report?
In addition, I'll resend a new version that fixes the checkpatch issues.
Thanks a lot.
Regards,
Jerry
-Original Message-
From: Jerry (Fangzhi) Zuo
Sent: December 4, 2019 1:03 PM
To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
amd-...@lists.freedesktop.org
Cc: Ville Syrjälä ; Jani Nikula
; manasi.d.nav...@intel.com; Wentland, Harry
; Kazlauskas, Nicholas ;
Siqueira, Rodrigo ; Deucher, Alexander
; Zuo, Jerry
Subject: [PATCH v4] drm: Add support for DP 1.4 Compliance edid corruption test
Unlike DP 1.2 edid corruption test, DP 1.4 requires to calculate real CRC value
of the last edid data block, and write it back.
Current edid CRC calculates routine adds the last CRC byte, and check if
non-zero.
This behavior is not accurate; actually, we need to return the actual CRC value
when corruption is detected.
This commit changes this issue by returning the calculated CRC, and initiate
the required sequence.
Change since v3
- Fix a minor typo.
Change since v2
- Rewrite checksum computation routine to avoid duplicated code.
- Rename to avoid confusion.
Change since v1
- Have separate routine for returning real CRC.
Signed-off-by: Jerry (Fangzhi) Zuo
---
drivers/gpu/drm/drm_dp_helper.c | 35 +++
drivers/gpu/drm/drm_edid.c | 23 +++
include/drm/drm_connector.h | 6 ++
include/drm/drm_dp_helper.h | 3 +++
4 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 2c7870aef469..c59f7c94ebf1 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -351,6 +351,41 @@ 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
+ * @real_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, test_resp = 0;
+
+ drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 1);
+ auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
+
+ drm_dp_dpcd_read(aux, DP_TEST_REQUEST, &link_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, &auto_test_req,
+1);
+
+ /* send back checksum for the last edid extension block data */
+ drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, &real_edid_checksum, 1);
+
+ test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
+ drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, &test_resp, 1);
+
+ return true;
+}
+EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
+
/**
* drm_dp_downstream_max_clock() - extract branch device max
* pixel rate for legacy VGA
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index
5b33b7cfd645..0e35405ecc74 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1450,11 +1450,22 @@ static int validate_displayid(u8 *displayid, int
length, int idx); static int drm_edid_block_checksum(const u8 *raw_edid) {
int i;
- u8 csum = 0;
- for (i = 0; i < EDID_LENGTH; i++)
+ u8 csum = 0, crc = 0;
+
+ for (i = 0; i < EDID_LENGTH - 1; i++)
csum += raw_edid[i];
- return csum;
+ crc = 0x100 - csum;
+
+ return crc;
+}
+
+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) @@ -1512,7 +1523,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;
@@ -1653,6 +1664,7 @@ static void connector_bad_edid(struct drm_connector
*connector,
u8 *edid, int num_blocks)
{
int i;
+ u8 num_of_ext = edid[0x7e];
if (co
RE: [PATCH v4] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6
Please kindly give a review on my latest revision. Thanks a lot.
Regards,
Jerry
-Original Message-
From: Jerry (Fangzhi) Zuo
Sent: November 5, 2019 11:38 AM
To: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org
Cc: ly...@redhat.com; manasi.d.nav...@intel.com; Wentland, Harry
; Zuo, Jerry
Subject: [PATCH v4] 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
-v4: Fix a minor typo.
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..22a0e966ea9f 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
+ * @real_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, &auto_test_req, 1);
+auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
+
+drm_dp_dpcd_read(aux, DP_TEST_REQUEST, &link_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,
+ &auto_test_req, 1);
+
+/* send back checksum for the last edid extension block data */
+drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM,
+ &real_edid_checksum, 1);
+
+test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
+drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, &test_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_
RE: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3)
[AMD Official Use Only - Internal Distribution Only] Hi Alex: The patch set is verified on Ubuntu, and I'll try it out on Chrome as well. Thanks a lot! Regards, Jerry -Original Message- From: Alex Deucher Sent: February 25, 2020 1:42 PM To: Zuo, Jerry Cc: Liu, Zhan ; Wentland, Harry ; amd-gfx list ; Maling list - DRI developers ; Deucher, Alexander ; Broadworth, Mark Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3) On Tue, Feb 25, 2020 at 1:32 PM Alex Deucher wrote: > > On Tue, Feb 25, 2020 at 1:30 PM Zuo, Jerry wrote: > > > > [AMD Official Use Only - Internal Distribution Only] > > > > Hi Alex: > > > > It happened when a MST monitor is attached, either in driver load or > > hotplug later. > > I think I found the issue. I'll send a patch shortly. Attaching two patches. I think patch 1 should fix it. Patch 2 is the same patch as before. I'm not sure drm_dp_mst_connector_late_register() is necessary since no other driver calls it. Alex > > Alex > > > > > > Regards, > > Jerry > > > > -Original Message----- > > From: Alex Deucher > > Sent: February 25, 2020 1:23 PM > > To: Liu, Zhan > > Cc: Wentland, Harry ; Zuo, Jerry > > ; amd-gfx list ; > > Maling list - DRI developers ; > > Deucher, Alexander ; Broadworth, Mark > > > > Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp connector > > registration (v3) > > > > On Tue, Feb 25, 2020 at 1:20 PM Liu, Zhan wrote: > > > > > > > > > > -Original Message- > > > > From: Liu, Zhan > > > > Sent: 2020/February/25, Tuesday 10:10 AM > > > > To: Alex Deucher ; Wentland, Harry > > > > > > > > Cc: amd-gfx list ; Maling list - > > > > DRI developers ; Deucher, > > > > Alexander ; Broadworth, Mark > > > > > > > > Subject: RE: [PATCH 13/15] drm/amdgpu/display: split dp > > > > connector registration (v3) > > > > > > > > > > > > > -Original Message- > > > > > From: Alex Deucher > > > > > Sent: 2020/February/25, Tuesday 9:07 AM > > > > > To: Wentland, Harry > > > > > Cc: amd-gfx list ; Maling list > > > > > - DRI developers ; Deucher, > > > > > Alexander ; Broadworth, Mark > > > > > ; Liu, Zhan > > > > > Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp > > > > > connector registration (v3) > > > > > > > > > > On Mon, Feb 24, 2020 at 4:09 PM Harry Wentland > > > > > > > > > > wrote: > > > > > > > > > > > > On 2020-02-07 4:17 p.m., Alex Deucher wrote: > > > > > > > Split into init and register functions to avoid a segfault > > > > > > > in some configs when the load/unload callbacks are removed. > > > > > > > > > > > > > > > > > > > Looks like MST is completely broken with this change with a > > > > > > NULL pointer dereference in drm_dp_aux_register. > > > > > > > > > > > > > v2: > > > > > > > - add back accidently dropped has_aux setting > > > > > > > - set dev in late_register > > > > > > > > > > > > > > v3: > > > > > > > - fix dp cec ordering > > > > > > > > > > > > > > Signed-off-by: Alex Deucher > > > > > > > --- > > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16 > > > > > > > > > > > > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 10 ++ > > > > > > > .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 7 > > > > > > > ++- > > > > > > > 3 files changed, 24 insertions(+), 9 deletions(-) > > > > > > > > > > > > > > diff --git > > > > > > > a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > > > > > > index ec1501e3a63a..f355d9a752d2 100644 > > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > > > > > > @@ -1461,6 +1461,20 @@ static enum drm_mode_st
RE: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3)
[AMD Official Use Only - Internal Distribution Only] Hi Alex: The patch set works. Please let me know when you push the change to drm-next. Thanks a lot. Regards, Jerry -Original Message- From: Alex Deucher Sent: February 25, 2020 1:42 PM To: Zuo, Jerry Cc: Liu, Zhan ; Wentland, Harry ; amd-gfx list ; Maling list - DRI developers ; Deucher, Alexander ; Broadworth, Mark Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3) On Tue, Feb 25, 2020 at 1:32 PM Alex Deucher wrote: > > On Tue, Feb 25, 2020 at 1:30 PM Zuo, Jerry wrote: > > > > [AMD Official Use Only - Internal Distribution Only] > > > > Hi Alex: > > > > It happened when a MST monitor is attached, either in driver load or > > hotplug later. > > I think I found the issue. I'll send a patch shortly. Attaching two patches. I think patch 1 should fix it. Patch 2 is the same patch as before. I'm not sure drm_dp_mst_connector_late_register() is necessary since no other driver calls it. Alex > > Alex > > > > > > Regards, > > Jerry > > > > -Original Message- > > From: Alex Deucher > > Sent: February 25, 2020 1:23 PM > > To: Liu, Zhan > > Cc: Wentland, Harry ; Zuo, Jerry > > ; amd-gfx list ; > > Maling list - DRI developers ; > > Deucher, Alexander ; Broadworth, Mark > > > > Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp connector > > registration (v3) > > > > On Tue, Feb 25, 2020 at 1:20 PM Liu, Zhan wrote: > > > > > > > > > > -Original Message- > > > > From: Liu, Zhan > > > > Sent: 2020/February/25, Tuesday 10:10 AM > > > > To: Alex Deucher ; Wentland, Harry > > > > > > > > Cc: amd-gfx list ; Maling list - > > > > DRI developers ; Deucher, > > > > Alexander ; Broadworth, Mark > > > > > > > > Subject: RE: [PATCH 13/15] drm/amdgpu/display: split dp > > > > connector registration (v3) > > > > > > > > > > > > > -Original Message- > > > > > From: Alex Deucher > > > > > Sent: 2020/February/25, Tuesday 9:07 AM > > > > > To: Wentland, Harry > > > > > Cc: amd-gfx list ; Maling list > > > > > - DRI developers ; Deucher, > > > > > Alexander ; Broadworth, Mark > > > > > ; Liu, Zhan > > > > > Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp > > > > > connector registration (v3) > > > > > > > > > > On Mon, Feb 24, 2020 at 4:09 PM Harry Wentland > > > > > > > > > > wrote: > > > > > > > > > > > > On 2020-02-07 4:17 p.m., Alex Deucher wrote: > > > > > > > Split into init and register functions to avoid a segfault > > > > > > > in some configs when the load/unload callbacks are removed. > > > > > > > > > > > > > > > > > > > Looks like MST is completely broken with this change with a > > > > > > NULL pointer dereference in drm_dp_aux_register. > > > > > > > > > > > > > v2: > > > > > > > - add back accidently dropped has_aux setting > > > > > > > - set dev in late_register > > > > > > > > > > > > > > v3: > > > > > > > - fix dp cec ordering > > > > > > > > > > > > > > Signed-off-by: Alex Deucher > > > > > > > --- > > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16 > > > > > > > > > > > > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 10 ++ > > > > > > > .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 7 > > > > > > > ++- > > > > > > > 3 files changed, 24 insertions(+), 9 deletions(-) > > > > > > > > > > > > > > diff --git > > > > > > > a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > > > > > > index ec1501e3a63a..f355d9a752d2 100644 > > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > > > > > > @@ -1461,6 +1461,20 @
RE: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3)
[AMD Official Use Only - Internal Distribution Only] Reviewed-by: Jerry (Fangzhi) Zuo -Original Message- From: Alex Deucher Sent: February 26, 2020 10:40 AM To: Zuo, Jerry Cc: Liu, Zhan ; Wentland, Harry ; amd-gfx list ; Maling list - DRI developers ; Deucher, Alexander ; Broadworth, Mark Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3) On Wed, Feb 26, 2020 at 10:36 AM Zuo, Jerry wrote: > > [AMD Official Use Only - Internal Distribution Only] > > Hi Alex: > > The patch set works. Please let me know when you push the change to > drm-next. Can someone give me and R-b or A-b on the patches? Thanks, Alex > > Thanks a lot. > > Regards, > Jerry > > -Original Message- > From: Alex Deucher > Sent: February 25, 2020 1:42 PM > To: Zuo, Jerry > Cc: Liu, Zhan ; Wentland, Harry > ; amd-gfx list > ; Maling list - DRI developers > ; Deucher, Alexander > ; Broadworth, Mark > > Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp connector > registration (v3) > > On Tue, Feb 25, 2020 at 1:32 PM Alex Deucher wrote: > > > > On Tue, Feb 25, 2020 at 1:30 PM Zuo, Jerry wrote: > > > > > > [AMD Official Use Only - Internal Distribution Only] > > > > > > Hi Alex: > > > > > > It happened when a MST monitor is attached, either in driver load or > > > hotplug later. > > > > I think I found the issue. I'll send a patch shortly. > > Attaching two patches. I think patch 1 should fix it. Patch 2 is the > same patch as before. I'm not sure > drm_dp_mst_connector_late_register() is necessary since no other driver calls > it. > > Alex > > > > > Alex > > > > > > > > > > Regards, > > > Jerry > > > > > > -Original Message- > > > From: Alex Deucher > > > Sent: February 25, 2020 1:23 PM > > > To: Liu, Zhan > > > Cc: Wentland, Harry ; Zuo, Jerry > > > ; amd-gfx list ; > > > Maling list - DRI developers ; > > > Deucher, Alexander ; Broadworth, Mark > > > > > > Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp connector > > > registration (v3) > > > > > > On Tue, Feb 25, 2020 at 1:20 PM Liu, Zhan wrote: > > > > > > > > > > > > > -Original Message- > > > > > From: Liu, Zhan > > > > > Sent: 2020/February/25, Tuesday 10:10 AM > > > > > To: Alex Deucher ; Wentland, Harry > > > > > > > > > > Cc: amd-gfx list ; Maling list > > > > > - DRI developers ; Deucher, > > > > > Alexander ; Broadworth, Mark > > > > > > > > > > Subject: RE: [PATCH 13/15] drm/amdgpu/display: split dp > > > > > connector registration (v3) > > > > > > > > > > > > > > > > -Original Message- > > > > > > From: Alex Deucher > > > > > > Sent: 2020/February/25, Tuesday 9:07 AM > > > > > > To: Wentland, Harry > > > > > > Cc: amd-gfx list ; Maling > > > > > > list > > > > > > - DRI developers ; Deucher, > > > > > > Alexander ; Broadworth, Mark > > > > > > ; Liu, Zhan > > > > > > Subject: Re: [PATCH 13/15] drm/amdgpu/display: split dp > > > > > > connector registration (v3) > > > > > > > > > > > > On Mon, Feb 24, 2020 at 4:09 PM Harry Wentland > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > On 2020-02-07 4:17 p.m., Alex Deucher wrote: > > > > > > > > Split into init and register functions to avoid a > > > > > > > > segfault in some configs when the load/unload callbacks are > > > > > > > > removed. > > > > > > > > > > > > > > > > > > > > > > Looks like MST is completely broken with this change with > > > > > > > a NULL pointer dereference in drm_dp_aux_register. > > > > > > > > > > > > > > > v2: > > > > > > > > - add back accidently dropped has_aux setting > > > > > > > > - set dev in late_register > > > > > > > > > > > > > > > > v3: > > > > > > > > - fix dp cec ordering > > >
