If mipi_dsi_dcs_set_display_on() returned an error then we'd store
that in the "ret" variable and jump to error handling. We'd then
attempt an orderly poweroff. Unfortunately we then blew away the value
stored in "ret". That means that if the orderly poweroff actually
worked then we're return 0 (no error) from hx8394_enable() even though
the panel wasn't enabled.
Fix this by not blowing away "ret".
Found by code inspection.
Fixes: 65dc9360f7 ("drm: panel: Add Himax HX8394 panel controller driver")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20240517143643.1.I0a6836fffd8d7620f353becb3df2370d2898f803@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240517143643.1.I0a6836fffd8d7620f353becb3df2370d2898f803@changeid
Unfortunately, the G200 ioburst workaround doesn't work on some
servers like Dell poweredge XR11, XR5610, or HPE XL260. In this case
completely disabling WC is the only option to achieve low-latency.
So this adds a new Kconfig option to disable WC mapping of the G200.
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/patch/msgid/20240517151050.624797-3-jfalempe@redhat.com
This reverts commit bfa4437fd3.
This workaround doesn't work reliably on all servers.
I'll replace it with an option to disable Write-Combine,
which has more impact on performance, but fix the latency
issue on all hardware.
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/patch/msgid/20240517151050.624797-2-jfalempe@redhat.com
The Performance Counters enum used to identify the index of each
performance counter and provide the total number of performance
counters (V3D_PERFCNT_NUM). But, this enum is only valid for V3D 4.2,
not for V3D 7.1.
As we implemented a new flexible structure to retrieve performance
counters information, we can deprecate this enum.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240512222655.2792754-7-mcanal@igalia.com
V3D_PERFCNT_NUM represents the maximum number of performance counters
for V3D 4.2, but not for V3D 7.1. This means that, if we use
V3D_PERFCNT_NUM, we might go out-of-bounds on V3D 7.1.
Therefore, use the number of performance counters on V3D 7.1 as the
maximum number of counters. This will allow us to create arrays on the
stack with reasonable size. Note that userspace must use the value
provided by DRM_V3D_PARAM_MAX_PERF_COUNTERS.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240512222655.2792754-6-mcanal@igalia.com
Userspace usually needs some information about the performance counters
available. Although we could replicate this information in the kernel
and user-space, let's use the kernel as the "single source of truth" to
avoid issues in the future (e.g. list of performance counters is updated
in user-space, but not in the kernel, generating invalid requests).
Therefore, create a new IOCTL to expose the performance counters
information, that is name, category, and description.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240512222655.2792754-5-mcanal@igalia.com
The maximum number of performance counters can change from version to
version and it's important for userspace to know this value, as it needs
to use the counters for performance queries. Therefore, expose the
maximum number of performance counters to userspace as a parameter.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240512222655.2792754-4-mcanal@igalia.com
Currently, even though V3D 7.1 has 93 performance counters, it is not
possible to create counters bigger than 87, as
`v3d_perfmon_create_ioctl()` understands that counters bigger than 87
are invalid.
Therefore, create a device variable to expose the maximum
number of counters for a given V3D version and make
`v3d_perfmon_create_ioctl()` check this variable.
This commit fixes CTS failures in the performance queries tests
`dEQP-VK.query_pool.performance_query.*` [1]
Link: ea1f09a5f2 [1]
Fixes: 6fd9487147 ("drm/v3d: add brcm,2712-v3d as a compatible V3D device")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240512222655.2792754-3-mcanal@igalia.com
Add name, category and description for each one of the 93 performance
counters available on V3D.
Note that V3D 4.2 has 87 performance counters, while V3D 7.1 has 93.
Therefore, there are two performance counters arrays. The index of the
performance counter for each V3D version is represented by its position
on the array.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240512222655.2792754-2-mcanal@igalia.com
Follow the pattern of mipi_dsi_dcs_*_multi() and wrap several existing
MIPI DSI functions to use the context for processing. This simplifies
and streamlines driver code to use simpler code pattern.
Note, msleep function is also wrapped in this way as it is frequently
called inbetween other mipi_dsi_dcs_*() functions.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://lore.kernel.org/r/20240512-dsi-panels-upd-api-v2-2-e31ca14d102e@linaro.org
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240512-dsi-panels-upd-api-v2-2-e31ca14d102e@linaro.org
The modeset lock protects the DDC code from concurrent modeset
operations, which use the same registers. Move that code from the
connector helpers into the DDC helpers .pre_xfer() and .post_xfer().
Both, .pre_xfer() and .post_xfer(), enclose the transfer of data blocks
over the I2C channel in the internal I2C function bit_xfer(). Both
calls are executed unconditionally if present. Invoking DDC transfers
from any where within the driver now takes the lock.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513125620.6337-10-tzimmermann@suse.de
Align the names of the algo-bit helpers with mgag200's convention of
using an mgag200 prefix plus the struct's name plus the callback's name
for such function symbols.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513125620.6337-9-tzimmermann@suse.de
Rename struct mga_i2c_chan to struct mgag200_ddc, define it in the
source file mgag200_i2c.c, and reorder its fields. Rename all related
variables from i2c to ddc. Also rename the i2c adapter accordingly.
Using the term 'ddc' documents the purpose of the code clearly. The
old term 'i2c' could refer to any functionality on an i2c bus. No
functional changes.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513125620.6337-7-tzimmermann@suse.de
The function mgag200_i2c_init() is an internal helper that sets
up the i2c data structure. Inline its code into the only caller.
Rearrange the individual steps to separate among i2c algorithm,
adapter and fields in struct mga_i2c_chan.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513125620.6337-6-tzimmermann@suse.de
Allocate instances of struct mga_i2c_chan in mgag200_ddc_create()
and return a pointer to the contained i2c adapter. The callers of
the function are now independent from struct mga_i2c_chan.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240513125620.6337-5-tzimmermann@suse.de
Managed cleanup with devm_add_action_or_reset() will release the I2C
adapter when the underlying Linux device goes away. But the connector
still refers to it, so this cleanup leaves behind a stale pointer
in struct drm_connector.ddc.
Bind the lifetime of the I2C adapter to the connector's lifetime by
using DRM's managed release. When the DRM device goes away (after
the Linux device) DRM will first clean up the connector and then
clean up the I2C adapter.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Fixes: b279df2429 ("drm/mgag200: Switch I2C code to managed cleanup")
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Jocelyn Falempe <jfalempe@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v6.0+
Link: https://patchwork.freedesktop.org/patch/msgid/20240513125620.6337-3-tzimmermann@suse.de
Compute the i2c timeout in jiffies from a value in milliseconds. The
original values of 2 jiffies equals 2 milliseconds if HZ has been
configured to a value of 1000. This corresponds to 2.2 milliseconds
used by most other DRM drivers. Update mgag200 accordingly.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Fixes: 414c453106 ("mgag200: initial g200se driver (v2)")
Cc: Dave Airlie <airlied@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Jocelyn Falempe <jfalempe@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v3.5+
Link: https://patchwork.freedesktop.org/patch/msgid/20240513125620.6337-2-tzimmermann@suse.de
The NPU device consists of two parts: NPU buttress and NPU IP.
Buttress is a platform specific part that integrates the NPU IP with
the CPU.
NPU IP is the platform agnostic part that does the inference.
This separation enables support for multiple platforms using
a single NPU IP, so for example NPU IP 37XX could be integrated into
MTL and LNL platforms.
Signed-off-by: Wachowski, Karol <karol.wachowski@intel.com>
Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240515113006.457472-3-jacek.lawrynowicz@linux.intel.com
Consensus on the mailing lists is that panels shouldn't use a table of
init commands but should instead use init functions. We'll use the
same concepts as the recently introduced
mipi_dsi_generic_write_seq_multi() to make this clean/easy and also
not bloat the driver too much. Measuring before/after this change:
$ scripts/bloat-o-meter \
.../before/panel-innolux-p079zca.ko \
.../after/panel-innolux-p079zca.ko
add/remove: 3/2 grow/shrink: 0/1 up/down: 2356/-1944 (412)
Function old new delta
innolux_p097pfg_init - 1772 +1772
innolux_p097pfg_init.d - 480 +480
innolux_panel_write_multi - 104 +104
innolux_panel_prepare 412 308 -104
.compoundliteral 480 - -480
innolux_p097pfg_init_cmds 1360 - -1360
Total: Before=5802, After=6214, chg +7.10%
Note that, unlike some other drivers, we actually make this panel
driver _bigger_ by using the new functions. This is because the
innolux-p079zca panel driver didn't have as complex of a table and
thus the old table was more efficient than the code. The bloat is
still not giant (only 412 bytes).
Also note that we can't direclty use
mipi_dsi_generic_write_seq_multi() here because we need to deal with
the crazy "nop" that this driver sends after all commands. This means
that we have to write code that is "inspired" by the new macros.
Since we're touching all the tables, let's also convert hex numbers to
lower case as per kernel conventions.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20240514102056.v5.9.I947e28c81f9ef7dcd3add6e193be72d6f8ea086f@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.9.I947e28c81f9ef7dcd3add6e193be72d6f8ea086f@changeid
Consensus on the mailing lists is that panels shouldn't use a table of
init commands but should instead use init functions. With the recently
introduced mipi_dsi_dcs_write_seq_multi() this is not only clean/easy
but also saves space. Measuring before/after this change:
$ scripts/bloat-o-meter \
.../before/panel-ilitek-ili9882t.ko \
.../after/panel-ilitek-ili9882t.ko
add/remove: 3/2 grow/shrink: 0/2 up/down: 6834/-8177 (-1343)
Function old new delta
starry_ili9882t_init - 6152 +6152
starry_ili9882t_init.d - 678 +678
ili9882t_disable.d - 4 +4
ili9882t_disable 260 228 -32
ili9882t_prepare 540 396 -144
.compoundliteral 681 - -681
starry_ili9882t_init_cmd 7320 - -7320
Total: Before=11928, After=10585, chg -11.26%
Let's do the conversion.
Since we're touching all the tables, let's also convert hex numbers to
lower case as per kernel conventions.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20240514102056.v5.8.I407b977f9508573103491df549dfaffc3688db13@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.8.I407b977f9508573103491df549dfaffc3688db13@changeid
This is a mechanical conversion of the novatek-nt36672e driver to use
the new mipi_dsi_dcs_write_seq_multi(). The new function is easier for
clients to understand and using it also causes smaller code to be
generated. Specifically:
$ 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%
Cc: Ritesh Kumar <quic_riteshk@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@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.6.I3c08a7d02c467d2bc88da14e513ea4c8649fce45@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.6.I3c08a7d02c467d2bc88da14e513ea4c8649fce45@changeid
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
We really don't expect these errors to be printed over and over
again. When a driver hits the error it should bail out. Just use a
normal error print.
This gives a nice space savings for users of these functions:
$ scripts/bloat-o-meter \
.../before/panel-novatek-nt36672e.ko \
.../after/panel-novatek-nt36672e.ko
add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-16760 (-16760)
Function old new delta
nt36672e_1080x2408_60hz_init 17080 10640 -6440
nt36672e_1080x2408_60hz_init._rs 10320 - -10320
Total: Before=31815, After=15055, chg -52.68%
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.3.I9982cd5d8014de7a4513f5619f66f88da49ce4ec@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.3.I9982cd5d8014de7a4513f5619f66f88da49ce4ec@changeid
The mipi_dsi_generic_write_seq() macro makes a call to
mipi_dsi_generic_write() which returns a type ssize_t. The macro then
stores it in an int and checks to see if it's negative. This could
theoretically be a problem if "ssize_t" is larger than "int".
To see the issue, imagine that "ssize_t" is 32-bits and "int" is
16-bits, you could see a problem if there was some code out there that
looked like:
mipi_dsi_generic_write_seq(dsi, <32768 bytes as arguments>);
...since we'd get back that 32768 bytes were transferred and 32768
stored in a 16-bit int would look negative.
Though there are no callsites where we'd actually hit this (even if
"int" was only 16-bit), it's cleaner to make the types match so let's
fix it.
Fixes: a9015ce593 ("drm/mipi-dsi: Add a mipi_dsi_dcs_write_seq() macro")
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.2.Iadb65b8add19ed3ae3ed6425011beb97e380a912@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.2.Iadb65b8add19ed3ae3ed6425011beb97e380a912@changeid
The mipi_dsi_dcs_write_seq() macro makes a call to
mipi_dsi_dcs_write_buffer() which returns a type ssize_t. The macro
then stores it in an int and checks to see if it's negative. This
could theoretically be a problem if "ssize_t" is larger than "int".
To see the issue, imagine that "ssize_t" is 32-bits and "int" is
16-bits, you could see a problem if there was some code out there that
looked like:
mipi_dsi_dcs_write_seq(dsi, cmd, <32767 bytes as arguments>);
...since we'd get back that 32768 bytes were transferred and 32768
stored in a 16-bit int would look negative.
Though there are no callsites where we'd actually hit this (even if
"int" was only 16-bit), it's cleaner to make the types match so let's
fix it.
Fixes: 2a9e9daf75 ("drm/mipi-dsi: Introduce mipi_dsi_dcs_write_seq macro")
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.1.I30fa4c8348ea316c886ef8a522a52fed617f930d@changeid
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.1.I30fa4c8348ea316c886ef8a522a52fed617f930d@changeid
