* [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation
@ 2018-04-06 16:23 Laurent Pinchart
2018-04-09 8:53 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2018-04-06 16:23 UTC (permalink / raw)
To: dri-devel
Cc: Yannick Fertre, Seung-Woo Kim, Philippe Cornu, Kyungmin Park,
Vincent Abriou
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>
---
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
:-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index d034b2cb5eee..f60d29defd18 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -389,7 +389,7 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
#ifdef CONFIG_DRM_I2C_ADV7533
void adv7533_dsi_power_on(struct adv7511 *adv);
void adv7533_dsi_power_off(struct adv7511 *adv);
-void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode);
+void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode);
int adv7533_patch_registers(struct adv7511 *adv);
int adv7533_patch_cec_registers(struct adv7511 *adv);
int adv7533_attach_dsi(struct adv7511 *adv);
@@ -405,7 +405,7 @@ static inline void adv7533_dsi_power_off(struct adv7511 *adv)
}
static inline void adv7533_mode_set(struct adv7511 *adv,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *mode)
{
}
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index efa29db5fc2b..99028d36ed74 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -664,8 +664,8 @@ static int adv7511_mode_valid(struct adv7511 *adv7511,
}
static void adv7511_mode_set(struct adv7511 *adv7511,
- struct drm_display_mode *mode,
- struct drm_display_mode *adj_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adj_mode)
{
unsigned int low_refresh_rate;
unsigned int hsync_polarity = 0;
@@ -827,8 +827,8 @@ static void adv7511_bridge_disable(struct drm_bridge *bridge)
}
static void adv7511_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adj_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adj_mode)
{
struct adv7511 *adv = bridge_to_adv7511(bridge);
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index 185b6d842166..5d5e7d9eded2 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -108,7 +108,7 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
regmap_write(adv->regmap_cec, 0x27, 0x0b);
}
-void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode)
+void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode)
{
struct mipi_dsi_device *dsi = adv->dsi;
int lanes, ret;
diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index b49043866be6..2a7d80da7fb7 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1082,8 +1082,8 @@ static void anx78xx_bridge_disable(struct drm_bridge *bridge)
}
static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct anx78xx *anx78xx = bridge_to_anx78xx(bridge);
struct hdmi_avi_infoframe frame;
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5c52307146c7..609da026e713 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1202,8 +1202,8 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
}
static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *orig_mode,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *orig_mode,
+ const struct drm_display_mode *mode)
{
struct analogix_dp_device *dp = bridge->driver_private;
struct drm_display_info *display_info = &dp->connector.display_info;
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 60373d7eb220..9320ff1116f3 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -254,8 +254,8 @@ static void sii902x_bridge_enable(struct drm_bridge *bridge)
}
static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adj)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adj)
{
struct sii902x *sii902x = bridge_to_sii902x(bridge);
struct regmap *regmap = sii902x->regmap;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ec8d0006ef7c..40dcf5172149 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1998,8 +1998,8 @@ dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
}
static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *orig_mode,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *orig_mode,
+ const struct drm_display_mode *mode)
{
struct dw_hdmi *hdmi = bridge->driver_private;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 226171a3ece1..13df70bcfe94 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -241,7 +241,7 @@ struct dw_mipi_dsi {
* The controller should generate 2 frames before
* preparing the peripheral.
*/
-static void dw_mipi_dsi_wait_for_two_frames(struct drm_display_mode *mode)
+static void dw_mipi_dsi_wait_for_two_frames(const struct drm_display_mode *mode)
{
int refresh, two_frames;
@@ -535,7 +535,7 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
}
static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *mode)
{
u32 val = 0, color = 0;
@@ -578,7 +578,7 @@ static void dw_mipi_dsi_packet_handler_config(struct dw_mipi_dsi *dsi)
}
static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *mode)
{
/*
* TODO dw drv improvements
@@ -609,7 +609,7 @@ static void dw_mipi_dsi_command_mode_config(struct dw_mipi_dsi *dsi)
/* Get lane byte clock cycles. */
static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi,
- struct drm_display_mode *mode,
+ const struct drm_display_mode *mode,
u32 hcomponent)
{
u32 frac, lbcc;
@@ -625,7 +625,7 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi,
}
static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *mode)
{
u32 htotal, hsa, hbp, lbcc;
@@ -648,7 +648,7 @@ static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
}
static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *mode)
{
u32 vactive, vsa, vfp, vbp;
@@ -765,8 +765,8 @@ static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
}
static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 08ab7d6aea65..535fcd83f313 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -203,7 +203,7 @@ struct tc_data {
/* display edid */
struct edid *edid;
/* current mode */
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
u32 rev;
u8 assr;
@@ -648,7 +648,8 @@ static int tc_get_display_props(struct tc_data *tc)
return ret;
}
-static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
+static int tc_set_video_mode(struct tc_data *tc,
+ const struct drm_display_mode *mode)
{
int ret;
int vid_sync_dly;
@@ -1113,8 +1114,8 @@ static int tc_connector_mode_valid(struct drm_connector *connector,
}
static void tc_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adj)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adj)
{
struct tc_data *tc = bridge_to_tc(bridge);
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 1638bfe9627c..c1dadb0a5d36 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -290,8 +290,8 @@ EXPORT_SYMBOL(drm_bridge_post_disable);
* Note: the bridge passed should be the one closest to the encoder
*/
void drm_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
if (!bridge)
return;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index 2174814273e2..5ee3d9d9cd5d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -246,8 +246,8 @@ static void mic_post_disable(struct drm_bridge *bridge)
}
static void mic_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct exynos_mic *mic = bridge->driver_private;
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 59a11026dceb..02127ddf340b 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1364,8 +1364,8 @@ static void mtk_hdmi_bridge_post_disable(struct drm_bridge *bridge)
}
static void mtk_hdmi_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 70d9a9a47acd..9730e5a6e1a7 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -165,7 +165,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
struct msm_dsi_phy_shared_timings *phy_shared_timings);
int msm_dsi_host_power_off(struct mipi_dsi_host *host);
int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
- struct drm_display_mode *mode);
+ const struct drm_display_mode *mode);
struct drm_panel *msm_dsi_host_get_panel(struct mipi_dsi_host *host,
unsigned long *panel_flags);
struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 7a03a9489708..9c2dd91348ac 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2329,7 +2329,7 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
}
int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *mode)
{
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 4cb1cb68878b..b742ef7d09ab 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -603,8 +603,8 @@ static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge)
}
static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
int id = dsi_mgr_bridge_get_id(bridge);
struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
index 931a5c97cccf..86366ba03e60 100644
--- a/drivers/gpu/drm/msm/edp/edp_bridge.c
+++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
@@ -52,8 +52,8 @@ static void edp_bridge_post_disable(struct drm_bridge *bridge)
}
static void edp_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct drm_device *dev = bridge->dev;
struct drm_connector *connector;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 7e357077ed26..50e00ec153bf 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -207,8 +207,8 @@ static void msm_hdmi_bridge_post_disable(struct drm_bridge *bridge)
}
static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 3d2d3bbd1342..802c5e8867e6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -318,8 +318,8 @@ static void rcar_lvds_get_lvds_mode(struct rcar_lvds *lvds)
}
static void rcar_lvds_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
index a5979cd25cc7..b6a70c605125 100644
--- a/drivers/gpu/drm/sti/sti_dvo.c
+++ b/drivers/gpu/drm/sti/sti_dvo.c
@@ -277,8 +277,8 @@ static void sti_dvo_pre_enable(struct drm_bridge *bridge)
}
static void sti_dvo_set_mode(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct sti_dvo *dvo = bridge->driver_private;
struct sti_mixer *mixer = to_sti_mixer(dvo->encoder->crtc);
diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
index 67bbdb49fffc..9ec0195df46c 100644
--- a/drivers/gpu/drm/sti/sti_hda.c
+++ b/drivers/gpu/drm/sti/sti_hda.c
@@ -508,8 +508,8 @@ static void sti_hda_pre_enable(struct drm_bridge *bridge)
}
static void sti_hda_set_mode(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct sti_hda *hda = bridge->driver_private;
u32 mode_idx;
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 58f431102512..08953c539980 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -917,8 +917,8 @@ static void sti_hdmi_pre_enable(struct drm_bridge *bridge)
}
static void sti_hdmi_set_mode(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
struct sti_hdmi *hdmi = bridge->driver_private;
int ret;
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index a514b593f37c..a672b59a2226 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -215,7 +215,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
}
static int
-dw_mipi_dsi_get_lane_mbps(void *priv_data, struct drm_display_mode *mode,
+dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode,
unsigned long mode_flags, u32 lanes, u32 format,
unsigned int *lane_mbps)
{
diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
index d9c6d549f971..3e7bf033c1f3 100644
--- a/include/drm/bridge/dw_mipi_dsi.h
+++ b/include/drm/bridge/dw_mipi_dsi.h
@@ -14,7 +14,8 @@ struct dw_mipi_dsi;
struct dw_mipi_dsi_phy_ops {
int (*init)(void *priv_data);
- int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode,
+ int (*get_lane_mbps)(void *priv_data,
+ const struct drm_display_mode *mode,
unsigned long mode_flags, u32 lanes, u32 format,
unsigned int *lane_mbps);
};
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 3270fec46979..661c54cd062e 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -180,8 +180,8 @@ struct drm_bridge_funcs {
* called.
*/
void (*mode_set)(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode);
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode);
/**
* @pre_enable:
*
@@ -292,8 +292,8 @@ enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
void drm_bridge_disable(struct drm_bridge *bridge);
void drm_bridge_post_disable(struct drm_bridge *bridge);
void drm_bridge_mode_set(struct drm_bridge *bridge,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode);
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode);
void drm_bridge_pre_enable(struct drm_bridge *bridge);
void drm_bridge_enable(struct drm_bridge *bridge);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation
2018-04-06 16:23 [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation Laurent Pinchart
@ 2018-04-09 8:53 ` Daniel Vetter
2018-04-09 8:56 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2018-04-09 8:53 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Seung-Woo Kim, Philippe Cornu, dri-devel, Yannick Fertre,
Kyungmin Park, Vincent Abriou
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?
-Daniel
> ---
> 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
> :-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index d034b2cb5eee..f60d29defd18 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -389,7 +389,7 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
> #ifdef CONFIG_DRM_I2C_ADV7533
> void adv7533_dsi_power_on(struct adv7511 *adv);
> void adv7533_dsi_power_off(struct adv7511 *adv);
> -void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode);
> +void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode);
> int adv7533_patch_registers(struct adv7511 *adv);
> int adv7533_patch_cec_registers(struct adv7511 *adv);
> int adv7533_attach_dsi(struct adv7511 *adv);
> @@ -405,7 +405,7 @@ static inline void adv7533_dsi_power_off(struct adv7511 *adv)
> }
>
> static inline void adv7533_mode_set(struct adv7511 *adv,
> - struct drm_display_mode *mode)
> + const struct drm_display_mode *mode)
> {
> }
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index efa29db5fc2b..99028d36ed74 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -664,8 +664,8 @@ static int adv7511_mode_valid(struct adv7511 *adv7511,
> }
>
> static void adv7511_mode_set(struct adv7511 *adv7511,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adj_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adj_mode)
> {
> unsigned int low_refresh_rate;
> unsigned int hsync_polarity = 0;
> @@ -827,8 +827,8 @@ static void adv7511_bridge_disable(struct drm_bridge *bridge)
> }
>
> static void adv7511_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adj_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adj_mode)
> {
> struct adv7511 *adv = bridge_to_adv7511(bridge);
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index 185b6d842166..5d5e7d9eded2 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -108,7 +108,7 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
> regmap_write(adv->regmap_cec, 0x27, 0x0b);
> }
>
> -void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode)
> +void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode)
> {
> struct mipi_dsi_device *dsi = adv->dsi;
> int lanes, ret;
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> index b49043866be6..2a7d80da7fb7 100644
> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> @@ -1082,8 +1082,8 @@ static void anx78xx_bridge_disable(struct drm_bridge *bridge)
> }
>
> static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct anx78xx *anx78xx = bridge_to_anx78xx(bridge);
> struct hdmi_avi_infoframe frame;
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 5c52307146c7..609da026e713 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1202,8 +1202,8 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
> }
>
> static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *orig_mode,
> - struct drm_display_mode *mode)
> + const struct drm_display_mode *orig_mode,
> + const struct drm_display_mode *mode)
> {
> struct analogix_dp_device *dp = bridge->driver_private;
> struct drm_display_info *display_info = &dp->connector.display_info;
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 60373d7eb220..9320ff1116f3 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -254,8 +254,8 @@ static void sii902x_bridge_enable(struct drm_bridge *bridge)
> }
>
> static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adj)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adj)
> {
> struct sii902x *sii902x = bridge_to_sii902x(bridge);
> struct regmap *regmap = sii902x->regmap;
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index ec8d0006ef7c..40dcf5172149 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1998,8 +1998,8 @@ dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
> }
>
> static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *orig_mode,
> - struct drm_display_mode *mode)
> + const struct drm_display_mode *orig_mode,
> + const struct drm_display_mode *mode)
> {
> struct dw_hdmi *hdmi = bridge->driver_private;
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 226171a3ece1..13df70bcfe94 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -241,7 +241,7 @@ struct dw_mipi_dsi {
> * The controller should generate 2 frames before
> * preparing the peripheral.
> */
> -static void dw_mipi_dsi_wait_for_two_frames(struct drm_display_mode *mode)
> +static void dw_mipi_dsi_wait_for_two_frames(const struct drm_display_mode *mode)
> {
> int refresh, two_frames;
>
> @@ -535,7 +535,7 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> }
>
> static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
> - struct drm_display_mode *mode)
> + const struct drm_display_mode *mode)
> {
> u32 val = 0, color = 0;
>
> @@ -578,7 +578,7 @@ static void dw_mipi_dsi_packet_handler_config(struct dw_mipi_dsi *dsi)
> }
>
> static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
> - struct drm_display_mode *mode)
> + const struct drm_display_mode *mode)
> {
> /*
> * TODO dw drv improvements
> @@ -609,7 +609,7 @@ static void dw_mipi_dsi_command_mode_config(struct dw_mipi_dsi *dsi)
>
> /* Get lane byte clock cycles. */
> static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi,
> - struct drm_display_mode *mode,
> + const struct drm_display_mode *mode,
> u32 hcomponent)
> {
> u32 frac, lbcc;
> @@ -625,7 +625,7 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi,
> }
>
> static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
> - struct drm_display_mode *mode)
> + const struct drm_display_mode *mode)
> {
> u32 htotal, hsa, hbp, lbcc;
>
> @@ -648,7 +648,7 @@ static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
> }
>
> static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> - struct drm_display_mode *mode)
> + const struct drm_display_mode *mode)
> {
> u32 vactive, vsa, vfp, vbp;
>
> @@ -765,8 +765,8 @@ static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
> }
>
> static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 08ab7d6aea65..535fcd83f313 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -203,7 +203,7 @@ struct tc_data {
> /* display edid */
> struct edid *edid;
> /* current mode */
> - struct drm_display_mode *mode;
> + const struct drm_display_mode *mode;
>
> u32 rev;
> u8 assr;
> @@ -648,7 +648,8 @@ static int tc_get_display_props(struct tc_data *tc)
> return ret;
> }
>
> -static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
> +static int tc_set_video_mode(struct tc_data *tc,
> + const struct drm_display_mode *mode)
> {
> int ret;
> int vid_sync_dly;
> @@ -1113,8 +1114,8 @@ static int tc_connector_mode_valid(struct drm_connector *connector,
> }
>
> static void tc_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adj)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adj)
> {
> struct tc_data *tc = bridge_to_tc(bridge);
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 1638bfe9627c..c1dadb0a5d36 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -290,8 +290,8 @@ EXPORT_SYMBOL(drm_bridge_post_disable);
> * Note: the bridge passed should be the one closest to the encoder
> */
> void drm_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> if (!bridge)
> return;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> index 2174814273e2..5ee3d9d9cd5d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> @@ -246,8 +246,8 @@ static void mic_post_disable(struct drm_bridge *bridge)
> }
>
> static void mic_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct exynos_mic *mic = bridge->driver_private;
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 59a11026dceb..02127ddf340b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1364,8 +1364,8 @@ static void mtk_hdmi_bridge_post_disable(struct drm_bridge *bridge)
> }
>
> static void mtk_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> index 70d9a9a47acd..9730e5a6e1a7 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> @@ -165,7 +165,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> struct msm_dsi_phy_shared_timings *phy_shared_timings);
> int msm_dsi_host_power_off(struct mipi_dsi_host *host);
> int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> - struct drm_display_mode *mode);
> + const struct drm_display_mode *mode);
> struct drm_panel *msm_dsi_host_get_panel(struct mipi_dsi_host *host,
> unsigned long *panel_flags);
> struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 7a03a9489708..9c2dd91348ac 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -2329,7 +2329,7 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
> }
>
> int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> - struct drm_display_mode *mode)
> + const struct drm_display_mode *mode)
> {
> struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index 4cb1cb68878b..b742ef7d09ab 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -603,8 +603,8 @@ static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge)
> }
>
> static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> int id = dsi_mgr_bridge_get_id(bridge);
> struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
> diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
> index 931a5c97cccf..86366ba03e60 100644
> --- a/drivers/gpu/drm/msm/edp/edp_bridge.c
> +++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
> @@ -52,8 +52,8 @@ static void edp_bridge_post_disable(struct drm_bridge *bridge)
> }
>
> static void edp_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct drm_device *dev = bridge->dev;
> struct drm_connector *connector;
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index 7e357077ed26..50e00ec153bf 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -207,8 +207,8 @@ static void msm_hdmi_bridge_post_disable(struct drm_bridge *bridge)
> }
>
> static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
> struct hdmi *hdmi = hdmi_bridge->hdmi;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 3d2d3bbd1342..802c5e8867e6 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -318,8 +318,8 @@ static void rcar_lvds_get_lvds_mode(struct rcar_lvds *lvds)
> }
>
> static void rcar_lvds_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
>
> diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> index a5979cd25cc7..b6a70c605125 100644
> --- a/drivers/gpu/drm/sti/sti_dvo.c
> +++ b/drivers/gpu/drm/sti/sti_dvo.c
> @@ -277,8 +277,8 @@ static void sti_dvo_pre_enable(struct drm_bridge *bridge)
> }
>
> static void sti_dvo_set_mode(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct sti_dvo *dvo = bridge->driver_private;
> struct sti_mixer *mixer = to_sti_mixer(dvo->encoder->crtc);
> diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> index 67bbdb49fffc..9ec0195df46c 100644
> --- a/drivers/gpu/drm/sti/sti_hda.c
> +++ b/drivers/gpu/drm/sti/sti_hda.c
> @@ -508,8 +508,8 @@ static void sti_hda_pre_enable(struct drm_bridge *bridge)
> }
>
> static void sti_hda_set_mode(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct sti_hda *hda = bridge->driver_private;
> u32 mode_idx;
> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index 58f431102512..08953c539980 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -917,8 +917,8 @@ static void sti_hdmi_pre_enable(struct drm_bridge *bridge)
> }
>
> static void sti_hdmi_set_mode(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> {
> struct sti_hdmi *hdmi = bridge->driver_private;
> int ret;
> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> index a514b593f37c..a672b59a2226 100644
> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> @@ -215,7 +215,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
> }
>
> static int
> -dw_mipi_dsi_get_lane_mbps(void *priv_data, struct drm_display_mode *mode,
> +dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode,
> unsigned long mode_flags, u32 lanes, u32 format,
> unsigned int *lane_mbps)
> {
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index d9c6d549f971..3e7bf033c1f3 100644
> --- a/include/drm/bridge/dw_mipi_dsi.h
> +++ b/include/drm/bridge/dw_mipi_dsi.h
> @@ -14,7 +14,8 @@ struct dw_mipi_dsi;
>
> struct dw_mipi_dsi_phy_ops {
> int (*init)(void *priv_data);
> - int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode,
> + int (*get_lane_mbps)(void *priv_data,
> + const struct drm_display_mode *mode,
> unsigned long mode_flags, u32 lanes, u32 format,
> unsigned int *lane_mbps);
> };
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 3270fec46979..661c54cd062e 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -180,8 +180,8 @@ struct drm_bridge_funcs {
> * called.
> */
> void (*mode_set)(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode);
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode);
> /**
> * @pre_enable:
> *
> @@ -292,8 +292,8 @@ enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> void drm_bridge_disable(struct drm_bridge *bridge);
> void drm_bridge_post_disable(struct drm_bridge *bridge);
> void drm_bridge_mode_set(struct drm_bridge *bridge,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode);
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode);
> void drm_bridge_pre_enable(struct drm_bridge *bridge);
> void drm_bridge_enable(struct drm_bridge *bridge);
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation
2018-04-09 8:53 ` Daniel Vetter
@ 2018-04-09 8:56 ` Laurent Pinchart
2018-04-09 9:18 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2018-04-09 8:56 UTC (permalink / raw)
To: Daniel Vetter
Cc: Laurent Pinchart, Seung-Woo Kim, Philippe Cornu, dri-devel,
Yannick Fertre, Kyungmin Park, Vincent Abriou
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().
>
> > ---
> >
> > 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
> >
> > :-)
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > b/drivers/gpu/drm/bridge/adv7511/adv7511.h index
> > d034b2cb5eee..f60d29defd18 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > @@ -389,7 +389,7 @@ static inline int adv7511_cec_init(struct device *dev,
> > struct adv7511 *adv7511)>
> > #ifdef CONFIG_DRM_I2C_ADV7533
> > void adv7533_dsi_power_on(struct adv7511 *adv);
> > void adv7533_dsi_power_off(struct adv7511 *adv);
> >
> > -void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode
> > *mode);
> > +void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode
> > *mode);>
> > int adv7533_patch_registers(struct adv7511 *adv);
> > int adv7533_patch_cec_registers(struct adv7511 *adv);
> > int adv7533_attach_dsi(struct adv7511 *adv);
> >
> > @@ -405,7 +405,7 @@ static inline void adv7533_dsi_power_off(struct
> > adv7511 *adv)>
> > }
> >
> > static inline void adv7533_mode_set(struct adv7511 *adv,
> >
> > - struct drm_display_mode *mode)
> > + const struct drm_display_mode *mode)
> >
> > {
> > }
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index
> > efa29db5fc2b..99028d36ed74 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > @@ -664,8 +664,8 @@ static int adv7511_mode_valid(struct adv7511 *adv7511,
> >
> > }
> >
> > static void adv7511_mode_set(struct adv7511 *adv7511,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adj_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adj_mode)
> >
> > {
> >
> > unsigned int low_refresh_rate;
> > unsigned int hsync_polarity = 0;
> >
> > @@ -827,8 +827,8 @@ static void adv7511_bridge_disable(struct drm_bridge
> > *bridge)>
> > }
> >
> > static void adv7511_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adj_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adj_mode)
> >
> > {
> >
> > struct adv7511 *adv = bridge_to_adv7511(bridge);
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > b/drivers/gpu/drm/bridge/adv7511/adv7533.c index
> > 185b6d842166..5d5e7d9eded2 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > @@ -108,7 +108,7 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
> >
> > regmap_write(adv->regmap_cec, 0x27, 0x0b);
> >
> > }
> >
> > -void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode)
> > +void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode
> > *mode)>
> > {
> >
> > struct mipi_dsi_device *dsi = adv->dsi;
> > int lanes, ret;
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > b/drivers/gpu/drm/bridge/analogix-anx78xx.c index
> > b49043866be6..2a7d80da7fb7 100644
> > --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > @@ -1082,8 +1082,8 @@ static void anx78xx_bridge_disable(struct drm_bridge
> > *bridge)>
> > }
> >
> > static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct anx78xx *anx78xx = bridge_to_anx78xx(bridge);
> > struct hdmi_avi_infoframe frame;
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index
> > 5c52307146c7..609da026e713 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -1202,8 +1202,8 @@ static void analogix_dp_bridge_disable(struct
> > drm_bridge *bridge)>
> > }
> >
> > static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *orig_mode,
> > - struct drm_display_mode *mode)
> > + const struct drm_display_mode *orig_mode,
> > + const struct drm_display_mode *mode)
> >
> > {
> >
> > struct analogix_dp_device *dp = bridge->driver_private;
> > struct drm_display_info *display_info = &dp->connector.display_info;
> >
> > diff --git a/drivers/gpu/drm/bridge/sii902x.c
> > b/drivers/gpu/drm/bridge/sii902x.c index 60373d7eb220..9320ff1116f3
> > 100644
> > --- a/drivers/gpu/drm/bridge/sii902x.c
> > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > @@ -254,8 +254,8 @@ static void sii902x_bridge_enable(struct drm_bridge
> > *bridge)>
> > }
> >
> > static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adj)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adj)
> >
> > {
> >
> > struct sii902x *sii902x = bridge_to_sii902x(bridge);
> > struct regmap *regmap = sii902x->regmap;
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > ec8d0006ef7c..40dcf5172149 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -1998,8 +1998,8 @@ dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
> >
> > }
> >
> > static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *orig_mode,
> > - struct drm_display_mode *mode)
> > + const struct drm_display_mode *orig_mode,
> > + const struct drm_display_mode *mode)
> >
> > {
> >
> > struct dw_hdmi *hdmi = bridge->driver_private;
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index
> > 226171a3ece1..13df70bcfe94 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > @@ -241,7 +241,7 @@ struct dw_mipi_dsi {
> >
> > * The controller should generate 2 frames before
> > * preparing the peripheral.
> > */
> >
> > -static void dw_mipi_dsi_wait_for_two_frames(struct drm_display_mode
> > *mode)
> > +static void dw_mipi_dsi_wait_for_two_frames(const struct drm_display_mode
> > *mode)>
> > {
> >
> > int refresh, two_frames;
> >
> > @@ -535,7 +535,7 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> >
> > }
> >
> > static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
> >
> > - struct drm_display_mode *mode)
> > + const struct drm_display_mode *mode)
> >
> > {
> >
> > u32 val = 0, color = 0;
> >
> > @@ -578,7 +578,7 @@ static void dw_mipi_dsi_packet_handler_config(struct
> > dw_mipi_dsi *dsi)>
> > }
> >
> > static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
> >
> > - struct drm_display_mode *mode)
> > + const struct drm_display_mode *mode)
> >
> > {
> >
> > /*
> >
> > * TODO dw drv improvements
> >
> > @@ -609,7 +609,7 @@ static void dw_mipi_dsi_command_mode_config(struct
> > dw_mipi_dsi *dsi)>
> > /* Get lane byte clock cycles. */
> > static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi,
> >
> > - struct drm_display_mode *mode,
> > + const struct drm_display_mode *mode,
> >
> > u32 hcomponent)
> >
> > {
> >
> > u32 frac, lbcc;
> >
> > @@ -625,7 +625,7 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct
> > dw_mipi_dsi *dsi,>
> > }
> >
> > static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
> >
> > - struct drm_display_mode *mode)
> > + const struct drm_display_mode *mode)
> >
> > {
> >
> > u32 htotal, hsa, hbp, lbcc;
> >
> > @@ -648,7 +648,7 @@ static void dw_mipi_dsi_line_timer_config(struct
> > dw_mipi_dsi *dsi,>
> > }
> >
> > static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> >
> > - struct drm_display_mode *mode)
> > + const struct drm_display_mode *mode)
> >
> > {
> >
> > u32 vactive, vsa, vfp, vbp;
> >
> > @@ -765,8 +765,8 @@ static void dw_mipi_dsi_bridge_post_disable(struct
> > drm_bridge *bridge)>
> > }
> >
> > static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> > const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
> >
> > diff --git a/drivers/gpu/drm/bridge/tc358767.c
> > b/drivers/gpu/drm/bridge/tc358767.c index 08ab7d6aea65..535fcd83f313
> > 100644
> > --- a/drivers/gpu/drm/bridge/tc358767.c
> > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > @@ -203,7 +203,7 @@ struct tc_data {
> >
> > /* display edid */
> > struct edid *edid;
> > /* current mode */
> >
> > - struct drm_display_mode *mode;
> > + const struct drm_display_mode *mode;
> >
> > u32 rev;
> > u8 assr;
> >
> > @@ -648,7 +648,8 @@ static int tc_get_display_props(struct tc_data *tc)
> >
> > return ret;
> >
> > }
> >
> > -static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode
> > *mode) +static int tc_set_video_mode(struct tc_data *tc,
> > + const struct drm_display_mode *mode)
> >
> > {
> >
> > int ret;
> > int vid_sync_dly;
> >
> > @@ -1113,8 +1114,8 @@ static int tc_connector_mode_valid(struct
> > drm_connector *connector,>
> > }
> >
> > static void tc_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adj)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adj)
> >
> > {
> >
> > struct tc_data *tc = bridge_to_tc(bridge);
> >
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 1638bfe9627c..c1dadb0a5d36 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -290,8 +290,8 @@ EXPORT_SYMBOL(drm_bridge_post_disable);
> >
> > * Note: the bridge passed should be the one closest to the encoder
> > */
> >
> > void drm_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > if (!bridge)
> >
> > return;
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > b/drivers/gpu/drm/exynos/exynos_drm_mic.c index
> > 2174814273e2..5ee3d9d9cd5d 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > @@ -246,8 +246,8 @@ static void mic_post_disable(struct drm_bridge
> > *bridge)
> >
> > }
> >
> > static void mic_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct exynos_mic *mic = bridge->driver_private;
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 59a11026dceb..02127ddf340b
> > 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > @@ -1364,8 +1364,8 @@ static void mtk_hdmi_bridge_post_disable(struct
> > drm_bridge *bridge)>
> > }
> >
> > static void mtk_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> > index 70d9a9a47acd..9730e5a6e1a7 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi.h
> > +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> > @@ -165,7 +165,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> >
> > struct msm_dsi_phy_shared_timings *phy_shared_timings);
> >
> > int msm_dsi_host_power_off(struct mipi_dsi_host *host);
> > int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> >
> > - struct drm_display_mode *mode);
> > + const struct drm_display_mode *mode);
> >
> > struct drm_panel *msm_dsi_host_get_panel(struct mipi_dsi_host *host,
> >
> > unsigned long *panel_flags);
> >
> > struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host);
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > b/drivers/gpu/drm/msm/dsi/dsi_host.c index 7a03a9489708..9c2dd91348ac
> > 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -2329,7 +2329,7 @@ int msm_dsi_host_power_off(struct mipi_dsi_host
> > *host)>
> > }
> >
> > int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> >
> > - struct drm_display_mode *mode)
> > + const struct drm_display_mode *mode)
> >
> > {
> >
> > struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 4cb1cb68878b..b742ef7d09ab
> > 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > @@ -603,8 +603,8 @@ static void dsi_mgr_bridge_post_disable(struct
> > drm_bridge *bridge)>
> > }
> >
> > static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > int id = dsi_mgr_bridge_get_id(bridge);
> > struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
> >
> > diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c
> > b/drivers/gpu/drm/msm/edp/edp_bridge.c index 931a5c97cccf..86366ba03e60
> > 100644
> > --- a/drivers/gpu/drm/msm/edp/edp_bridge.c
> > +++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
> > @@ -52,8 +52,8 @@ static void edp_bridge_post_disable(struct drm_bridge
> > *bridge)>
> > }
> >
> > static void edp_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct drm_device *dev = bridge->dev;
> > struct drm_connector *connector;
> >
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index 7e357077ed26..50e00ec153bf
> > 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > @@ -207,8 +207,8 @@ static void msm_hdmi_bridge_post_disable(struct
> > drm_bridge *bridge)>
> > }
> >
> > static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
> > struct hdmi *hdmi = hdmi_bridge->hdmi;
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 3d2d3bbd1342..802c5e8867e6
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -318,8 +318,8 @@ static void rcar_lvds_get_lvds_mode(struct rcar_lvds
> > *lvds)>
> > }
> >
> > static void rcar_lvds_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> >
> > diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> > index a5979cd25cc7..b6a70c605125 100644
> > --- a/drivers/gpu/drm/sti/sti_dvo.c
> > +++ b/drivers/gpu/drm/sti/sti_dvo.c
> > @@ -277,8 +277,8 @@ static void sti_dvo_pre_enable(struct drm_bridge
> > *bridge)>
> > }
> >
> > static void sti_dvo_set_mode(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct sti_dvo *dvo = bridge->driver_private;
> > struct sti_mixer *mixer = to_sti_mixer(dvo->encoder->crtc);
> >
> > diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> > index 67bbdb49fffc..9ec0195df46c 100644
> > --- a/drivers/gpu/drm/sti/sti_hda.c
> > +++ b/drivers/gpu/drm/sti/sti_hda.c
> > @@ -508,8 +508,8 @@ static void sti_hda_pre_enable(struct drm_bridge
> > *bridge)>
> > }
> >
> > static void sti_hda_set_mode(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct sti_hda *hda = bridge->driver_private;
> > u32 mode_idx;
> >
> > diff --git a/drivers/gpu/drm/sti/sti_hdmi.c
> > b/drivers/gpu/drm/sti/sti_hdmi.c index 58f431102512..08953c539980 100644
> > --- a/drivers/gpu/drm/sti/sti_hdmi.c
> > +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> > @@ -917,8 +917,8 @@ static void sti_hdmi_pre_enable(struct drm_bridge
> > *bridge)>
> > }
> >
> > static void sti_hdmi_set_mode(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> >
> > {
> >
> > struct sti_hdmi *hdmi = bridge->driver_private;
> > int ret;
> >
> > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c index a514b593f37c..a672b59a2226
> > 100644
> > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > @@ -215,7 +215,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
> >
> > }
> >
> > static int
> >
> > -dw_mipi_dsi_get_lane_mbps(void *priv_data, struct drm_display_mode *mode,
> > +dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode
> > *mode,>
> > unsigned long mode_flags, u32 lanes, u32 format,
> > unsigned int *lane_mbps)
> >
> > {
> >
> > diff --git a/include/drm/bridge/dw_mipi_dsi.h
> > b/include/drm/bridge/dw_mipi_dsi.h index d9c6d549f971..3e7bf033c1f3
> > 100644
> > --- a/include/drm/bridge/dw_mipi_dsi.h
> > +++ b/include/drm/bridge/dw_mipi_dsi.h
> > @@ -14,7 +14,8 @@ struct dw_mipi_dsi;
> >
> > struct dw_mipi_dsi_phy_ops {
> >
> > int (*init)(void *priv_data);
> >
> > - int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode,
> > + int (*get_lane_mbps)(void *priv_data,
> > + const struct drm_display_mode *mode,
> >
> > unsigned long mode_flags, u32 lanes, u32 format,
> > unsigned int *lane_mbps);
> >
> > };
> >
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index 3270fec46979..661c54cd062e 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -180,8 +180,8 @@ struct drm_bridge_funcs {
> >
> > * called.
> > */
> >
> > void (*mode_set)(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode);
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode);
> >
> > /**
> >
> > * @pre_enable:
> > *
> >
> > @@ -292,8 +292,8 @@ enum drm_mode_status drm_bridge_mode_valid(struct
> > drm_bridge *bridge,>
> > void drm_bridge_disable(struct drm_bridge *bridge);
> > void drm_bridge_post_disable(struct drm_bridge *bridge);
> > void drm_bridge_mode_set(struct drm_bridge *bridge,
> >
> > - struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode);
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode);
> >
> > void drm_bridge_pre_enable(struct drm_bridge *bridge);
> > void drm_bridge_enable(struct drm_bridge *bridge);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation
2018-04-09 8:56 ` Laurent Pinchart
@ 2018-04-09 9:18 ` Daniel Vetter
2018-04-09 9:43 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2018-04-09 9:18 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Laurent Pinchart, Seung-Woo Kim, Philippe Cornu, dri-devel,
Yannick Fertre, Kyungmin Park, Vincent Abriou
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>
>
> >
> > > ---
> > >
> > > 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
> > >
> > > :-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > > b/drivers/gpu/drm/bridge/adv7511/adv7511.h index
> > > d034b2cb5eee..f60d29defd18 100644
> > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> > > @@ -389,7 +389,7 @@ static inline int adv7511_cec_init(struct device *dev,
> > > struct adv7511 *adv7511)>
> > > #ifdef CONFIG_DRM_I2C_ADV7533
> > > void adv7533_dsi_power_on(struct adv7511 *adv);
> > > void adv7533_dsi_power_off(struct adv7511 *adv);
> > >
> > > -void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode
> > > *mode);
> > > +void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode
> > > *mode);>
> > > int adv7533_patch_registers(struct adv7511 *adv);
> > > int adv7533_patch_cec_registers(struct adv7511 *adv);
> > > int adv7533_attach_dsi(struct adv7511 *adv);
> > >
> > > @@ -405,7 +405,7 @@ static inline void adv7533_dsi_power_off(struct
> > > adv7511 *adv)>
> > > }
> > >
> > > static inline void adv7533_mode_set(struct adv7511 *adv,
> > >
> > > - struct drm_display_mode *mode)
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index
> > > efa29db5fc2b..99028d36ed74 100644
> > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > > @@ -664,8 +664,8 @@ static int adv7511_mode_valid(struct adv7511 *adv7511,
> > >
> > > }
> > >
> > > static void adv7511_mode_set(struct adv7511 *adv7511,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adj_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adj_mode)
> > >
> > > {
> > >
> > > unsigned int low_refresh_rate;
> > > unsigned int hsync_polarity = 0;
> > >
> > > @@ -827,8 +827,8 @@ static void adv7511_bridge_disable(struct drm_bridge
> > > *bridge)>
> > > }
> > >
> > > static void adv7511_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adj_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adj_mode)
> > >
> > > {
> > >
> > > struct adv7511 *adv = bridge_to_adv7511(bridge);
> > >
> > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > > b/drivers/gpu/drm/bridge/adv7511/adv7533.c index
> > > 185b6d842166..5d5e7d9eded2 100644
> > > --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > > @@ -108,7 +108,7 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
> > >
> > > regmap_write(adv->regmap_cec, 0x27, 0x0b);
> > >
> > > }
> > >
> > > -void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode)
> > > +void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode
> > > *mode)>
> > > {
> > >
> > > struct mipi_dsi_device *dsi = adv->dsi;
> > > int lanes, ret;
> > >
> > > diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > > b/drivers/gpu/drm/bridge/analogix-anx78xx.c index
> > > b49043866be6..2a7d80da7fb7 100644
> > > --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > > +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > > @@ -1082,8 +1082,8 @@ static void anx78xx_bridge_disable(struct drm_bridge
> > > *bridge)>
> > > }
> > >
> > > static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct anx78xx *anx78xx = bridge_to_anx78xx(bridge);
> > > struct hdmi_avi_infoframe frame;
> > >
> > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index
> > > 5c52307146c7..609da026e713 100644
> > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > @@ -1202,8 +1202,8 @@ static void analogix_dp_bridge_disable(struct
> > > drm_bridge *bridge)>
> > > }
> > >
> > > static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *orig_mode,
> > > - struct drm_display_mode *mode)
> > > + const struct drm_display_mode *orig_mode,
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > >
> > > struct analogix_dp_device *dp = bridge->driver_private;
> > > struct drm_display_info *display_info = &dp->connector.display_info;
> > >
> > > diff --git a/drivers/gpu/drm/bridge/sii902x.c
> > > b/drivers/gpu/drm/bridge/sii902x.c index 60373d7eb220..9320ff1116f3
> > > 100644
> > > --- a/drivers/gpu/drm/bridge/sii902x.c
> > > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > > @@ -254,8 +254,8 @@ static void sii902x_bridge_enable(struct drm_bridge
> > > *bridge)>
> > > }
> > >
> > > static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adj)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adj)
> > >
> > > {
> > >
> > > struct sii902x *sii902x = bridge_to_sii902x(bridge);
> > > struct regmap *regmap = sii902x->regmap;
> > >
> > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > > ec8d0006ef7c..40dcf5172149 100644
> > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > @@ -1998,8 +1998,8 @@ dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
> > >
> > > }
> > >
> > > static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *orig_mode,
> > > - struct drm_display_mode *mode)
> > > + const struct drm_display_mode *orig_mode,
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > >
> > > struct dw_hdmi *hdmi = bridge->driver_private;
> > >
> > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index
> > > 226171a3ece1..13df70bcfe94 100644
> > > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > > @@ -241,7 +241,7 @@ struct dw_mipi_dsi {
> > >
> > > * The controller should generate 2 frames before
> > > * preparing the peripheral.
> > > */
> > >
> > > -static void dw_mipi_dsi_wait_for_two_frames(struct drm_display_mode
> > > *mode)
> > > +static void dw_mipi_dsi_wait_for_two_frames(const struct drm_display_mode
> > > *mode)>
> > > {
> > >
> > > int refresh, two_frames;
> > >
> > > @@ -535,7 +535,7 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> > >
> > > }
> > >
> > > static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
> > >
> > > - struct drm_display_mode *mode)
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > >
> > > u32 val = 0, color = 0;
> > >
> > > @@ -578,7 +578,7 @@ static void dw_mipi_dsi_packet_handler_config(struct
> > > dw_mipi_dsi *dsi)>
> > > }
> > >
> > > static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
> > >
> > > - struct drm_display_mode *mode)
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > >
> > > /*
> > >
> > > * TODO dw drv improvements
> > >
> > > @@ -609,7 +609,7 @@ static void dw_mipi_dsi_command_mode_config(struct
> > > dw_mipi_dsi *dsi)>
> > > /* Get lane byte clock cycles. */
> > > static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi,
> > >
> > > - struct drm_display_mode *mode,
> > > + const struct drm_display_mode *mode,
> > >
> > > u32 hcomponent)
> > >
> > > {
> > >
> > > u32 frac, lbcc;
> > >
> > > @@ -625,7 +625,7 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct
> > > dw_mipi_dsi *dsi,>
> > > }
> > >
> > > static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
> > >
> > > - struct drm_display_mode *mode)
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > >
> > > u32 htotal, hsa, hbp, lbcc;
> > >
> > > @@ -648,7 +648,7 @@ static void dw_mipi_dsi_line_timer_config(struct
> > > dw_mipi_dsi *dsi,>
> > > }
> > >
> > > static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> > >
> > > - struct drm_display_mode *mode)
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > >
> > > u32 vactive, vsa, vfp, vbp;
> > >
> > > @@ -765,8 +765,8 @@ static void dw_mipi_dsi_bridge_post_disable(struct
> > > drm_bridge *bridge)>
> > > }
> > >
> > > static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> > > const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
> > >
> > > diff --git a/drivers/gpu/drm/bridge/tc358767.c
> > > b/drivers/gpu/drm/bridge/tc358767.c index 08ab7d6aea65..535fcd83f313
> > > 100644
> > > --- a/drivers/gpu/drm/bridge/tc358767.c
> > > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > > @@ -203,7 +203,7 @@ struct tc_data {
> > >
> > > /* display edid */
> > > struct edid *edid;
> > > /* current mode */
> > >
> > > - struct drm_display_mode *mode;
> > > + const struct drm_display_mode *mode;
> > >
> > > u32 rev;
> > > u8 assr;
> > >
> > > @@ -648,7 +648,8 @@ static int tc_get_display_props(struct tc_data *tc)
> > >
> > > return ret;
> > >
> > > }
> > >
> > > -static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode
> > > *mode) +static int tc_set_video_mode(struct tc_data *tc,
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > >
> > > int ret;
> > > int vid_sync_dly;
> > >
> > > @@ -1113,8 +1114,8 @@ static int tc_connector_mode_valid(struct
> > > drm_connector *connector,>
> > > }
> > >
> > > static void tc_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adj)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adj)
> > >
> > > {
> > >
> > > struct tc_data *tc = bridge_to_tc(bridge);
> > >
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index 1638bfe9627c..c1dadb0a5d36 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -290,8 +290,8 @@ EXPORT_SYMBOL(drm_bridge_post_disable);
> > >
> > > * Note: the bridge passed should be the one closest to the encoder
> > > */
> > >
> > > void drm_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > if (!bridge)
> > >
> > > return;
> > >
> > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > > b/drivers/gpu/drm/exynos/exynos_drm_mic.c index
> > > 2174814273e2..5ee3d9d9cd5d 100644
> > > --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > > +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > > @@ -246,8 +246,8 @@ static void mic_post_disable(struct drm_bridge
> > > *bridge)
> > >
> > > }
> > >
> > > static void mic_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct exynos_mic *mic = bridge->driver_private;
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 59a11026dceb..02127ddf340b
> > > 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > @@ -1364,8 +1364,8 @@ static void mtk_hdmi_bridge_post_disable(struct
> > > drm_bridge *bridge)>
> > > }
> > >
> > > static void mtk_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> > > index 70d9a9a47acd..9730e5a6e1a7 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi.h
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> > > @@ -165,7 +165,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > >
> > > struct msm_dsi_phy_shared_timings *phy_shared_timings);
> > >
> > > int msm_dsi_host_power_off(struct mipi_dsi_host *host);
> > > int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> > >
> > > - struct drm_display_mode *mode);
> > > + const struct drm_display_mode *mode);
> > >
> > > struct drm_panel *msm_dsi_host_get_panel(struct mipi_dsi_host *host,
> > >
> > > unsigned long *panel_flags);
> > >
> > > struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host);
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > b/drivers/gpu/drm/msm/dsi/dsi_host.c index 7a03a9489708..9c2dd91348ac
> > > 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > @@ -2329,7 +2329,7 @@ int msm_dsi_host_power_off(struct mipi_dsi_host
> > > *host)>
> > > }
> > >
> > > int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> > >
> > > - struct drm_display_mode *mode)
> > > + const struct drm_display_mode *mode)
> > >
> > > {
> > >
> > > struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 4cb1cb68878b..b742ef7d09ab
> > > 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > @@ -603,8 +603,8 @@ static void dsi_mgr_bridge_post_disable(struct
> > > drm_bridge *bridge)>
> > > }
> > >
> > > static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > int id = dsi_mgr_bridge_get_id(bridge);
> > > struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
> > >
> > > diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c
> > > b/drivers/gpu/drm/msm/edp/edp_bridge.c index 931a5c97cccf..86366ba03e60
> > > 100644
> > > --- a/drivers/gpu/drm/msm/edp/edp_bridge.c
> > > +++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
> > > @@ -52,8 +52,8 @@ static void edp_bridge_post_disable(struct drm_bridge
> > > *bridge)>
> > > }
> > >
> > > static void edp_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct drm_device *dev = bridge->dev;
> > > struct drm_connector *connector;
> > >
> > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index 7e357077ed26..50e00ec153bf
> > > 100644
> > > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > @@ -207,8 +207,8 @@ static void msm_hdmi_bridge_post_disable(struct
> > > drm_bridge *bridge)>
> > > }
> > >
> > > static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
> > > struct hdmi *hdmi = hdmi_bridge->hdmi;
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 3d2d3bbd1342..802c5e8867e6
> > > 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > @@ -318,8 +318,8 @@ static void rcar_lvds_get_lvds_mode(struct rcar_lvds
> > > *lvds)>
> > > }
> > >
> > > static void rcar_lvds_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > >
> > > diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> > > index a5979cd25cc7..b6a70c605125 100644
> > > --- a/drivers/gpu/drm/sti/sti_dvo.c
> > > +++ b/drivers/gpu/drm/sti/sti_dvo.c
> > > @@ -277,8 +277,8 @@ static void sti_dvo_pre_enable(struct drm_bridge
> > > *bridge)>
> > > }
> > >
> > > static void sti_dvo_set_mode(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct sti_dvo *dvo = bridge->driver_private;
> > > struct sti_mixer *mixer = to_sti_mixer(dvo->encoder->crtc);
> > >
> > > diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> > > index 67bbdb49fffc..9ec0195df46c 100644
> > > --- a/drivers/gpu/drm/sti/sti_hda.c
> > > +++ b/drivers/gpu/drm/sti/sti_hda.c
> > > @@ -508,8 +508,8 @@ static void sti_hda_pre_enable(struct drm_bridge
> > > *bridge)>
> > > }
> > >
> > > static void sti_hda_set_mode(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct sti_hda *hda = bridge->driver_private;
> > > u32 mode_idx;
> > >
> > > diff --git a/drivers/gpu/drm/sti/sti_hdmi.c
> > > b/drivers/gpu/drm/sti/sti_hdmi.c index 58f431102512..08953c539980 100644
> > > --- a/drivers/gpu/drm/sti/sti_hdmi.c
> > > +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> > > @@ -917,8 +917,8 @@ static void sti_hdmi_pre_enable(struct drm_bridge
> > > *bridge)>
> > > }
> > >
> > > static void sti_hdmi_set_mode(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode)
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode)
> > >
> > > {
> > >
> > > struct sti_hdmi *hdmi = bridge->driver_private;
> > > int ret;
> > >
> > > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > > b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c index a514b593f37c..a672b59a2226
> > > 100644
> > > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > > @@ -215,7 +215,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
> > >
> > > }
> > >
> > > static int
> > >
> > > -dw_mipi_dsi_get_lane_mbps(void *priv_data, struct drm_display_mode *mode,
> > > +dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode
> > > *mode,>
> > > unsigned long mode_flags, u32 lanes, u32 format,
> > > unsigned int *lane_mbps)
> > >
> > > {
> > >
> > > diff --git a/include/drm/bridge/dw_mipi_dsi.h
> > > b/include/drm/bridge/dw_mipi_dsi.h index d9c6d549f971..3e7bf033c1f3
> > > 100644
> > > --- a/include/drm/bridge/dw_mipi_dsi.h
> > > +++ b/include/drm/bridge/dw_mipi_dsi.h
> > > @@ -14,7 +14,8 @@ struct dw_mipi_dsi;
> > >
> > > struct dw_mipi_dsi_phy_ops {
> > >
> > > int (*init)(void *priv_data);
> > >
> > > - int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode,
> > > + int (*get_lane_mbps)(void *priv_data,
> > > + const struct drm_display_mode *mode,
> > >
> > > unsigned long mode_flags, u32 lanes, u32 format,
> > > unsigned int *lane_mbps);
> > >
> > > };
> > >
> > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > > index 3270fec46979..661c54cd062e 100644
> > > --- a/include/drm/drm_bridge.h
> > > +++ b/include/drm/drm_bridge.h
> > > @@ -180,8 +180,8 @@ struct drm_bridge_funcs {
> > >
> > > * called.
> > > */
> > >
> > > void (*mode_set)(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode);
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode);
> > >
> > > /**
> > >
> > > * @pre_enable:
> > > *
> > >
> > > @@ -292,8 +292,8 @@ enum drm_mode_status drm_bridge_mode_valid(struct
> > > drm_bridge *bridge,>
> > > void drm_bridge_disable(struct drm_bridge *bridge);
> > > void drm_bridge_post_disable(struct drm_bridge *bridge);
> > > void drm_bridge_mode_set(struct drm_bridge *bridge,
> > >
> > > - struct drm_display_mode *mode,
> > > - struct drm_display_mode *adjusted_mode);
> > > + const struct drm_display_mode *mode,
> > > + const struct drm_display_mode *adjusted_mode);
> > >
> > > void drm_bridge_pre_enable(struct drm_bridge *bridge);
> > > void drm_bridge_enable(struct drm_bridge *bridge);
>
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation
2018-04-09 9:18 ` Daniel Vetter
@ 2018-04-09 9:43 ` Laurent Pinchart
2018-04-13 15:55 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2018-04-09 9:43 UTC (permalink / raw)
To: Daniel Vetter
Cc: Laurent Pinchart, Seung-Woo Kim, Philippe Cornu, dri-devel,
Yannick Fertre, Kyungmin Park, Vincent Abriou
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation
2018-04-09 9:43 ` Laurent Pinchart
@ 2018-04-13 15:55 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2018-04-13 15:55 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Laurent Pinchart, Seung-Woo Kim, Philippe Cornu, dri-devel,
Yannick Fertre, Kyungmin Park, Vincent Abriou
On Mon, Apr 09, 2018 at 12:43:07PM +0300, Laurent Pinchart wrote:
> 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.
btw in case it's not clear: Since you have drm-misc commit rights I won't
push this for you :-)
-Daniel
>
> > >>> ---
> > >>>
> > >>> 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
>
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-13 15:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-06 16:23 [PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation Laurent Pinchart
2018-04-09 8:53 ` Daniel Vetter
2018-04-09 8:56 ` Laurent Pinchart
2018-04-09 9:18 ` Daniel Vetter
2018-04-09 9:43 ` Laurent Pinchart
2018-04-13 15:55 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.