[PATCH] drm/i915/display: fix display param dup for NULL char * params

[Lucas De Marchi]

On Tue, Apr 02, 2024 at 06:55:34PM +0300, Jani Nikula wrote:
>The display param duplication deviates from the original param
>duplication in that it converts NULL params to to allocated empty
>strings. This works for the vbt_firmware parameter, but not for
>dmc_firmware_path, the user of which interprets NULL and the empty
>string as distinct values. Specifically, the empty dmc_firmware_path
>leads to DMC and PM being disabled.
>
>Just remove the NULL check and pass it to kstrdup(), which safely
>returns NULL for NULL input.
>
>Fixes: 8015bee0bfec ("drm/i915/display: Add framework to add parameters specific to display")
>Fixes: 0d82a0d6f556 ("drm/i915/display: move dmc_firmware_path to display params")
>Cc: Jouni Högander <jouni.hogander at intel.com>
>Cc: Luca Coelho <luciano.coelho at intel.com>
>Cc: <stable at vger.kernel.org> # v6.8+
>Signed-off-by: Jani Nikula <jani.nikula at intel.com>

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

... but what's the purpose of the duplication?  How one is supposed to
use the dmc_firmware with e.g. LNL + BMG? Setting it later via debugfs
doesn´t change the behavior. AFAIR this was done to support multiple
devices, but I don't think it achieves its purpose or I'm missing
something.

By leaving a param writable and not duplicate it at all, we are at
least be allowed to:

1) disable autoprobe
2) load module
3) bind do LNL
4) set dmc_firmware param
5) bind to BMG

Yeah, it's manual and not intuitive, but should only be used by
developers with targeted debug.  How would we do something like that
with the current code?

I know that for params via sysfs, it's impossible to get them back to
NULL, so I think we should make sure NULL and empty is handled the same
way. Getting it back to empty is hard enough but at least possible (see
https://lore.kernel.org/igt-dev/20240228223134.3908035-4-lucas.demarchi@intel.com/),
but I think this is not the case for debugfs.

Lucas De Marchi