lkml.org 
[lkml]   [2021]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
    From
    Hi,

    On 12/20/21 23:28, Rajat Jain wrote:
    > Add a static entry in the x86 table, to detect and wait for
    > privacy-screen on some ChromeOS platforms.
    >
    > Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
    > enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
    > shall return EPROBE_DEFER until a platform driver actually registers the
    > privacy-screen: https://hansdegoede.livejournal.com/25948.html
    >
    > Signed-off-by: Rajat Jain <rajatja@google.com>
    > ---
    > v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
    > enhance the one already present in drm_privacy_screen_lookup_init()
    > v2: * Use #if instead of #elif
    > * Reorder the patches in the series.
    > * Rebased on drm-tip
    >
    > drivers/gpu/drm/drm_privacy_screen_x86.c | 23 ++++++++++++++++++++++-
    > 1 file changed, 22 insertions(+), 1 deletion(-)
    >
    > diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
    > index a2cafb294ca6..0fdd2b500e6d 100644
    > --- a/drivers/gpu/drm/drm_privacy_screen_x86.c
    > +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
    > @@ -47,6 +47,16 @@ static bool __init detect_thinkpad_privacy_screen(void)
    > }
    > #endif
    >
    > +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
    > +static bool __init detect_chromeos_privacy_screen(void)
    > +{
    > + if (!acpi_dev_present("GOOG0010", NULL, -1))
    > + return false;
    > +
    > + return true;

    This can be simplified to just:

    return acpi_dev_present("GOOG0010", NULL, -1);

    > +}
    > +#endif
    > +
    > static const struct arch_init_data arch_init_data[] __initconst = {
    > #if IS_ENABLED(CONFIG_THINKPAD_ACPI)
    > {
    > @@ -58,6 +68,16 @@ static const struct arch_init_data arch_init_data[] __initconst = {
    > .detect = detect_thinkpad_privacy_screen,
    > },
    > #endif
    > +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
    > + {
    > + .lookup = {
    > + .dev_id = NULL,
    > + .con_id = NULL,
    > + .provider = "privacy_screen-GOOG0010:00",
    > + },
    > + .detect = detect_chromeos_privacy_screen,
    > + },
    > +#endif
    > };
    >
    > void __init drm_privacy_screen_lookup_init(void)
    > @@ -68,7 +88,8 @@ void __init drm_privacy_screen_lookup_init(void)
    > if (!arch_init_data[i].detect())
    > continue;
    >
    > - pr_info("Found '%s' privacy-screen provider\n",
    > + pr_info("Found '%s' privacy-screen provider."
    > + "Might have to defer probe for it...\n",
    > arch_init_data[i].lookup.provider);

    I'm afraid this change in the log message will only confuse users,
    and for your goal of checking if a privacy-screen provider has
    been detected, the original message is good enough.

    Please drop this part of the patch.

    Regards,

    Hans



    >
    > /* Make a copy because arch_init_data is __initconst */
    >

    \
     
     \ /
      Last update: 2021-12-21 19:53    [W:8.174 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site