Re: [PATCH V2 04/11] drm/bridge: tc358767: Implement atomic_check callback

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

 



On 2/18/22 18:34, Lucas Stach wrote:

Hi

[...]

  drivers/gpu/drm/bridge/tc358767.c | 26 ++++++++++++++++++++++++++
  1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 522c2c4d8514f..01d11adee6c74 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1289,6 +1289,31 @@ static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
  	return true;
  }
+static int tc_edp_common_atomic_check(struct drm_bridge *bridge,

Drop the edp in the name here? Later in the series you call this
function from the DPI code, so this breaks the nice clean naming
separation from patch 1.

+				      struct drm_bridge_state *bridge_state,
+				      struct drm_crtc_state *crtc_state,
+				      struct drm_connector_state *conn_state,
+				      const unsigned int max_khz)
+{
+	tc_bridge_mode_fixup(bridge, &crtc_state->mode,
+			     &crtc_state->adjusted_mode);
+
+	if (crtc_state->adjusted_mode.clock > max_khz)
+		crtc_state->adjusted_mode.clock = max_khz;

I don't think this is correct. The adjusted most is just for minor
adjustments if the bridge can not fully match the mode. If userspace
supplies a invalid high modeclock I think it would be better to fail
the atomic check -> return -EINVAL

Maxime was telling me that returning -EINVAL from atomic_check is weird, so maybe we should also wait for his opinion on this part.



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux