From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750955AbdE2Jpy (ORCPT ); Mon, 29 May 2017 05:45:54 -0400 Received: from mail-wm0-f41.google.com ([74.125.82.41]:37537 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750837AbdE2Jpw (ORCPT ); Mon, 29 May 2017 05:45:52 -0400 Subject: Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use bridge->mode_valid() callback To: Jose Abreu , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <3d8d449e4d13d2535fa292c75f5fa931de4a4fa8.1495720737.git.joabreu@synopsys.com> Cc: Daniel Vetter , Carlos Palminha , Laurent Pinchart , Kevin Hilman , Carlo Caione From: Neil Armstrong Organization: Baylibre Message-ID: <949af4a3-c9d3-7a16-77df-d72c03b77985@baylibre.com> Date: Mon, 29 May 2017 11:45:44 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <3d8d449e4d13d2535fa292c75f5fa931de4a4fa8.1495720737.git.joabreu@synopsys.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/25/2017 04:19 PM, Jose Abreu wrote: > Now that we have a callback to check if bridge supports a given mode > we can use it in Synopsys Designware HDMI bridge so that we restrict > the number of probbed modes to the ones we can actually display. > > Also, there is no need to use mode_fixup() callback as mode_valid() > will handle the mode validation. > > NOTE: Only compile tested > NOTE 2: I also had to change the pdata declaration of mode_valid > custom callback so that the passed modes are const. I also changed > in the platforms I found. Not even compiled it though. > > Signed-off-by: Jose Abreu > Acked-by: Neil Armstrong > Cc: Carlos Palminha > Cc: Daniel Vetter > Cc: Archit Taneja > Cc: Andrzej Hajda > Cc: Laurent Pinchart > Cc: David Airlie > Cc: Philipp Zabel > Cc: Carlo Caione > Cc: Kevin Hilman > Cc: Mark Yao > Cc: Heiko Stuebner > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 40 +++++++++-------------------- > drivers/gpu/drm/imx/dw_hdmi-imx.c | 4 +-- > drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 +- > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +- > include/drm/bridge/dw_hdmi.h | 2 +- > 5 files changed, 17 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 8737de8..038dc43 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -1907,24 +1907,6 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) > return ret; > } > > -static enum drm_mode_status > -dw_hdmi_connector_mode_valid(struct drm_connector *connector, > - struct drm_display_mode *mode) > -{ > - struct dw_hdmi *hdmi = container_of(connector, > - struct dw_hdmi, connector); > - enum drm_mode_status mode_status = MODE_OK; > - > - /* We don't support double-clocked modes */ > - if (mode->flags & DRM_MODE_FLAG_DBLCLK) > - return MODE_BAD; > - > - if (hdmi->plat_data->mode_valid) > - mode_status = hdmi->plat_data->mode_valid(connector, mode); > - > - return mode_status; > -} > - > static void dw_hdmi_connector_force(struct drm_connector *connector) > { > struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, > @@ -1950,7 +1932,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector) > > static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs = { > .get_modes = dw_hdmi_connector_get_modes, > - .mode_valid = dw_hdmi_connector_mode_valid, > .best_encoder = drm_atomic_helper_best_encoder, > }; > > @@ -1973,18 +1954,21 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge) > return 0; > } > > -static bool dw_hdmi_bridge_mode_fixup(struct drm_bridge *bridge, > - const struct drm_display_mode *orig_mode, > - struct drm_display_mode *mode) > +static enum drm_mode_status dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge, > + const struct drm_display_mode *mode) > { > struct dw_hdmi *hdmi = bridge->driver_private; > struct drm_connector *connector = &hdmi->connector; > - enum drm_mode_status status; > + enum drm_mode_status mode_status = MODE_OK; > > - status = dw_hdmi_connector_mode_valid(connector, mode); > - if (status != MODE_OK) > - return false; > - return true; > + /* We don't support double-clocked modes */ > + if (mode->flags & DRM_MODE_FLAG_DBLCLK) > + return MODE_BAD; > + > + if (hdmi->plat_data->mode_valid) > + mode_status = hdmi->plat_data->mode_valid(connector, mode); > + > + return mode_status; > } > > static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge, > @@ -2028,7 +2012,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge) > .enable = dw_hdmi_bridge_enable, > .disable = dw_hdmi_bridge_disable, > .mode_set = dw_hdmi_bridge_mode_set, > - .mode_fixup = dw_hdmi_bridge_mode_fixup, > + .mode_valid = dw_hdmi_bridge_mode_valid, > }; > > static irqreturn_t dw_hdmi_i2c_irq(struct dw_hdmi *hdmi) > diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c > index f039641..5f561c8 100644 > --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c > +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c > @@ -148,7 +148,7 @@ static int dw_hdmi_imx_atomic_check(struct drm_encoder *encoder, > }; > > static enum drm_mode_status imx6q_hdmi_mode_valid(struct drm_connector *con, > - struct drm_display_mode *mode) > + const struct drm_display_mode *mode) > { > if (mode->clock < 13500) > return MODE_CLOCK_LOW; > @@ -160,7 +160,7 @@ static enum drm_mode_status imx6q_hdmi_mode_valid(struct drm_connector *con, > } > > static enum drm_mode_status imx6dl_hdmi_mode_valid(struct drm_connector *con, > - struct drm_display_mode *mode) > + const struct drm_display_mode *mode) > { > if (mode->clock < 13500) > return MODE_CLOCK_LOW; > diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c > index 7b86eb7..f043904 100644 > --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c > +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c > @@ -537,7 +537,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id) > > /* TOFIX Enable support for non-vic modes */ > static enum drm_mode_status dw_hdmi_mode_valid(struct drm_connector *connector, > - struct drm_display_mode *mode) > + const struct drm_display_mode *mode) > { > unsigned int vclk_freq; > unsigned int venc_freq; > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > index 63dab6f..f820848 100644 > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > @@ -155,7 +155,7 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) > > static enum drm_mode_status > dw_hdmi_rockchip_mode_valid(struct drm_connector *connector, > - struct drm_display_mode *mode) > + const struct drm_display_mode *mode) > { > const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg; > int pclk = mode->clock * 1000; > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h > index ed599be..4c8d4c8 100644 > --- a/include/drm/bridge/dw_hdmi.h > +++ b/include/drm/bridge/dw_hdmi.h > @@ -125,7 +125,7 @@ struct dw_hdmi_phy_ops { > struct dw_hdmi_plat_data { > struct regmap *regm; > enum drm_mode_status (*mode_valid)(struct drm_connector *connector, > - struct drm_display_mode *mode); > + const struct drm_display_mode *mode); > unsigned long input_bus_format; > unsigned long input_bus_encoding; > > Hi Jose, Tested-by: Neil Armstrong On Meson GXL, no issues. Thanks, Neil