| Message ID | 20240321-dp-live-fmt-v3-7-d5090d796b7e@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Setting live video input format for ZynqMP DPSUB | expand |
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
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 521a71c61b16..955ca108cd4b 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -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; 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); +} +EXPORT_SYMBOL(drm_helper_crtc_select_output_bus_format); diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index 8c886fc46ef2..b7eb52f3ce41 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -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 diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 881b03e4dc28..6d5a081e21a4 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -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); }; /**
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(-)