On Mon, Dec 9, 2019 at 7:05 AM Tomi Valkeinen tomi.valkeinen@ti.com wrote:
On 09/12/2019 16:38, Andrey Smirnov wrote:
On Mon, Dec 9, 2019 at 1:38 AM Tomi Valkeinen tomi.valkeinen@ti.com wrote:
(Cc'ing Daniel for the last paragraph)
On 09/12/2019 07:08, Andrey Smirnov wrote:
Presently, the driver code artificially limits test pattern mode to a single pattern with fixed color selection. It being a kernel module parameter makes switching "test pattern" <-> "proper output" modes on-the-fly clunky and outright impossible if the driver is built into the kernel.
That's not correct, /sys/module/tc358767/parameters/test is there even if the driver is built-in.
True, I'll drop the "impossible" part of the descrption. Having to unbind and bind device to the driver to use that parameter definitely falls under "clunky" for me still, so I'll just stick to that in the description.
You don't need to re-bind. You can change the module parameter at runtime, and if the driver happens to use the value, then it uses the new value. If I recall right, changing the module parameter and then doing a full modeset from userspace made the driver to use the test mode (I'm not 100% sure, though).
In any case, I'm not advocating for the use of module parameter here =)
Hmm, actually, just echoing 0 to tstctl multiple times, it makes the screen go black and then restores it with every other echo.
Strange, works on my setup every time. No error messages in kernel log I assume? Probably unrelated, but when you echo "0" and the screen
No errors.
stays black, what do you see in DP_SINK_STATUS register:
dd if=/dev/drm_dp_aux0 bs=1 skip=$((0x205)) count=1 2>/dev/null | hexdump -Cv
? Note that this needs CONFIG_DRM_DP_AUX_CHARDEV to be enabled.
I'll check this later, and do a few more tests.
debugfs = debugfs_create_dir(dev_name(dev), NULL);if (!IS_ERR(debugfs)) {debugfs_create_file_unsafe("tstctl", 0200, debugfs, tc,&tc_tstctl_fops);devm_add_action_or_reset(dev, tc_remove_debugfs, debugfs);}For me this creates debugfs/3-000f/tstctl. I don't think that's a clear or usable path, and could even cause a name conflict in the worst case.
I agree on usability aspect, but I am not sure I can see how a conflict can happen. What scenario do you have in mind that would cause that? My thinking was that the combination of I2C bus number + I2C address should always be unique on the system, but maybe I am missing something?
Well, the dir name doesn't have "i2c" anywhere, so at least in theory, some other bus could have "3-000f" address too.
Maybe bigger problem is that it's not at all obvious what "3-000f" means. All the other debugfs dirs make sense when you look at the name, and "3-000f" looks very odd there.
Fair enough, so what if we changed the name say "tc358767-3-000f" (i. e. used "tc358767-" + dev_name(dev)), would that be a reasonable path forward?
Thanks, Andrey Smirnov