lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJpoMC-YbWvyfCsdAHOL9aw3nfQ=g8BgLp2mb9iozeRgBpg@mail.gmail.com>
Date:   Wed, 2 Aug 2023 12:05:50 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     neil.armstrong@...aro.org
Cc:     David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Robert Foss <rfoss@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Simon Ser <contact@...rsion.fr>, Janne Grunau <j@...nau.net>,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>,
        "Pan, Xinhui" <Xinhui.Pan@....com>,
        Harry Wentland <harry.wentland@....com>,
        Leo Li <sunpeng.li@....com>,
        Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, intel-gfx@...ts.freedesktop.org
Subject: Re: [PATCH 2/4] drm/bridge-connector: handle subconnector types

On Wed, 2 Aug 2023 at 11:35, Neil Armstrong <neil.armstrong@...aro.org> wrote:
>
> On 29/07/2023 02:49, Dmitry Baryshkov wrote:
> > If the created connector type supports subconnector type property,
> > create and attach corresponding it. The default subtype value is 0,
> > which maps to the DRM_MODE_SUBCONNECTOR_Unknown type.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > ---
> >   drivers/gpu/drm/drm_bridge_connector.c | 33 +++++++++++++++++++++++++-
> >   include/drm/drm_bridge.h               |  4 ++++
> >   2 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
> > index 07b5930b1282..a7b92f0d2430 100644
> > --- a/drivers/gpu/drm/drm_bridge_connector.c
> > +++ b/drivers/gpu/drm/drm_bridge_connector.c
> > @@ -329,7 +329,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >       struct drm_connector *connector;
> >       struct i2c_adapter *ddc = NULL;
> >       struct drm_bridge *bridge, *panel_bridge = NULL;
> > +     enum drm_mode_subconnector subconnector;
> >       int connector_type;
> > +     int ret;
> >
> >       bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL);
> >       if (!bridge_connector)
> > @@ -365,8 +367,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >               if (bridge->ops & DRM_BRIDGE_OP_MODES)
> >                       bridge_connector->bridge_modes = bridge;
> >
> > -             if (!drm_bridge_get_next_bridge(bridge))
> > +             if (!drm_bridge_get_next_bridge(bridge)) {
> >                       connector_type = bridge->type;
> > +                     subconnector = bridge->subtype;
> > +             }
> >
> >   #ifdef CONFIG_OF
> >               if (!drm_bridge_get_next_bridge(bridge) &&
> > @@ -399,6 +403,33 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >       if (panel_bridge)
> >               drm_panel_bridge_set_orientation(connector, panel_bridge);
> >
> > +     if (connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> > +             drm_connector_attach_dp_subconnector_property(connector, subconnector);
> > +     } else if (connector_type == DRM_MODE_CONNECTOR_DVII) {
> > +             ret = drm_mode_create_dvi_i_properties(drm);
> > +             if (ret)
> > +                     return ERR_PTR(ret);
> > +
> > +             drm_object_attach_property(&connector->base,
> > +                                        drm->mode_config.dvi_i_subconnector_property,
> > +                                        subconnector);
> > +     } else if (connector_type == DRM_MODE_CONNECTOR_TV) {
> > +             ret = drm_mode_create_tv_properties(drm,
> > +                                                 BIT(DRM_MODE_TV_MODE_NTSC) |
> > +                                                 BIT(DRM_MODE_TV_MODE_NTSC_443) |
> > +                                                 BIT(DRM_MODE_TV_MODE_NTSC_J) |
> > +                                                 BIT(DRM_MODE_TV_MODE_PAL) |
> > +                                                 BIT(DRM_MODE_TV_MODE_PAL_M) |
> > +                                                 BIT(DRM_MODE_TV_MODE_PAL_N) |
> > +                                                 BIT(DRM_MODE_TV_MODE_SECAM));
> > +             if (ret)
> > +                     return ERR_PTR(ret);
>
> I don't think this is right, this should be called from the appropriate encoder
> device depending on the analog tv mode capabilities.

Good question. My logic was the following: the DRM device can have
different TV out ports with different capabilities (yeah, pure
theoretical construct). In this case it might be impossible to create
a single subset of values. Thus it is more correct to create the
property listing all possible values. The property is immutable anyway
(and so the user doesn't have control over the value).


> > +
> > +             drm_object_attach_property(&connector->base,
> > +                                        drm->mode_config.tv_subconnector_property,
> > +                                        subconnector);
>
> Here, only add the property if drm->mode_config.tv_subconnector_property exists,
> and perhaps add a warning if not.

This property is created in the previous call,
drm_mode_create_tv_properties() ->
drm_mode_create_tv_properties_legacy().

>
> AFAIK same for DRM_MODE_CONNECTOR_DVII.
>
> > +     }
> > +
> >       return connector;
> >   }
> >   EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index bf964cdfb330..68b14ac5ac0d 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -739,6 +739,10 @@ struct drm_bridge {
> >        * identifies the type of connected display.
> >        */
> >       int type;
> > +     /**
> > +      * @subtype: the subtype of the connector for the DP/TV/DVI-I cases.
> > +      */
> > +     enum drm_mode_subconnector subtype;
> >       /**
> >        * @interlace_allowed: Indicate that the bridge can handle interlaced
> >        * modes.
>


-- 
With best wishes
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ