[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f8871b8f-af40-61df-7a77-a632015c9201@amd.com>
Date: Mon, 8 May 2023 17:24:49 -0400
From: Harry Wentland <harry.wentland@....com>
To: Melissa Wen <mwen@...lia.com>, amd-gfx@...ts.freedesktop.org,
Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
sunpeng.li@....com, Alex Deucher <alexander.deucher@....com>,
dri-devel@...ts.freedesktop.org, christian.koenig@....com,
Xinhui.Pan@....com, airlied@...il.com, daniel@...ll.ch
Cc: Joshua Ashton <joshua@...ggi.es>,
Sebastian Wick <sebastian.wick@...hat.com>,
Xaver Hugl <xaver.hugl@...il.com>,
Shashank Sharma <Shashank.Sharma@....com>,
Nicholas Kazlauskas <nicholas.kazlauskas@....com>,
sungjoon.kim@....com, Alex Hung <alex.hung@....com>,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 09/40] drm/amd/display: move replace blob func to dm
plane
On 4/23/23 10:10, Melissa Wen wrote:
> From amdgpu_dm_plane we can get it for both CRTC and plane color
> properties. We are adding new plane properties for AMD driver-private
> color mgmt.
>
> Signed-off-by: Melissa Wen <mwen@...lia.com>
> ---
> .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 37 +------------------
> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 35 ++++++++++++++++++
> .../amd/display/amdgpu_dm/amdgpu_dm_plane.h | 7 ++++
> 3 files changed, 44 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 79324fbab1f1..27d7a8b18013 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -344,39 +344,6 @@ dm_crtc_additional_color_mgmt(struct drm_crtc *crtc)
> DRM_TRANSFER_FUNCTION_DEFAULT);
> }
>
> -static int
> -atomic_replace_property_blob_from_id(struct drm_device *dev,
> - struct drm_property_blob **blob,
> - uint64_t blob_id,
> - ssize_t expected_size,
> - ssize_t expected_elem_size,
> - bool *replaced)
> -{
> - struct drm_property_blob *new_blob = NULL;
> -
> - if (blob_id != 0) {
> - new_blob = drm_property_lookup_blob(dev, blob_id);
> - if (new_blob == NULL)
> - return -EINVAL;
> -
> - if (expected_size > 0 &&
> - new_blob->length != expected_size) {
> - drm_property_blob_put(new_blob);
> - return -EINVAL;
> - }
> - if (expected_elem_size > 0 &&
> - new_blob->length % expected_elem_size != 0) {
> - drm_property_blob_put(new_blob);
> - return -EINVAL;
> - }
> - }
> -
> - *replaced |= drm_property_replace_blob(blob, new_blob);
> - drm_property_blob_put(new_blob);
> -
> - return 0;
> -}
> -
> static int
> amdgpu_dm_atomic_crtc_set_property(struct drm_crtc *crtc,
> struct drm_crtc_state *state,
> @@ -389,7 +356,7 @@ amdgpu_dm_atomic_crtc_set_property(struct drm_crtc *crtc,
> int ret;
>
> if (property == adev->mode_info.shaper_lut_property) {
> - ret = atomic_replace_property_blob_from_id(crtc->dev,
> + ret = amdgpu_dm_replace_property_blob_from_id(crtc->dev,
> &acrtc_state->shaper_lut,
> val,
> -1, sizeof(struct drm_color_lut),
> @@ -397,7 +364,7 @@ amdgpu_dm_atomic_crtc_set_property(struct drm_crtc *crtc,
> acrtc_state->base.color_mgmt_changed |= replaced;
> return ret;
> } else if (property == adev->mode_info.lut3d_property) {
> - ret = atomic_replace_property_blob_from_id(crtc->dev,
> + ret = amdgpu_dm_replace_property_blob_from_id(crtc->dev,
> &acrtc_state->lut3d,
> val,
> -1, sizeof(struct drm_color_lut),
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 322668973747..4e5498153be2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1411,6 +1411,41 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
> drm_atomic_helper_plane_destroy_state(plane, state);
> }
>
> +#ifdef CONFIG_STEAM_DECK
> +int
> +amdgpu_dm_replace_property_blob_from_id(struct drm_device *dev,
> + struct drm_property_blob **blob,
> + uint64_t blob_id,
> + ssize_t expected_size,
> + ssize_t expected_elem_size,
> + bool *replaced)
> +{
> + struct drm_property_blob *new_blob = NULL;
> +
> + if (blob_id != 0) {
> + new_blob = drm_property_lookup_blob(dev, blob_id);
> + if (new_blob == NULL)
> + return -EINVAL;
> +
> + if (expected_size > 0 &&
> + new_blob->length != expected_size) {
> + drm_property_blob_put(new_blob);
> + return -EINVAL;
> + }
> + if (expected_elem_size > 0 &&
> + new_blob->length % expected_elem_size != 0) {
> + drm_property_blob_put(new_blob);
> + return -EINVAL;
> + }
> + }
> +
> + *replaced |= drm_property_replace_blob(blob, new_blob);
> + drm_property_blob_put(new_blob);
> +
> + return 0;
> +}
amdgpu_dm_plane doesn't seem the right place for it either. Maybe a new
amdgpu_dm_helper.c/h?
Alternatively would this make sense to live in DRM core as a helper?
Harry
> +#endif
> +
> static const struct drm_plane_funcs dm_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
> index 930f1572f898..1b05ac4c15f6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
> @@ -51,6 +51,13 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
> bool tmz_surface,
> bool force_disable_dcc);
>
> +int amdgpu_dm_replace_property_blob_from_id(struct drm_device *dev,
> + struct drm_property_blob **blob,
> + uint64_t blob_id,
> + ssize_t expected_size,
> + ssize_t expected_elem_size,
> + bool *replaced);
> +
> int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
> struct drm_plane *plane,
> unsigned long possible_crtcs,
Powered by blists - more mailing lists