Re: [PATCH v6 3/8] drm/bridge: mhdp8546: Add minimal format negotiation

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

 



On 15/05/2023 17:59, Aradhya Bhatia wrote:
Hi Tomi,

On 12-May-23 14:45, Tomi Valkeinen wrote:
On 09/05/2023 12:30, Aradhya Bhatia wrote:
From: Nikhil Devshatwar <nikhil.nd@xxxxxx>

With new connector model, mhdp bridge will not create the connector and
SoC driver will rely on format negotiation to setup the encoder format.

Support minimal format negotiations hooks in the drm_bridge_funcs.
Complete format negotiation can be added based on EDID data.
This patch adds the minimal required support to avoid failure
after moving to new connector model.

Signed-off-by: Nikhil Devshatwar <nikhil.nd@xxxxxx>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>

You need to add your SoB to this and the other patches.

Okay!


---

Notes:

      changes from v1:
      * cosmetic fixes, commit message update.

      changes from v5:
      * dropped the default_bus_format variable and directly assigned
        MEDIA_BUS_FMT_RGB121212_1X36 to input_fmts.

   .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 25 +++++++++++++++++++
   1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index f6822dfa3805..623e4235c94f 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2146,6 +2146,30 @@ cdns_mhdp_bridge_atomic_reset(struct drm_bridge
*bridge)
       return &cdns_mhdp_state->base;
   }
   +static u32 *cdns_mhdp_get_input_bus_fmts(struct drm_bridge *bridge,
+                     struct drm_bridge_state *bridge_state,
+                     struct drm_crtc_state *crtc_state,
+                     struct drm_connector_state *conn_state,
+                     u32 output_fmt,
+                     unsigned int *num_input_fmts)
+{
+    u32 *input_fmts;
+
+    *num_input_fmts = 0;
+
+    if (output_fmt != MEDIA_BUS_FMT_FIXED)
+        return NULL;

The tfp410 and sii902x drivers don't have the above check. Why does mhdp
need it? Or the other way, why don't tfp410 and sii902x need it?

I had removed this condition in order to follow status quo, from the
ITE-66121 HDMI bridge driver.

The idea would have been to drop this for MHDP as well, but I guess I
overlooked this one.

However...

I guess at the moment we always do get MEDIA_BUS_FMT_FIXED as the out
fmt (in all three bridge drivers), don't we?

... I tested again to ensure that the above is indeed the case. And
ended up catching some odd behavior.

It turns out that for all the HDMI bridges (TFP410, SII902X, ITE-66121),
the format negotiation doesn't stop at output_fmt = MEDIA_BUS_FMT_FIXED.
The {bridge}_get_input_format API gets called again with the output_fmt
= MEDIA_BUS_FMT_RGB24_1X24.

This doesn't happen with the MHDP driver. Format negotiation with MHDP
bridge stops after one round, at output_fmt = MEDIA_BUS_FMT_FIXED.

This is because the bridge negociation logic will test with all possible
output formats from the chain, and won't stop at first working test.

If your bridge only supports a single input format, it should return the
same format whatever output_fmt is tried.

So indeed remove this test on mhdp aswell, or filter out invalid output
formats.

The MEDIA_BUS_FMT_FIXED is when there's no output format to test, so this
should be always supported.

Neil



Regards
Aradhya




[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