Re: [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

On Monday, 9 April 2018 12:18:28 EEST Daniel Vetter wrote:
> On Mon, Apr 09, 2018 at 11:56:55AM +0300, Laurent Pinchart wrote:
> > On Monday, 9 April 2018 11:53:07 EEST Daniel Vetter wrote:
> >> On Fri, Apr 06, 2018 at 07:23:57PM +0300, Laurent Pinchart wrote:
> >>> The mode and ajusted_mode passed to the bridge .mode_set() operation
> >>> should never be modified by the bridge (and are not in any of the
> >>> existing bridge drivers). Make them const to make this clear.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >> > <laurent.pinchart+renesas@ideasonboard.com>
> >> 
> >> Why should the adjusted_mode not be modified?
> > 
> > Why should it ? .mode_set() performs a mode set, mode adjustment should be
> > performed in .mode_fixup().
> 
> Oh right, totally missed that you're only touching the ->mode_set
> callbacks. Assuming gcc is happy (not going to check for that, too lazy):
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I've compile-tested this patch for all the drivers it touches and gcc didn't 
complain. I'll give 0day a few days to give it a go too, just in case.

> >>> ---
> >>> 
> >>>  drivers/gpu/drm/bridge/adv7511/adv7511.h           |  4 ++--
> >>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c       |  8 ++++----
> >>>  drivers/gpu/drm/bridge/adv7511/adv7533.c           |  2 +-
> >>>  drivers/gpu/drm/bridge/analogix-anx78xx.c          |  4 ++--
> >>>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 ++--
> >>>  drivers/gpu/drm/bridge/sii902x.c                   |  4 ++--
> >>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c          |  4 ++--
> >>>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c      | 16 ++++++++-------
> >>>  drivers/gpu/drm/bridge/tc358767.c                  |  9 +++++----
> >>>  drivers/gpu/drm/drm_bridge.c                       |  4 ++--
> >>>  drivers/gpu/drm/exynos/exynos_drm_mic.c            |  4 ++--
> >>>  drivers/gpu/drm/mediatek/mtk_hdmi.c                |  4 ++--
> >>>  drivers/gpu/drm/msm/dsi/dsi.h                      |  2 +-
> >>>  drivers/gpu/drm/msm/dsi/dsi_host.c                 |  2 +-
> >>>  drivers/gpu/drm/msm/dsi/dsi_manager.c              |  4 ++--
> >>>  drivers/gpu/drm/msm/edp/edp_bridge.c               |  4 ++--
> >>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c             |  4 ++--
> >>>  drivers/gpu/drm/rcar-du/rcar_lvds.c                |  4 ++--
> >>>  drivers/gpu/drm/sti/sti_dvo.c                      |  4 ++--
> >>>  drivers/gpu/drm/sti/sti_hda.c                      |  4 ++--
> >>>  drivers/gpu/drm/sti/sti_hdmi.c                     |  4 ++--
> >>>  drivers/gpu/drm/stm/dw_mipi_dsi-stm.c              |  2 +-
> >>>  include/drm/bridge/dw_mipi_dsi.h                   |  3 ++-
> >>>  include/drm/drm_bridge.h                           |  8 ++++----
> >>>  24 files changed, 57 insertions(+), 55 deletions(-)
> >>> 
> >>> Philippe,
> >>> 
> >>> I wrote this patch while reviewing your adjusted_mode documentation. I
> >>> initially wanted to document the fact that the adjusted_mode argument
> >>> to the bridge .mode_set() operation should not be modified by the
> >>> bridge, and then realized that constifying it would be a better way to
> >>> express that. I thus wouldn't mind if you took that patch in your tree
> >>> and pushed it with the documentation patch once we get the necessary
> >>> acks :-)

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel