Re: [PATCH RESEND] drm/atomic-helper: rename drm_atomic_helper_check_wb_encoder_state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/12/2023 10:38, Maxime Ripard wrote:
On Sat, Dec 02, 2023 at 12:07:49AM +0200, Dmitry Baryshkov wrote:
The drm_atomic_helper_check_wb_encoder_state() function doesn't use
encoder for anything other than getting the drm_device instance. The
function's description talks about checking the writeback connector
state, not the encoder state. Moreover, there is no such thing as an
encoder state, encoders generally do not have a state on their own.

Drop the first argument and rename the function to
drm_atomic_helper_check_wb_connector_state().

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---

Resending, no reaction for two months

---
  drivers/gpu/drm/drm_atomic_helper.c   | 10 ++++------
  drivers/gpu/drm/vkms/vkms_writeback.c |  2 +-
  include/drm/drm_atomic_helper.h       |  3 +--
  3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 2444fc33dd7c..d69591381f00 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -795,8 +795,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
  EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
/**
- * drm_atomic_helper_check_wb_encoder_state() - Check writeback encoder state
- * @encoder: encoder state to check
+ * drm_atomic_helper_check_wb_connector_state() - Check writeback connector state
   * @conn_state: connector state to check
   *
   * Checks if the writeback connector state is valid, and returns an error if it
@@ -806,8 +805,7 @@ EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
   * Zero for success or -errno
   */
  int
-drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
-					 struct drm_connector_state *conn_state)
+drm_atomic_helper_check_wb_connector_state(struct drm_connector_state *conn_state)

AFAIK, all the helpers take the object as first argument, so I'm fine
with the name change but it should take a drm_connector too. And ideally
a drm_atomic_state pointer instead of drm_connector_state too.

I think we then might take even further step and pass drm_writeback_connector to this function. I'll send this as a part of v2.


  {
  	struct drm_writeback_job *wb_job = conn_state->writeback_job;
  	struct drm_property_blob *pixel_format_blob;
@@ -827,11 +825,11 @@ drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
  		if (fb->format->format == formats[i])
  			return 0;
- drm_dbg_kms(encoder->dev, "Invalid pixel format %p4cc\n", &fb->format->format);
+	drm_dbg_kms(conn_state->connector->dev, "Invalid pixel format %p4cc\n", &fb->format->format);

Which would also avoid the checkpatch warning there.

Maxime

--
With best wishes
Dmitry





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux