diff mbox series

[RFC,09/40] drm/amd/display: move replace blob func to dm plane

Message ID 20230423141051.702990-10-mwen@igalia.com
State New
Headers
Series drm/amd/display: add AMD driver-specific properties for color mgmt |

Commit Message

Melissa Wen April 23, 2023, 2:10 p.m. UTC 
  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@igalia.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(-)

Comments

Harry Wentland May 8, 2023, 9:24 p.m. UTC | #1 
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@igalia.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,
Melissa Wen May 9, 2023, 4:35 p.m. UTC | #2 
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@igalia.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?

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,
> 
>
diff mbox series

Patch

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;
+}
+#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,