When specifying a frame buffer format like "RG16_BE" (big-endian RG16),
modetest still uses the little-endian variant, as the format string is
truncated to four characters.
Fix this by increasing the format string size to 8 bytes (7 characters +
NUL terminator).
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
v5:
- Add Reviewed-by,
v4:
- No changes,
v3:
- Update for suffix change from "be" to "_BE", cfr. commit
ffb9375a50 ("xf86drm: handle DRM_FORMAT_BIG_ENDIAN in
drmGetFormatName()"),
- Replace hardcoded numbers in code by sizeof(),
v2:
- New.
Add support for drawing the SMPTE pattern in buffers using a
color-indexed frame buffer formats with two, four, or sixteen colors.
Note that this still uses 256 as the CLUT size, as
DRM_IOCTL_MODE_SETGAMMA enforces that the size matches against the
(fixed) gamma size, while the CLUT size depends on the format.
Move clearing the color LUT entries from util_smpte_index_gamma() to its
caller, as only the caller knows how many entries there really are
(currently DRM always assumes 256 entries).
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
v5:
- Add Reviewed-by,
v4:
- Add missing C[12] to oneline-summary,
- Do not remove memset() of full lut, else some entries may stay
uninitialized,
v3:
- Add Acked-by,
v2:
- Split off changes to tests/modetest/modetest.c,
- Add C1 and C2 support.
The linuxdoc comments say userspace can query the gamma size:
* drm_mode_gamma_set_ioctl - set the gamma table
*
* Set the gamma table of a CRTC to the one passed in by the user. Userspace can
* inquire the required gamma table size through drm_mode_gamma_get_ioctl.
* drm_mode_gamma_get_ioctl - get the gamma table
*
* Copy the current gamma table into the storage provided. This also provides
* the gamma table size the driver expects, which can be used to size the
* allocated storage.
but the code doesn't seem to support that in an easy way (like setting
red/green/blue to NULL on input, retrieving gamma_size on output), only
by providing big enough buffers for red/green/blue, and looping over
gamma_size until -EINVAL is no longer returned.
The color LUT for the SMPTE pattern in indexed mode contains 22 entries,
although only 13 are non-unique.
Reduce the size of the color LUT by dropping duplicate entries, so it
can be reused for formats supporting e.g. 16 colors. Rename the
function util_smpte_c8_gamma() to util_smpte_fill_lut(), and its first
parameter size to ncolors, to match their actual use.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
v5:
- Add Reviewed-by,
v4:
- Rename util_smpte_index_gamma() to util_smpte_fill_lut(), and its
first parameter from size to ncolors,
- Move smpte_color_lut[] down,
- Kill FILL_COLOR() macro,
- Add and use EXPAND_COLOR() macro,
v3:
- Add Acked-by,
v2:
- Factor out smpte color LUT.
Print modifiers in hex in addtion to in strings returned by
drmGetFormatModifierName. In some cases, hex numbers can be more easily
compared visually.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Currently the atomic request is only assigned after `set_property()` is
called, leaving `dev.req` in its uninitialized state causing
`drmModeAtomicAddProperty()` to return an error code, which is printed
as `"Success"` because `errno` is not set by `libdrm` (but it would have
been when non-atomic `drmModeObjectSetProperty()` called an IOCTL
immediately):
sony-akatsuki-row ~ $ modetest -M msm -a -w 81:ACTIVE:0
failed to set CRTC 81 property ACTIVE to 0: Success
Solve this by assigning a new atomic request object before calling
`set_property()`, when there are properties to set. Likewise, commit
these properties after `set_property()` even if there is no other
operation (setting modes or planes) specified.
Furthermore `drmModeObjectSetProperty()` is implemented in terms of
`DRM_IOCTL()` which already returns `-errno` when `ioctl()` returns
`-1`, so we should instead pass `ret` to `strerror()` and get an
accurate error string out of `drmModeAtomicAddProperty()` too.
Fixes: 93220283 ("tests/modetest: Add atomic support")
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Let's permit testing vsync with the default mode, this returns
back the pipe content and count when calling set_mode() so the
vsync test can also be used.
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
As found and discussed in [MR 58] a blob is not created in the else arm
because adding the GAMMA_LUT property with a NULL/0 blob_id causes it
to be reset to a default linear / pass-thru gamma table. The values
in the gamma_lut table might still be consumed in the legacy API path
below though, so it has to be initialized to a linear table.
[MR 58]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/58#note_466972
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Let's follow the Rule of Silence. And while here,
document what's going on.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
It is useful to be able to specify mode parameters manually. Add support
for setting user-supplied modes. This patch is based on the original
idea by Rohit and Jessica, but implemented from scratch.
Suggested-by: Rohith Iyer <quic_rohiiyer@quicinc.com>
Suggested-by: Jessica Zhang <quic_jesszhan@quicinc.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
mode_vrefresh() does not take into account interlaced, doublescan, and
multiscan modes, leading to incorrect refresh rates.
Fix this, based on drm_mode_vrefresh() in Linux.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Add writeback support to modetest with the below options:
- Passing in -a -c will now also show the writeback connector
- Dump the writeback output buffer to bitstream
Usage: "./modetest -M msm -s <connector_id>:<widthxheight>
-a -o <filepath>
-P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24"
This currently supports single writeback connector.
Co-developed-by: Rohith Iyer <quic_rohiiyer@quicinc.com>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
[DB: dropped custom mode support, fixed segfault]
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
It seems that __u64 values are defined differently across systems. In
glibc it's defined as unsigned long, in Linux kernel headers
(int-ll64.h) as unsigned long long, and on FreeBSD as uint64_t so it
matches glibc. A temporal solution is to cast all __u64 values to
uint64_t to avoid warnings on Linux, but ideally we'd like a better fix
in the future.
See also: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/212
for discussion.
Signed-off-by: Eleni Maria Stea <elene.mst@gmail.com>
Add support for parsing IN_FORMATS property blobs. Providing libdrm
with this functionality helps to standardise how user-space reads
kernel blobs and decreases duplication on the client side.
drmModeFormatModifierBlobIterNext() allows the caller to view
formats and associated modifiers given a valid property blob.
An example is available inside the libdrm unit test, modetest.c.
Signed-off-by: Luigi Santivetti <luigi.santivetti@imgtec.com>
Reviewed-by: Simon Ser <contact@emersion.fr>
On 64-bit FreeBSD targets uint64_t is generally defined as `unsigned long`
and not `unsigned long long`. Use the PRI macros to fix -Wformat.
Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
Reviewed-by: Simon Ser <contact@emersion.fr>
Introduces two new methods to retrieve a human readable representation of a
format modifier:
drmGetFormatModifierName() - returns a format modifier as a string,
from a token modifier
drmGetFormatModifierVendor() - returns the vendor as a string, from a
token modifier
and the fourcc_mod_get_vendor macro that returns the vendor.
New format modifiers added in drm_fourcc.h uapi kernel header should be
sync'ed up with libdrm and should include a human readable
representation for that format modifier, in order to display it
correctly as a string.
That happens with the help of a python script that reads up drm_fourcc
header file and outputs a static table comprised of token modifiers
alongside a vendor table (Suggested-by Simon Ser <contact@emersion.fr>).
The reason for doing it in libdrm is to have a unified place instead of each
user of libdrm having a way to keep track of the format modifiers.
With this patch, modetest has also been modified to make use of it.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This option finds all connected connector and then sets its preferred
mode on it. If no preferred mode is available, first mode is used.
This option must be set w/o any mode or plane.
This allows for a quick test on all connected outputs.
Loosely based on the work by Ezequiel Garcia <ezequiel@collabora.com>
Changes since Ezequiel's work:
- implement atomic codepath
- set all connectors
- pick correct crtc
- don't set -r by default
- nearly identical output in atomic and non-atomic codepaths
v2:
- Use the crtc->crtc_id, instead of the plane's current crtc_id
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Makes the code a tiny bit more symmetrical.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
The very final drmModeAtommicCommit tears down the existing mode/plane
setup. Following it we clean up other misc state laying around.
Chances are that it will not fail, but in the extremely unlikely case it
does, there's nothing one can do.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Move the hunk of code into a function, making the overall flow easier to
follow and providing some symmetry to the non-atomic path.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
The function is closely related to pipe_find_crtc_and_mode() so we might
as well keep them together.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Move the function above set_mode, since we'll be using it from there as
of next commit.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Instead of duplicating the exact same code across the two functions,
fold them into one.
For some strange reason git diff may show atomic_clear_mode() as changed
The function in untouched, despite the misleading output.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Makes the code shorter and easier to read.
Currently if the user has not set the crtc_id, we fetch the crtc yet do
not "bother" setting the id - do so.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Let's make the code shorter, this avoid crashes (when drmModeGetCrtc()
fails) by using a couple of helpers. As get_resources() considers the
drmModeGetCrtc() fail non-fatal, we might as well handle it properly.
v2: Add a comment above the unreachable abort() (Eze)
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
There's no point in keeping these around since we already fetch the
complete data set. Add respective count_ variables and greatly simplify
the existing code.
Extra brownie points for:
- using the inverse order in free_resources()
- don't memory leak the connector properties
- free the properties themselves, instead of only the objects
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Flesh out the bo_create + drmModeAddFB2 dance into a helper and use it.
Currently we're duplicating that in 4 places, many of which leaking et
al.
As a bonus point this highlights that the atomic_set_plane() seems tad
buggy. That'll be fixed with separate commit.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Don't bother opening the device node, if the args combination is invalid
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
The two functions have been stubs for ages. The alluded generic ioctls
never came to be, assumingly because all new drivers support those.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Combined with -Wundef (added in 75758d2ccf & enforced in ba17673eed),
this provides absolute safety against #ifdef typos.
Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
When a mode is set with just a connector "-s foo",
we get a nasty segmentation fault. Fix it.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Often there are many similar modes, which cannot be selected
via modetest due to its simple string matching.
This change adds a mode index in the display output, which can
then be used to specify a specific modeline to be set.
Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Cc: Rob Clark <robdclark@chromium.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: John Stultz <john.stultz@linaro.org>
[emil: rebase]
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Add function to derive floating value of vertical
refresh rate from drm mode using pixel clock,
horizontal total size and vertical total size.
Use this function to find suitable mode having vrefresh
value which is matching with user provided vrefresh value.
If user doesn't provide any vrefresh value in args then
update vertical refresh rate value in pipe args using this
function.
Also use this function for printing floating vrefresh while
dumping all available modes.
This will give more accurate picture to user for available modes
differentiated by floating vertical refresh rate and help user
select more appropriate mode using suitable refresh rate value.
V4:
1) While setting mode, print mode name and vrefresh using struct
drmModeModeInfo instead of struct pipe_args.
2) Revert back to using a float value instead of float *
for vrefresh arg in connector_find_mode().
V3:
1) Change name of function used to derive refresh rate.
V2:
1) Don't use inline function for deriving refresh rate from mode.
2) If requested mode not found, print refresh rate only
if user had provided it in args.
Signed-off-by: Devarsh Thakkar <devarsh.thakkar@xilinx.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
For the scenario where user may require to modeset with a mode
supporting a fractional value for vertical refresh-rate,
appropriate mode can be selected by searching for mode
having matching fractional vertical refresh rate using
below equation.
vrefresh = (1000 * pixel clock) / (htotal * vtotal) Hz.
We do this way since driver doesn't return float value of vrefresh
as it use int for vrefresh in struct drm_mode_info, but we can derive
the actual value using pixel clock, horizontal total size and
vertical total size values.
So for e.g. if user want to select mode having 59.94 Hz as refresh rate
then with this patch it be can done as shown in below command,
given there is an appropriate mode is available :
modetest -M xlnx -s 39:1920x1080-59.94@BG24 -v
NOTE: Above command was tested on xilinx DRM driver with DP
monitor which was supporting mode having 59.94 Hz refresh rate.
V2: Update commit message
V3: Update with below changes as per review comments :
1) Use epsilon for vrefresh comparison
2) Use implicit type-casting wherever possible
V4: Keep patch version history on main commit message
Signed-off-by: Devarsh Thakkar <devarsh.thakkar@xilinx.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Instead of hacking the binary every time, we can now specify directly.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
This includes logic to configure the LUT accordingly.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
As new features are added and others are declared to be legacy, it's
nice to be able to implement fallbacks. As such, create a
property-setting variant that does not generate errors which can very
well be entirely expected.
Will be used for gamma control in a future change.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
If "-a" option is set this make modetest use atomic API instead
of legacy API.
Test the frame rate ("-v") it does a loop and swap between two
framebuffer for each active planes.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
This will prevent any more missing `#include "config.h"` bug, at the
cost of having to recompile some files that didn't need to be when
changing build options.
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
The existing format modifier definitions were merged prematurely, and
recent work has unveiled that the definitions are suboptimal in several
ways:
- The format specifiers, except for one, are not Tegra specific, but
the names don't reflect that.
- The number space is split into two, reserving 32 bits for some
"parameter" which most of the modifiers are not going to have.
- Symbolic names for the modifiers are not using the standard
DRM_FORMAT_MOD_* prefix, which makes them awkward to use.
- The vendor prefix NV is somewhat ambiguous.
Fortunately, nobody's started using these modifiers, so we can still fix
the above issues. Do so by using the standard prefix. Also, remove TEGRA
from the name of those modifiers that exist on NVIDIA GPUs as well. In
case of the block linear modifiers, make the "parameter" smaller (4
bits, though only 6 values are valid) and don't let that leak into any
of the other modifiers.
Finally, also use the more canonical NVIDIA instead of the ambiguous NV
prefix.
This is based on commit 5843f4e02fbe86a59981e35adc6cabebee46fdc0 from
Linux v4.16-rc1 and also updates modetest to use the new defines.
Acked-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
There is warning about ignoring return value of 'asprintf'. Fix to
check return value of asprintf().
Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Geert Uytterhoeven
Chia-I Wu
Marijn Suijten
Neil Armstrong
Ezequiel Garcia
Dmitry Baryshkov
Rohith Iyer
Simon Ser
Eleni Maria Stea
Luigi Santivetti
Alex Richardson
ZhiJie.Zhang
Marius Vlad
Nicolas Caramelli
Emil Velikov
Emil Velikov
Eric Engestrom
John Stultz
Devarsh Thakkar
Ilia Mirkin
Fritz Koenig
Benjamin Gaignard
Eric Engestrom
Thierry Reding
Seung-Woo Kim