[v3,7/9] drm/atomic-helper: Add select_output_bus_format callback
Commit Message
Add optional drm_crtc_helper_funcs.select_output_bus_format callback. This
callback allows to negotiate compatible media bus format on the link
between CRTC and connected DRM encoder or DRM bridge chain. A good usage
example is the CRTC implemented as FPGA soft IP. This kind of CRTC will
most certainly support a single output media bus format, as supporting
multiple runtime options consumes extra FPGA resources. A variety of
options for the FPGA designs are usually achieved by synthesizing IP with
different parameters.
Add drm_helper_crtc_select_output_bus_format that wraps
drm_crtc_helper_funcs.select_output_bus_format.
Incorporate select_output_bus_format callback into the format negotiation
stage to fix the input bus format of the first DRM bridge in the chain.
Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
---
drivers/gpu/drm/drm_bridge.c | 14 +++++++++++--
drivers/gpu/drm/drm_crtc_helper.c | 36 ++++++++++++++++++++++++++++++++
include/drm/drm_crtc_helper.h | 5 +++++
include/drm/drm_modeset_helper_vtables.h | 30 ++++++++++++++++++++++++++
4 files changed, 83 insertions(+), 2 deletions(-)
Comments
On Thu, Mar 21, 2024 at 01:43:45PM -0700, Anatoliy Klymenko wrote:
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 2dafc39a27cb..f2e12a3c4e5f 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -1055,3 +1055,39 @@ int drm_helper_force_disable_all(struct drm_device *dev)
> return ret;
> }
> EXPORT_SYMBOL(drm_helper_force_disable_all);
> +
> +/**
> + * drm_helper_crtc_select_output_bus_format - Select output media bus format
> + * @crtc: The CRTC to query
> + * @crtc_state: The new CRTC state
> + * @supported_fmts: List of media bus format options to pick from
> + * @num_supported_fmts: Number of media bus formats in @supported_fmts list
> + *
> + * Encoder drivers may call this helper to give the connected CRTC a chance to
> + * select compatible or preffered media bus format to use over the CRTC encoder
> + * link. Encoders should provide list of supported input MEDIA_BUS_FMT_* for
> + * CRTC to pick from. CRTC driver is expected to select preferred media bus
> + * format from the list and, once enabled, generate the signal accordingly.
> + *
> + * Returns:
> + * Selected preferred media bus format or 0 if CRTC does not support any from
> + * @supported_fmts list.
> + */
> +u32 drm_helper_crtc_select_output_bus_format(struct drm_crtc *crtc,
> + struct drm_crtc_state *crtc_state,
> + const u32 *supported_fmts,
> + unsigned int num_supported_fmts)
> +{
> + if (!crtc || !supported_fmts || !num_supported_fmts)
> + return 0;
> +
> + if (!crtc->helper_private ||
> + !crtc->helper_private->select_output_bus_format)
> + return supported_fmts[0];
> +
> + return crtc->helper_private->select_output_bus_format(crtc,
> + crtc_state,
> + supported_fmts,
> + num_supported_fmts);
> +}
Again, the output of that selection must be found in the CRTC state,
otherwise CRTCs have no way to know which have been selected.
Maxime
Hi Maxime,
Thank you for the review.
> -----Original Message-----
> From: Maxime Ripard <mripard@kernel.org>
> Sent: Friday, March 22, 2024 2:45 AM
> To: Klymenko, Anatoliy <Anatoliy.Klymenko@amd.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Thomas Zimmermann
> <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Daniel Vetter
> <daniel@ffwll.ch>; Simek, Michal <michal.simek@amd.com>; Andrzej Hajda
> <andrzej.hajda@intel.com>; Neil Armstrong <neil.armstrong@linaro.org>; Robert
> Foss <rfoss@kernel.org>; Jonas Karlman <jonas@kwiboo.se>; Jernej Skrabec
> <jernej.skrabec@gmail.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Mauro Carvalho Chehab <mchehab@kernel.org>; Tomi
> Valkeinen <tomi.valkeinen@ideasonboard.com>; dri-devel@lists.freedesktop.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-media@vger.kernel.org
> Subject: Re: [PATCH v3 7/9] drm/atomic-helper: Add select_output_bus_format
> callback
>
> On Thu, Mar 21, 2024 at 01:43:45PM -0700, Anatoliy Klymenko wrote:
> > diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> > b/drivers/gpu/drm/drm_crtc_helper.c
> > index 2dafc39a27cb..f2e12a3c4e5f 100644
> > --- a/drivers/gpu/drm/drm_crtc_helper.c
> > +++ b/drivers/gpu/drm/drm_crtc_helper.c
> > @@ -1055,3 +1055,39 @@ int drm_helper_force_disable_all(struct
> drm_device *dev)
> > return ret;
> > }
> > EXPORT_SYMBOL(drm_helper_force_disable_all);
> > +
> > +/**
> > + * drm_helper_crtc_select_output_bus_format - Select output media bus
> > +format
> > + * @crtc: The CRTC to query
> > + * @crtc_state: The new CRTC state
> > + * @supported_fmts: List of media bus format options to pick from
> > + * @num_supported_fmts: Number of media bus formats in
> > +@supported_fmts list
> > + *
> > + * Encoder drivers may call this helper to give the connected CRTC a
> > +chance to
> > + * select compatible or preffered media bus format to use over the
> > +CRTC encoder
> > + * link. Encoders should provide list of supported input
> > +MEDIA_BUS_FMT_* for
> > + * CRTC to pick from. CRTC driver is expected to select preferred
> > +media bus
> > + * format from the list and, once enabled, generate the signal accordingly.
> > + *
> > + * Returns:
> > + * Selected preferred media bus format or 0 if CRTC does not support
> > +any from
> > + * @supported_fmts list.
> > + */
> > +u32 drm_helper_crtc_select_output_bus_format(struct drm_crtc *crtc,
> > + struct drm_crtc_state *crtc_state,
> > + const u32 *supported_fmts,
> > + unsigned int num_supported_fmts) {
> > + if (!crtc || !supported_fmts || !num_supported_fmts)
> > + return 0;
> > +
> > + if (!crtc->helper_private ||
> > + !crtc->helper_private->select_output_bus_format)
> > + return supported_fmts[0];
> > +
> > + return crtc->helper_private->select_output_bus_format(crtc,
> > + crtc_state,
> > + supported_fmts,
> > + num_supported_fmts);
> > +}
>
> Again, the output of that selection must be found in the CRTC state, otherwise
> CRTCs have no way to know which have been selected.
>
Yes, now I got it - thank you. I'll fix this in the next version.
> Maxime
Thank you,
Anatoliy
@@ -28,6 +28,7 @@
#include <drm/drm_atomic_state_helper.h>
#include <drm/drm_bridge.h>
+#include <drm/drm_crtc_helper.h>
#include <drm/drm_debugfs.h>
#include <drm/drm_edid.h>
#include <drm/drm_encoder.h>
@@ -879,7 +880,8 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge,
unsigned int i, num_in_bus_fmts = 0;
struct drm_bridge_state *cur_state;
struct drm_bridge *prev_bridge;
- u32 *in_bus_fmts;
+ struct drm_crtc *crtc = crtc_state->crtc;
+ u32 *in_bus_fmts, in_fmt;
int ret;
prev_bridge = drm_bridge_get_prev_bridge(cur_bridge);
@@ -933,7 +935,15 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge,
return -ENOMEM;
if (first_bridge == cur_bridge) {
- cur_state->input_bus_cfg.format = in_bus_fmts[0];
+ in_fmt = drm_helper_crtc_select_output_bus_format(crtc,
+ crtc_state,
+ in_bus_fmts,
+ num_in_bus_fmts);
+ if (!in_fmt) {
+ kfree(in_bus_fmts);
+ return -ENOTSUPP;
+ }
+ cur_state->input_bus_cfg.format = in_fmt;
cur_state->output_bus_cfg.format = out_bus_fmt;
kfree(in_bus_fmts);
return 0;
@@ -1055,3 +1055,39 @@ int drm_helper_force_disable_all(struct drm_device *dev)
return ret;
}
EXPORT_SYMBOL(drm_helper_force_disable_all);
+
+/**
+ * drm_helper_crtc_select_output_bus_format - Select output media bus format
+ * @crtc: The CRTC to query
+ * @crtc_state: The new CRTC state
+ * @supported_fmts: List of media bus format options to pick from
+ * @num_supported_fmts: Number of media bus formats in @supported_fmts list
+ *
+ * Encoder drivers may call this helper to give the connected CRTC a chance to
+ * select compatible or preffered media bus format to use over the CRTC encoder
+ * link. Encoders should provide list of supported input MEDIA_BUS_FMT_* for
+ * CRTC to pick from. CRTC driver is expected to select preferred media bus
+ * format from the list and, once enabled, generate the signal accordingly.
+ *
+ * Returns:
+ * Selected preferred media bus format or 0 if CRTC does not support any from
+ * @supported_fmts list.
+ */
+u32 drm_helper_crtc_select_output_bus_format(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state,
+ const u32 *supported_fmts,
+ unsigned int num_supported_fmts)
+{
+ if (!crtc || !supported_fmts || !num_supported_fmts)
+ return 0;
+
+ if (!crtc->helper_private ||
+ !crtc->helper_private->select_output_bus_format)
+ return supported_fmts[0];
+
+ return crtc->helper_private->select_output_bus_format(crtc,
+ crtc_state,
+ supported_fmts,
+ num_supported_fmts);
+}
+EXPORT_SYMBOL(drm_helper_crtc_select_output_bus_format);
@@ -38,6 +38,7 @@
struct drm_atomic_state;
struct drm_connector;
struct drm_crtc;
+struct drm_crtc_state;
struct drm_device;
struct drm_display_mode;
struct drm_encoder;
@@ -61,5 +62,9 @@ int drm_helper_connector_dpms(struct drm_connector *connector, int mode);
void drm_helper_resume_force_mode(struct drm_device *dev);
int drm_helper_force_disable_all(struct drm_device *dev);
+u32 drm_helper_crtc_select_output_bus_format(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state,
+ const u32 *supported_fmts,
+ unsigned int num_supported_fmts);
#endif
@@ -489,6 +489,36 @@ struct drm_crtc_helper_funcs {
bool in_vblank_irq, int *vpos, int *hpos,
ktime_t *stime, ktime_t *etime,
const struct drm_display_mode *mode);
+
+ /**
+ * @select_output_bus_format
+ *
+ * Called by the connected DRM encoder to negotiate input media bus
+ * format. CRTC is expected to pick preferable media formats from the
+ * list provided by the DRM encoder.
+ *
+ * This callback is optional.
+ *
+ * Parameters:
+ *
+ * crtc:
+ * The CRTC.
+ * crcs_state:
+ * New CRTC state.
+ * supported_fmts:
+ * List of input bus formats supported by the encoder.
+ * num_supported_fmts:
+ * Number of formats in the list.
+ *
+ * Returns:
+ *
+ * Preferred bus format from the list or 0 if CRTC doesn't support any
+ * from the provided list.
+ */
+ u32 (*select_output_bus_format)(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state,
+ const u32 *supported_fmts,
+ unsigned int num_supported_fmts);
};
/**