The current mipi_dsi_*_write_seq() macros are non-intutitive because
they contain a hidden "return" statement that will return out of the
_caller_ of the macro. Let's mark them as deprecated and instead
introduce some new macros that are more intuitive.
These new macros are less optimal when an error occurs but should
behave more optimally when there is no error. Specifically these new
macros cause smaller code to get generated and the code size savings
(less to fetch from RAM, less cache space used, less RAM used) are
important. Since the error case isn't something we need to optimize
for and these new macros are easier to understand and more flexible,
they should be used.
After converting to use these new functions, one example shows some
nice savings while also being easier to understand.
$ scripts/bloat-o-meter \
...after/panel-novatek-nt36672e.ko \
...ctx/panel-novatek-nt36672e.ko
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-988 (-988)
Function old new delta
nt36672e_1080x2408_60hz_init 6236 5248 -988
Total: Before=10651, After=9663, chg -9.28%
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20240514102056.v5.5.Ie94246c30fe95101e0e26dd5f96e976dbeb8f242@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.5.Ie94246c30fe95101e0e26dd5f96e976dbeb8f242@changeid
Through a cooperative effort between Hsin-Yi Wang and Dmitry
Baryshkov, we have realized the dev_err() in the
mipi_dsi_*_write_seq() macros was causing quite a bit of bloat to the
kernel. Let's hoist this call into drm_mipi_dsi.c by adding a "chatty"
version of the functions that includes the print. While doing this,
add a bit more comments to these macros making it clear that they
print errors and also that they return out of _the caller's_ function.
Without any changes to clients this gives a nice savings. Specifically
the macro was inlined and thus the error report call was inlined into
every call to mipi_dsi_dcs_write_seq() and
mipi_dsi_generic_write_seq(). By using a call to a "chatty" function,
the usage is reduced to one call in the chatty function and a function
call at the invoking site.
Building with my build system shows one example:
$ scripts/bloat-o-meter \
.../before/panel-novatek-nt36672e.ko \
.../after/panel-novatek-nt36672e.ko
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-4404 (-4404)
Function old new delta
nt36672e_1080x2408_60hz_init 10640 6236 -4404
Total: Before=15055, After=10651, chg -29.25%
Note that given the change in location of the print it's harder to
include the "cmd" in the printout for mipi_dsi_dcs_write_seq() since,
theoretically, someone could call the new chatty function with a
zero-size array and it would be illegal to dereference data[0].
There's a printk format to print the whole buffer and this is probably
more useful for debugging anyway. Given that we're doing this for
mipi_dsi_dcs_write_seq(), let's also print the buffer for
mipi_dsi_generic_write_seq() in the error case.
It should be noted that the current consensus of DRM folks is that the
mipi_dsi_*_write_seq() should be deprecated due to the non-intuitive
return behavior. A future patch will formally mark them as deprecated
and provide an alternative.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20240514102056.v5.4.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.4.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid
The checks on the existence of bridge->encoder in the implementation of
drm_bridge_funcs::attach() is not necessary, as it has already been checked
in the drm_bridge_attach() function call by previous bridge or KMS driver.
The drm_bridge_attach() will quit with a negative error code returned if
it fails for some reasons, hence, it is guaranteed that the .encoder member
of the drm_bridge instance is not NULL when various bridge attach functions
are called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-13-sui.jingfeng@linux.dev
The checks on the existence of bridge->encoder in the implementation of
drm_bridge_funcs::attach() is not necessary, as it has already been checked
in the drm_bridge_attach() function call by previous bridge or KMS driver.
The drm_bridge_attach() will quit with a negative error code returned if
it fails for some reasons, hence, it is guaranteed that the .encoder member
of the drm_bridge instance is not NULL when various i.MX specific bridge
attach functions are called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-12-sui.jingfeng@linux.dev
In the lt9611uxc_connector_init() function, the check on the existence
of bridge->encoder is not necessary, as it has already been checked in
the drm_bridge_attach() function. And the check on the drm bridge core
happens before check in the implementation. Hence, it is guaranteed that
the .encoder member of the struct drm_bridge is not NULL when
lt9611uxc_connector_init() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-11-sui.jingfeng@linux.dev
In the dw_mipi_dsi_bridge_attach() function, the check on the existence
of bridge->encoder is not necessary, as it has already been checked in
the drm_bridge_attach() function invocked by previous bridge or KMS driver.
The previous drm_bridge_attach() will quit with a negative error code
returned if it fails for some reasons, hence, it is guaranteed that the
.encoder member of the struct drm_bridge is not NULL when
dw_mipi_dsi_bridge_attach() function gets called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-10-sui.jingfeng@linux.dev
In the ge_b850v3_lvds_create_connector function, the check on the existence
of bridge->encoder is not necessary, as it has already been checked in the
drm_bridge_attach() function called by upstream bridge or driver. Hence, it
is guaranteed that the .encoder member of the drm_bridge instance is not
NULL when cdns_mhdp_connector_init() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-9-sui.jingfeng@linux.dev
In the cdns_mhdp_connector_init() function, the check on the existence
of bridge->encoder is not necessary, as it has already been checked in
the drm_bridge_attach() function. As the cdns_mhdp_connector_init() is
only called by cdns_mhdp_attach(), it is guaranteed that the .encoder
member of the struct drm_bridge is not NULL when cdns_mhdp_attach() gets
called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-8-sui.jingfeng@linux.dev
In the adv7511_connector_init() function, the check on the existence of
bridge->encoder is not necessary. As it has already been checked in the
drm_bridge_attach() which happens prior to the adv7511_bridge_attach()
get called. Also note that the adv7511_connector_init() is only called by
adv7511_bridge_attach(). Hence, it is guaranteed that the .encoder member
of the drm_bridge instance is not NULL when adv7511_connector_init() get
called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-7-sui.jingfeng@linux.dev
In it6505_bridge_attach(), the check on the existence of 'bridge->encoder'
is not necessary, as it has already been checked in the drm_bridge_attach()
which happens prior to it6505_bridge_attach() get called. Note that the
it6505_bridge_attach() will only be called by .attach() of the previous
bridge or KMS driver. The previous drm_bridge_attach() will quit with a
negative error code returned if it fails for some reasons. Hence, it is
guaranteed that the .encoder member of the drm_bridge instance is not NULL
when it6505_bridge_attach() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-6-sui.jingfeng@linux.dev
Because the existence of 'bridge->encoder' has already been checked before
the panel_bridge_attach() function get called, and the drm_bridge_attach()
will quit with a negative error code returned if it fails for some reasons.
Hence, it is guaranteed that the .encoder member of the drm_bridge instance
is not NULL when panel_bridge_attach() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-5-sui.jingfeng@linux.dev
Because the existence of 'bridge->encoder' has already been checked before
the ptn3460_bridge_attach() function get called, and drm_bridge_attach()
will quit with a negative error code returned if it fails for some reasons.
Hence, it is guaranteed that the .encoder member of the drm_bridge instance
is not NULL when the ptn3460_bridge_attach() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-4-sui.jingfeng@linux.dev
Because the existence of bridge->encoder has already been checked before
the simple_bridge_attach() function get called, And drm_bridge_attach()
will quit with a negative error code returned if it fails for some reasons.
Hence, it is guaranteed that the .encoder member of the drm_bridge instance
is not NULL when the tfp410_attach() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-3-sui.jingfeng@linux.dev
Because the existence of 'bridge->encoder' has already been checked before
the simple_bridge_attach() function get called, and drm_bridge_attach()
will quit with a negative error code returned if it fails for some reasons.
Hence, it is guaranteed that the .encoder member of the drm_bridge instance
is not NULL when the simple_bridge_attach() get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Robert Foss <rfoss@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513153109.46786-2-sui.jingfeng@linux.dev
Provide separate implementations of .get_modes() and .detect_ctx()
from struct drm_connector. Switch to struct drm_edid.
Udl's .detect() helper used to fetch the EDID from the adapter and the
.get_modes() helper provided display modes from the data. But this
relied on the DRM helpers to call the functions in the correct order.
When no EDID could be retrieved, .detect() regularly printed a warning
to the kernel log.
Switching to the new helpers around struct drm_edid separates both from
each other. The .get_modes() helper now fetches the EDID by itself and
the .detect_ctx() helper only tests for its presence. The patch does a
number of things to implement this.
- Move udl_get_edid_block() to udl_edid.c and rename it to
udl_read_edid_block(). Then use the helper to implement probing in
udl_probe_edid() and reading in udl_edid_read(). The latter helper
is build on top of DRM helpers.
- Replace the existing code in .get_modes() and .detect() with udl's
new EDID helpers. The new code behaves like DRM's similar DDC-based
helpers. Instead of .detect(), udl now implements .detect_ctx().
- Remove the edid data from struct udl_connector. The field cached
the EDID data between calls to .detect() and .get_modes(), but is now
unused.
v3:
- implement udl_probe_edid() with memchr_inv() (Jani)
v2:
- implement udl_probe_edid() within udl
- reword commit description
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240510154841.11370-5-tzimmermann@suse.de
Protect the code in udl_get_edid_block() with drm_dev_enter() and
drm_dev_exit(), so that all callers automatically invoke it. The
function uses hardware resources, which can be hot-unplugged at
any time. The other code in udl_connector_detect() does not use the
resources of the hardware device and therefore does not require
protection.
This change will allow to use udl_get_edid_block() in various
contexts easily.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240510154841.11370-3-tzimmermann@suse.de
The komeda driver prints a pretty verbose log in komeda_pipeline_dump()
detailing the components of each of the two pipelines. This gets printed
multiple times during boot as komeda EPROBE_DEFERs waiting for the
remote bridge drivers to come up. Move this log to after this has
happened indicating that the printed pipeline is actually completely up.
Signed-off-by: Faiz Abbas <faiz.abbas@arm.com>
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240219100915.192475-3-faiz.abbas@arm.com
Acked-by: Liviu Dudau <liviu.dudau@arm.com>
Since commit 4cfe5cc02e ("drm/arm/komeda: Remove component framework and
add a simple encoder"), the devm_drm_of_get_bridge() call happens
regardless of whether any remote nodes are available on the pipeline. Fix
this by moving the bridge attach to its own function and calling it
conditional on there being an output link.
Fixes: 4cfe5cc02e ("drm/arm/komeda: Remove component framework and add a simple encoder")
Signed-off-by: Faiz Abbas <faiz.abbas@arm.com>
[Corrected Commit-id of the fixed patch to match mainline]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
Acked-by: Liviu Dudau <liviu.dudau@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240219100915.192475-2-faiz.abbas@arm.com
