[Which address should be used: sshankar@xxxxxxx or shirish.s@xxxxxxx?]
Dear Shirish,
Am 08.11.21 um 12:11 schrieb S, Shirish:
On 11/8/2021 2:25 PM, Paul Menzel wrote:
Am 08.11.21 um 09:15 schrieb Shirish S:
limit the MPO rejection only for DCN1x as its not required on later
it’s
versions.
Where is it documented, that it’s not required for later versions?
This is a workaround to avoid system hang & I've verified its not
required DCN2.0.
Please extend the commit message with that information, and also add how
you verified, that it’s not required for DCN2.0 exactly. (Just test one
system?)
We generally don't have documentation for WA's.
WA is workaround?
Kind regards,
Paul
Shortly describing the implementation is also useful. Something like:
Require `fill_dc_scaling_info()` to receive the device to be able to
check the version.
Fixes: d89f6048bdcb ("drm/amd/display: Reject non-zero src_y and
src_x for video planes")
I’d remove the blank line.
Signed-off-by: Shirish S <shirish.s@xxxxxxx>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 20 ++++++++++---------
1 file changed, 11 insertions(+), 9 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 1e26d9be8993..26b29d561919 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4572,7 +4572,8 @@ static void get_min_max_dc_plane_scaling(struct
drm_device *dev,
}
-static int fill_dc_scaling_info(const struct drm_plane_state
*state,
+static int fill_dc_scaling_info(struct amdgpu_device *adev,
+ const struct drm_plane_state *state,
struct dc_scaling_info *scaling_info)
{
int scale_w, scale_h, min_downscale, max_upscale;
@@ -4586,7 +4587,8 @@ static int fill_dc_scaling_info(const struct
drm_plane_state *state,
/*
* For reasons we don't (yet) fully understand a non-zero
* src_y coordinate into an NV12 buffer can cause a
- * system hang. To avoid hangs (and maybe be overly cautious)
+ * system hang on DCN1x.
+ * To avoid hangs (and maybe be overly cautious)
I’d remove the added line break.
* let's reject both non-zero src_x and src_y.
*
* We currently know of only one use-case to reproduce a
@@ -4594,10 +4596,10 @@ static int fill_dc_scaling_info(const struct
drm_plane_state *state,
* is to gesture the YouTube Android app into full screen
* on ChromeOS.
*/
- if (state->fb &&
- state->fb->format->format == DRM_FORMAT_NV12 &&
- (scaling_info->src_rect.x != 0 ||
- scaling_info->src_rect.y != 0))
+ if (((adev->ip_versions[DCE_HWIP][0] == IP_VERSION(1, 0, 0)) ||
+ (adev->ip_versions[DCE_HWIP][0] == IP_VERSION(1, 0, 1))) &&
+ (state->fb && state->fb->format->format == DRM_FORMAT_NV12 &&
+ (scaling_info->src_rect.x != 0 || scaling_info->src_rect.y
!= 0)))
return -EINVAL;
scaling_info->src_rect.width = state->src_w >> 16;
@@ -5503,7 +5505,7 @@ static int fill_dc_plane_attributes(struct
amdgpu_device *adev,
int ret;
bool force_disable_dcc = false;
- ret = fill_dc_scaling_info(plane_state, &scaling_info);
+ ret = fill_dc_scaling_info(adev, plane_state, &scaling_info);
if (ret)
return ret;
@@ -7566,7 +7568,7 @@ static int dm_plane_atomic_check(struct
drm_plane *plane,
if (ret)
return ret;
- ret = fill_dc_scaling_info(new_plane_state, &scaling_info);
+ ret = fill_dc_scaling_info(adev, new_plane_state, &scaling_info);
if (ret)
return ret;
@@ -9014,7 +9016,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
bundle->surface_updates[planes_count].gamut_remap_matrix =
&dc_plane->gamut_remap_matrix;
}
- fill_dc_scaling_info(new_plane_state,
+ fill_dc_scaling_info(dm->adev, new_plane_state,
&bundle->scaling_infos[planes_count]);
bundle->surface_updates[planes_count].scaling_info =
