[PATCH] drm/amd/display: reject both non-zero src_x and src_y only for DCN1x
S, Shirish
sshankar at amd.com
Mon Nov 8 14:31:04 UTC 2021
Hi Paul,
On 11/8/2021 7:51 PM, Paul Menzel wrote:
> [Which address should be used: sshankar at amd.com or shirish.s at amd.com?]
"shirish.s at amd.com"
>
> 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?
yes.
Regards,
Shirish S
>
>
> 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 at amd.com>
>>>> ---
>>>> .../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 =
>>>>
More information about the amd-gfx
mailing list