> On 13 Mar 2025, at 4:18 PM, Petr Mladek <pmladek@xxxxxxxx> wrote: > > Adding Kees into Cc to resolve how to get this patch into the mainline. > > On Thu 2025-03-13 09:13:23, Aditya Garg wrote: >> >> >>> On 13 Mar 2025, at 2:27 PM, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: >>> >>> On Thu, Mar 13, 2025 at 08:53:28AM +0000, Aditya Garg wrote: >>>>>> On 13 Mar 2025, at 2:19 PM, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: >>>>> On Thu, Mar 13, 2025 at 07:26:05AM +0000, Aditya Garg wrote: >>>>>>>> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: >>>>>>> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote: >>>>>>>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@xxxxxxxxxxxxx> wrote: >>>>>>>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote: >>> >>> ... >>> >>>>>>>>> I don't have a strong opinion either way: for SMC I just need to print >>>>>>>>> FourCC keys for debugging / information in a few places. >>>>>>>>> >>>>>>>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-() >>>>>>>>> and was just going to use macros to print the SMC FourCC keys similar to >>>>>>>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit >>>>>>>>> the topic later. >>>>>>>>> >>>>>>>>> Right now I have these in my local tree (only compile tested so far): >>>>>>>>> >>>>>>>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)" >>>>>>>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k) >>>>>>>> >>>>>>>> That seems to be a nice alternative, which I guess Thomas was also suggesting. >>>>>>> >>>>>>> I don't think it's "nice". Each of the approaches has pros and cons. >>>>>>> You can start from bloat-o-meter here and compare it with your %p extension. >>>>>>> >>>>>>> Also, can you show the bloat-o-meter output for the vsprintf.c? >>>>>> >>>>>> Here are your outputs: >>>>> >>>>> Thank you! >>>>> >>>>>> --------------------------------------------------------------------- >>>>>> For appletbdrm: >>>>>> >>>>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO >>>>>> add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45) >>>>>> Function old new delta >>>>>> appletbdrm_read_response 395 459 +64 >>>>>> appletbdrm_probe 1786 1767 -19 >>>>>> Total: Before=13418, After=13463, chg +0.34% >>>>> >>>>> This is enough, no need to repeat this for every parameter. >>>>> >>>>>> --------------------------------------------------------------------- >>>>>> For vsprintf: >>>>>> >>>>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW >>>>>> add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220) >>>>>> Function old new delta >>>>>> fourcc_string 479 699 +220 >>>>>> Total: Before=26454, After=26674, chg +0.83% >>>>> >>>>> So, we get +220 bytes vs +43 bytes. It means if we found 5+ users, it worth >>>>> doing. >>>> >>>> Will it also depend upon the number of times it's being used? In appletbdrm, >>>> it is being used 3 times. Probably more in Asahi SMC. >>> >>> Right, it depends on the usage count. Also on different architectures it may >>> give different results. On 32-bit it probably gives better statistics. >> >> Best to go ahead with vsprintf then. Petr, are you still there? > > I am here but there were many other things in the queue ;-) > > I do not have strong opinion. I am not familiar with the FourCC > format and it looks like a magic to me. But it seems that it makes > sense for the users. > > I personally find the %pcX modifiers a bit less hacky than > the two macros SMC_KEY_FMT/SMC_KEY_ARG. > > So I am fine with this patch: > > Reviewed-by: Petr Mladek <pmladek@xxxxxxxx> > Tested-by: Petr Mladek <pmladek@xxxxxxxx> > > > Now, the question is how to get this patch into the mainline. > > Normally, it would make perfect sense to queue it via the DRM tree > because drivers/gpu/drm/tiny/appletbdrm.c is a new driver... > > But this time there is a conflicting patchset which is reworking > the entire lib/test_printf.c file, see > 20250307-printf-kunit-convert-v6-0-4d85c361c241@xxxxxxxxx Link seems to be broken > And it will likely be ready for the next merge window as well. > I am going to review it right away. > > It is even more complicated because the patchset converting > the printf test module to KUNIT depends on another changes > in Kees' tree (moving kunit test modules to lib/tests/). > So it might be easier when it goes via Kees' tree. > > And it might be easier when even this patch goes via Kees' tree. > > My proposal: > > I suggest to separate the fourcc_pointer() test update > to a separate patch and add it later after the merge window > when things settle down. > > I mean to send the vsprintf.c, checkpatch.pl, and doc update > via DRM tree together with the new appletbdrm.c driver. Sounds good. At least we can get it working. I’ll make sure the self tests get updated once 6.15-rc1 gets released, or Kees can share his tree, where I can add the tests as well. I’ll send a v2 so that Thomas can take them up. > > And update the selftest later when both DRM tree and KUNIT > update reaches mainline. > > How does that sound, please? > > Best Regards, > Petr
