On 05/08, Harry Wentland wrote:
>
>
> 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@xxxxxxxxxx>
> > ---
> > .../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?
A DRM helper sounds better for me. I'll follow this path.
Melissa
>
> 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,
>
>
Attachment:
signature.asc
Description: PGP signature
