On Fri, Nov 13, 2020 at 03:56:41PM -0500, Bindu Ramamurthy wrote:
> From: Wayne Lin
>
> [Why]
> Instead of calculating CRC on whole frame, add flexibility to calculate
> CRC on specific frame region.
>
> [How]
> Add few crc window coordinate properties. By default, CRC is calculated
> on whole frame unless user space specifies the CRC calculation window.
>
> Signed-off-by: Wayne Lin
> Acked-by: Bindu Ramamurthy
Already pinged Alex on irc, but here also as a mail.
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 142 +-
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 19 +++
> .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 43 +-
> .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h | 3 +
> drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +
> 5 files changed, 201 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 77c06f999040..f81c49f28bc9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -943,6 +943,41 @@ static void mmhub_read_system_context(struct
> amdgpu_device *adev, struct dc_phy_
> }
> #endif
>
> +#ifdef CONFIG_DEBUG_FS
> +static int create_crtc_crc_properties(struct amdgpu_display_manager *dm)
Yes it's behind a #ifdef but a) most distros enable this anyway and b)
it's still a KMS property, so still uapi, i.e.
- should be discussed on dri-devel
- needs igt testcases and stuff
- and real userspace
Drivers adding random kms properties has brought us into a pretty giant
mess, we need to stop this. That's why we've increased merge criteria for
these to include an igt and have at least some hopes of a cross-driver
standard. Also the crc interface is all in debugfs, that's where this
belongs.
Please fix this before we ship it. Ideally we'd make this a standard part
so it can be used in igt testcase, but quick fix would be to either revert
or at least move into debugfs files (we have per-crtc files, so not hard
to pull off).
If this is for functional safety or whatever the IVI standard for that
was, then it needs real uapi treatment.
Thanks, Daniel
> +{
> + dm->crc_win_x_start_property =
> + drm_property_create_range(adev_to_drm(dm->adev),
> + DRM_MODE_PROP_ATOMIC,
> + "AMD_CRC_WIN_X_START", 0, U16_MAX);
> + if (!dm->crc_win_x_start_property)
> + return -ENOMEM;
> +
> + dm->crc_win_y_start_property =
> + drm_property_create_range(adev_to_drm(dm->adev),
> + DRM_MODE_PROP_ATOMIC,
> + "AMD_CRC_WIN_Y_START", 0, U16_MAX);
> + if (!dm->crc_win_y_start_property)
> + return -ENOMEM;
> +
> + dm->crc_win_x_end_property =
> + drm_property_create_range(adev_to_drm(dm->adev),
> + DRM_MODE_PROP_ATOMIC,
> + "AMD_CRC_WIN_X_END", 0, U16_MAX);
> + if (!dm->crc_win_x_end_property)
> + return -ENOMEM;
> +
> + dm->crc_win_y_end_property =
> + drm_property_create_range(adev_to_drm(dm->adev),
> + DRM_MODE_PROP_ATOMIC,
> + "AMD_CRC_WIN_Y_END", 0, U16_MAX);
> + if (!dm->crc_win_y_end_property)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +#endif
> +
> static int amdgpu_dm_init(struct amdgpu_device *adev)
> {
> struct dc_init_data init_data;
> @@ -1084,6 +1119,10 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>
> dc_init_callbacks(adev->dm.dc, _params);
> }
> +#endif
> +#ifdef CONFIG_DEBUG_FS
> + if (create_crtc_crc_properties(>dm))
> + DRM_ERROR("amdgpu: failed to create crc property.\n");
> #endif
> if (amdgpu_dm_initialize_drm_device(adev)) {
> DRM_ERROR(
> @@ -5409,12 +5448,64 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
> state->crc_src = cur->crc_src;
> state->cm_has_degamma = cur->cm_has_degamma;
> state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
> -
> +#ifdef CONFIG_DEBUG_FS
> + state->crc_window = cur->crc_window;
> +#endif
> /* TODO Duplicate dc_stream after objects are stream object is
> flattened */
>
> return >base;
> }
>
> +#ifdef CONFIG_DEBUG_FS
> +int amdgpu_dm_crtc_atomic_set_property(struct drm_crtc *crtc,
> + struct drm_crtc_state *crtc_state,
> + struct drm_property *property,
> + uint64_t val)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct amdgpu_device *adev = drm_to_adev(dev);
> + struct dm_crtc_state *dm_new_state =
> + to_dm_crtc_state(crtc_state);
> +
> + if
