From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S940017AbdEXMak (ORCPT ); Wed, 24 May 2017 08:30:40 -0400 Received: from mail-wr0-f170.google.com ([209.85.128.170]:33937 "EHLO mail-wr0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752255AbdEXMaF (ORCPT ); Wed, 24 May 2017 08:30:05 -0400 Subject: Re: [PATCH v4 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: Cc: Daniel Vetter , Alexey Brodkin , Carlos Palminha , Laurent Pinchart From: Neil Armstrong Organization: Baylibre Message-ID: <33417844-b72b-ff73-8cf7-a30bc0352613@baylibre.com> Date: Wed, 24 May 2017 14:30:01 +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: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jose, On 05/19/2017 02:52 AM, 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 > Cc: Carlos Palminha > Cc: Alexey Brodkin > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: Dave Airlie > Cc: Andrzej Hajda > Cc: Archit Taneja > Cc: Laurent Pinchart > --- > 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 4e1f54a..864e946 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -1881,24 +1881,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, > @@ -1924,7 +1906,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, > }; > > @@ -1947,18 +1928,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, > @@ -2002,7 +1986,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; For the meson/meson_dw_hdmi.c file : Acked-by: Neil Armstrong I can have a try in meson if you have a tree I can pull the full patchset from. Neil