lkml.org 
[lkml]   [2022]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v6 8/8] drm: Config orientation property if panel provides it
    On Wed, Jun 8, 2022 at 10:17 PM Doug Anderson <dianders@chromium.org> wrote:
    >
    > Hi,
    >
    > On Wed, Jun 8, 2022 at 2:48 AM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
    > >
    > > @@ -269,6 +280,31 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge)
    > > }
    > > EXPORT_SYMBOL(drm_panel_bridge_remove);
    > >
    > > +/**
    > > + * drm_panel_bridge_set_orientation - Set the connector's panel orientation
    > > + * if the bridge is a panel bridge.
    > > + *
    > > + * @connector: The connector to be set panel orientation.
    > > + * @bridge: The drm_bridge to be transformed to panel bridge.
    >
    > Ideally you should have a kernel doc to describe what you're returning.
    >
    >
    > > + */
    > > +int drm_panel_bridge_set_orientation(struct drm_connector *connector,
    > > + struct drm_bridge *bridge)
    > > +{
    > > + struct panel_bridge *panel_bridge;
    > > +
    > > + if (!bridge)
    > > + return 0;
    > > +
    > > + if (bridge->funcs != &panel_bridge_bridge_funcs)
    > > + return 0;
    >
    > nit: Why do you need to handle NULL bridge and the case that someone
    > calls you with something other than a panel-bridge? I'm not convinced
    > that's useful. In general kernel style doesn't do massive validation
    > of parameters unless there's a reason for it. ...if we do need to
    > handle them then it feels like they should be returning -EINVAL or
    > something, not 0.

    The only caller had checked it. I can remove the check here.

    >
    >
    > > @@ -917,10 +917,13 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
    > > enum drm_connector_status status);
    > >
    > > #ifdef CONFIG_DRM_PANEL_BRIDGE
    > > +bool drm_bridge_is_panel(const struct drm_bridge *bridge);
    > > struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
    > > struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,
    > > u32 connector_type);
    > > void drm_panel_bridge_remove(struct drm_bridge *bridge);
    > > +int drm_panel_bridge_set_orientation(struct drm_connector *connector,
    > > + struct drm_bridge *bridge);
    >
    > I suspect that you need some dummy versions of your new functions
    > defined if "CONFIG_DRM_PANEL_BRIDGE" is not defined. Otherwise we're
    > going to be yelled at by the kernel robot eventually.
    >
    > -Doug

    \
     
     \ /
      Last update: 2022-06-09 05:14    [W:9.496 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site