[PATCHv3 00/30] drm/omap: miscallaneous improvements

[PATCHv3 00/30] drm/omap: miscallaneous improvements

[Tomi Valkeinen]

This is the third revision of this series. Note that this series depends on
"drm/atomic: Introduce drm_atomic_helper_shutdown" which has not yet been
merged to drm-next.

The main changes in v3:

- improve variable names in 'work-around for errata i886'
- drop 'Init fbdev emulation only when we have displays'
- drop 'Create fbdev emulation only for the first DRM connector'
- add 'use drm_atomic_helper_shutdown'
- change 'fix crash on module unload' based on drm_atomic_helper_shutdown change.

The main changes in v2:

- Interrupt based HPD removed, as there's a race issue which needs to be fixed
- Added patch to fix module unload crash, introduced in drm-next
- Drop changes affecting userspace APIs


Hemant Hariyani (1):
  drm/omap: Add support for render nodes

Peter Ujfalusi (6):
  drm/omap: dss: Functions to check components in the display/output
    list
  drm/omap: dss: Support for detecting display stack readiness
  drm/omap: Use omapdss_stack_is_ready() to check that the display stack
    is up
  drm/omap: display: Add displays in sorted order to the panel_list
  drm/omap: poll only connectors where the connect/disconnect can be
    checked
  drm/omap: displays: panel-dpi: Support for handling backlight devices

Tomi Valkeinen (23):
  drm/omap: work-around for errata i886
  drm/omap: refactor CRTC HW property setup
  drm/omap: remove divider constraint from hsdiv
  drm/omap: decrease min width & height
  drm/omap: improve DPI clock selection on DRA7xx
  drm/omap: fix HDMI sync polarities
  drm/omap: add omapdss-base.ko
  drm/omap: move dss_initialized to omapdss-base
  drm/omap: output: use dev_err instead of DSSERR
  drm/omap: display: don't use dsi_get_pixel_size()
  drm/omap: move display, dss-of, output to omapdss-base
  drm/omap: move dispc related dss-feat funcs to dispc
  drm/omap: add dispc_ops
  drm/omap: fill dispc_ops
  drm/omap: use dispc_ops
  drm/omap: remove all EXPORT_SYMBOLs from dispc.c
  drm/omap: remove unused dispc_wb_enable & dispc_wb_is_enabled
  drm/omap: fix replication logic
  drm/omap: fix plane update warning when crtc is disabled
  drm/omap: dispc: improve debug print of display flags
  drm/omap: fix display SYNC/DE flags
  drm/omap: use drm_atomic_helper_shutdown()
  drm/omap: fix crash on module unload

 drivers/gpu/drm/omapdrm/displays/panel-dpi.c |  37 +++++-
 drivers/gpu/drm/omapdrm/dss/Kconfig          |   4 +
 drivers/gpu/drm/omapdrm/dss/Makefile         |   8 +-
 drivers/gpu/drm/omapdrm/dss/base.c           | 140 +++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/dispc.c          | 165 ++++++++++++++++-----------
 drivers/gpu/drm/omapdrm/dss/display.c        |  36 +++++-
 drivers/gpu/drm/omapdrm/dss/dpi.c            |  55 ++++++---
 drivers/gpu/drm/omapdrm/dss/dsi.c            |   2 +-
 drivers/gpu/drm/omapdrm/dss/dss-of.c         |   3 +-
 drivers/gpu/drm/omapdrm/dss/dss.c            |  13 +--
 drivers/gpu/drm/omapdrm/dss/dss.h            |  17 +--
 drivers/gpu/drm/omapdrm/dss/dss_features.c   |   3 -
 drivers/gpu/drm/omapdrm/dss/dss_features.h   |   4 +
 drivers/gpu/drm/omapdrm/dss/hdmi_wp.c        |  12 +-
 drivers/gpu/drm/omapdrm/dss/omapdss.h        |  95 +++++++++------
 drivers/gpu/drm/omapdrm/dss/output.c         |  27 ++++-
 drivers/gpu/drm/omapdrm/dss/pll.c            |  17 +--
 drivers/gpu/drm/omapdrm/omap_connector.c     |  12 +-
 drivers/gpu/drm/omapdrm/omap_crtc.c          |  87 ++++++++++----
 drivers/gpu/drm/omapdrm/omap_drv.c           |  45 +++++---
 drivers/gpu/drm/omapdrm/omap_drv.h           |   2 +
 drivers/gpu/drm/omapdrm/omap_irq.c           |  47 +++-----
 drivers/gpu/drm/omapdrm/omap_plane.c         |  20 +++-
 23 files changed, 588 insertions(+), 263 deletions(-)
 create mode 100644 drivers/gpu/drm/omapdrm/dss/base.c

-- 
2.7.4

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

[PATCHv3 01/30] drm/omap: work-around for errata i886

[Tomi Valkeinen]

DRA7 errata i886 (FPDLink PLL Unlocks With Certain SoC PLL M/N Values)
says that FPDLink is sensitive to jitter on the vout clock, and that low
PLL M and N values result in more jitter than high M and N values.

This patch implements a workaround for the problem by changing the PLL
setup to search for clocks starting from high M and N values, instead of
low values. This should not cause any functional change, and only
reduces the jitter.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/pll.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/pll.c b/drivers/gpu/drm/omapdrm/dss/pll.c
index 0a76c89cdc2e..5e221302768b 100644
--- a/drivers/gpu/drm/omapdrm/dss/pll.c
+++ b/drivers/gpu/drm/omapdrm/dss/pll.c
@@ -215,8 +215,8 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin,
 		dss_pll_calc_func func, void *data)
 {
 	const struct dss_pll_hw *hw = pll->hw;
-	int n, n_start, n_stop;
-	int m, m_start, m_stop;
+	int n, n_min, n_max;
+	int m, m_min, m_max;
 	unsigned long fint, clkdco;
 	unsigned long pll_hw_max;
 	unsigned long fint_hw_min, fint_hw_max;
@@ -226,21 +226,22 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin,
 	fint_hw_min = hw->fint_min;
 	fint_hw_max = hw->fint_max;
 
-	n_start = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
-	n_stop = min((unsigned)(clkin / fint_hw_min), hw->n_max);
+	n_min = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
+	n_max = min((unsigned)(clkin / fint_hw_min), hw->n_max);
 
 	pll_max = pll_max ? pll_max : ULONG_MAX;
 
-	for (n = n_start; n <= n_stop; ++n) {
+	/* Try to find high N & M to avoid jitter (DRA7 errata i886) */
+	for (n = n_max; n >= n_min; --n) {
 		fint = clkin / n;
 
-		m_start = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
+		m_min = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
 				1ul);
-		m_stop = min3((unsigned)(pll_max / fint / 2),
+		m_max = min3((unsigned)(pll_max / fint / 2),
 				(unsigned)(pll_hw_max / fint / 2),
 				hw->m_max);
 
-		for (m = m_start; m <= m_stop; ++m) {
+		for (m = m_max; m >= m_min; --m) {
 			clkdco = 2 * m * fint;
 
 			if (func(n, m, fint, clkdco, data))
-- 
2.7.4

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

Re: [PATCHv3 01/30] drm/omap: work-around for errata i886

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:47 Tomi Valkeinen wrote:
> DRA7 errata i886 (FPDLink PLL Unlocks With Certain SoC PLL M/N Values)
> says that FPDLink is sensitive to jitter on the vout clock, and that low
> PLL M and N values result in more jitter than high M and N values.
> 
> This patch implements a workaround for the problem by changing the PLL
> setup to search for clocks starting from high M and N values, instead of
> low values. This should not cause any functional change, and only
> reduces the jitter.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/pll.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/pll.c
> b/drivers/gpu/drm/omapdrm/dss/pll.c index 0a76c89cdc2e..5e221302768b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/pll.c
> +++ b/drivers/gpu/drm/omapdrm/dss/pll.c
> @@ -215,8 +215,8 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned
> long clkin, dss_pll_calc_func func, void *data)
>  {
>  	const struct dss_pll_hw *hw = pll->hw;
> -	int n, n_start, n_stop;
> -	int m, m_start, m_stop;
> +	int n, n_min, n_max;
> +	int m, m_min, m_max;
>  	unsigned long fint, clkdco;
>  	unsigned long pll_hw_max;
>  	unsigned long fint_hw_min, fint_hw_max;
> @@ -226,21 +226,22 @@ bool dss_pll_calc_a(const struct dss_pll *pll,
> unsigned long clkin, fint_hw_min = hw->fint_min;
>  	fint_hw_max = hw->fint_max;
> 
> -	n_start = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
> -	n_stop = min((unsigned)(clkin / fint_hw_min), hw->n_max);
> +	n_min = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
> +	n_max = min((unsigned)(clkin / fint_hw_min), hw->n_max);
> 
>  	pll_max = pll_max ? pll_max : ULONG_MAX;
> 
> -	for (n = n_start; n <= n_stop; ++n) {
> +	/* Try to find high N & M to avoid jitter (DRA7 errata i886) */
> +	for (n = n_max; n >= n_min; --n) {
>  		fint = clkin / n;
> 
> -		m_start = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
> +		m_min = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
>  				1ul);
> -		m_stop = min3((unsigned)(pll_max / fint / 2),
> +		m_max = min3((unsigned)(pll_max / fint / 2),
>  				(unsigned)(pll_hw_max / fint / 2),
>  				hw->m_max);
> 
> -		for (m = m_start; m <= m_stop; ++m) {
> +		for (m = m_max; m >= m_min; --m) {
>  			clkdco = 2 * m * fint;
> 
>  			if (func(n, m, fint, clkdco, data))

-- 
Regards,

Laurent Pinchart

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

[PATCHv3 02/30] drm/omap: refactor CRTC HW property setup

[Tomi Valkeinen]

The current driver doesn't expose any of the CRTC HW properties like
background color or transparency key, and sets them at CRTC enable time.

Refactor this into a separate function and call that function from
omap_crtc_atomic_flush(). This is the behavior we want when the
properties can be configured, so this patch makes it easier to add
patches later which implement those properties.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 2fe735c269fc..49fc61963af4 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -198,15 +198,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 static int omap_crtc_dss_enable(enum omap_channel channel)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[channel];
-	struct omap_overlay_manager_info info;
 
-	memset(&info, 0, sizeof(info));
-	info.default_color = 0x00000000;
-	info.trans_key = 0x00000000;
-	info.trans_key_type = OMAP_DSS_COLOR_KEY_GFX_DST;
-	info.trans_enabled = false;
-
-	dispc_mgr_setup(omap_crtc->channel, &info);
 	dispc_mgr_set_timings(omap_crtc->channel,
 			&omap_crtc->vm);
 	omap_crtc_set_enabled(&omap_crtc->base, true);
@@ -313,6 +305,21 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
 	DBG("%s: apply done", omap_crtc->name);
 }
 
+static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_overlay_manager_info info;
+
+	memset(&info, 0, sizeof(info));
+
+	info.default_color = 0x000000;
+	info.trans_enabled = false;
+	info.partial_alpha_enabled = false;
+	info.cpr_enable = false;
+
+	dispc_mgr_setup(omap_crtc->channel, &info);
+}
+
 /* -----------------------------------------------------------------------------
  * CRTC Functions
  */
@@ -410,6 +417,8 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 		dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
 	}
 
+	omap_crtc_write_crtc_properties(crtc);
+
 	/* Only flush the CRTC if it is currently enabled. */
 	if (!omap_crtc->enabled)
 		return;
-- 
2.7.4

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

Re: [PATCHv3 02/30] drm/omap: refactor CRTC HW property setup

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:48 Tomi Valkeinen wrote:
> The current driver doesn't expose any of the CRTC HW properties like
> background color or transparency key, and sets them at CRTC enable time.
> 
> Refactor this into a separate function and call that function from
> omap_crtc_atomic_flush(). This is the behavior we want when the
> properties can be configured, so this patch makes it easier to add
> patches later which implement those properties.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2fe735c269fc..49fc61963af4
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -198,15 +198,7 @@ static void omap_crtc_set_enabled(struct drm_crtc
> *crtc, bool enable) static int omap_crtc_dss_enable(enum omap_channel
> channel)
>  {
>  	struct omap_crtc *omap_crtc = omap_crtcs[channel];
> -	struct omap_overlay_manager_info info;
> 
> -	memset(&info, 0, sizeof(info));
> -	info.default_color = 0x00000000;
> -	info.trans_key = 0x00000000;
> -	info.trans_key_type = OMAP_DSS_COLOR_KEY_GFX_DST;
> -	info.trans_enabled = false;
> -
> -	dispc_mgr_setup(omap_crtc->channel, &info);
>  	dispc_mgr_set_timings(omap_crtc->channel,
>  			&omap_crtc->vm);
>  	omap_crtc_set_enabled(&omap_crtc->base, true);
> @@ -313,6 +305,21 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
>  	DBG("%s: apply done", omap_crtc->name);
>  }
> 
> +static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
> +{
> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	struct omap_overlay_manager_info info;
> +
> +	memset(&info, 0, sizeof(info));
> +
> +	info.default_color = 0x000000;
> +	info.trans_enabled = false;
> +	info.partial_alpha_enabled = false;
> +	info.cpr_enable = false;

Nitpicking, you could initialize info when declaring it, I think it would be 
slightly more efficient.

> +
> +	dispc_mgr_setup(omap_crtc->channel, &info);

dispc_mgr_setup() was previously called from omap_crtc_dss_enable() with the 
DSS disabled. Now it can be called with the DSS enabled. Have you double-
checked that this isn't an issue ?

> +}
> +
>  /* ------------------------------------------------------------------------
>   * CRTC Functions
>   */
> @@ -410,6 +417,8 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
>  	}
> 
> +	omap_crtc_write_crtc_properties(crtc);
> +
>  	/* Only flush the CRTC if it is currently enabled. */
>  	if (!omap_crtc->enabled)
>  		return;

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 02/30] drm/omap: refactor CRTC HW property setup

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1395 bytes --]

On 29/03/17 11:05, Laurent Pinchart wrote:

>> +static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
>> +{
>> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>> +	struct omap_overlay_manager_info info;
>> +
>> +	memset(&info, 0, sizeof(info));
>> +
>> +	info.default_color = 0x000000;
>> +	info.trans_enabled = false;
>> +	info.partial_alpha_enabled = false;
>> +	info.cpr_enable = false;
> 
> Nitpicking, you could initialize info when declaring it, I think it would be 
> slightly more efficient.

These are open coded here so that it's easy to change these line by line
when we add the properties, and that code most likely can't be in the
initializer. But yes, at the moment it looks a bit silly.

>> +
>> +	dispc_mgr_setup(omap_crtc->channel, &info);
> 
> dispc_mgr_setup() was previously called from omap_crtc_dss_enable() with the 
> DSS disabled. Now it can be called with the DSS enabled. Have you double-
> checked that this isn't an issue ?

It's not an issue, it's how it's supposed to work. These mgr-settings
are "shadow" settings, i.e. they take effect when we set the GO bit
(like the plane settings).

They were only written in dss_enable() as we didn't have support to
change those properties. I hope will get those properties some time
soon, so we need to write the mgr-settings even when the output is enabled.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCHv3 03/30] drm/omap: remove divider constraint from hsdiv

[Tomi Valkeinen]

The driver only uses even dividers for hsdiv when pclk >= 100MHz, as odd
dividers can create uneven duty cycle. However, while this holds true
for some dividers like DISPC's LCK and PCK dividers, it is not actually
true for hsdiv.

hsdiv always produces even duty cycle, so the constraint can be removed.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dpi.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
index e75162d26ac0..e0b0c5c24c55 100644
--- a/drivers/gpu/drm/omapdrm/dss/dpi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
@@ -170,14 +170,6 @@ static bool dpi_calc_hsdiv_cb(int m_dispc, unsigned long dispc,
 {
 	struct dpi_clk_calc_ctx *ctx = data;
 
-	/*
-	 * Odd dividers give us uneven duty cycle, causing problem when level
-	 * shifted. So skip all odd dividers when the pixel clock is on the
-	 * higher side.
-	 */
-	if (m_dispc > 1 && m_dispc % 2 != 0 && ctx->pck_min >= 100000000)
-		return false;
-
 	ctx->pll_cinfo.mX[ctx->clkout_idx] = m_dispc;
 	ctx->pll_cinfo.clkout[ctx->clkout_idx] = dispc;
 
-- 
2.7.4

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

Re: [PATCHv3 03/30] drm/omap: remove divider constraint from hsdiv

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:49 Tomi Valkeinen wrote:
> The driver only uses even dividers for hsdiv when pclk >= 100MHz, as odd
> dividers can create uneven duty cycle. However, while this holds true
> for some dividers like DISPC's LCK and PCK dividers, it is not actually
> true for hsdiv.
> 
> hsdiv always produces even duty cycle, so the constraint can be removed.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/dpi.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c
> b/drivers/gpu/drm/omapdrm/dss/dpi.c index e75162d26ac0..e0b0c5c24c55 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> @@ -170,14 +170,6 @@ static bool dpi_calc_hsdiv_cb(int m_dispc, unsigned
> long dispc, {
>  	struct dpi_clk_calc_ctx *ctx = data;
> 
> -	/*
> -	 * Odd dividers give us uneven duty cycle, causing problem when level
> -	 * shifted. So skip all odd dividers when the pixel clock is on the
> -	 * higher side.
> -	 */
> -	if (m_dispc > 1 && m_dispc % 2 != 0 && ctx->pck_min >= 100000000)
> -		return false;
> -
>  	ctx->pll_cinfo.mX[ctx->clkout_idx] = m_dispc;
>  	ctx->pll_cinfo.clkout[ctx->clkout_idx] = dispc;

-- 
Regards,

Laurent Pinchart

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

[PATCHv3 04/30] drm/omap: decrease min width & height

[Tomi Valkeinen]

mode_config's min_width and min_height are both set to 32, which is
overly restrictive.

The real limits depend on whether we're configuring a crtc or a plane,
but a limit of 8x2 is safe for both cases.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 79a4aad35e0f..fe83efbbf127 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -488,8 +488,8 @@ static int omap_modeset_init(struct drm_device *dev)
 		priv->num_planes, priv->num_crtcs, priv->num_encoders,
 		priv->num_connectors);
 
-	dev->mode_config.min_width = 32;
-	dev->mode_config.min_height = 32;
+	dev->mode_config.min_width = 8;
+	dev->mode_config.min_height = 2;
 
 	/* note: eventually will need some cpu_is_omapXYZ() type stuff here
 	 * to fill in these limits properly on different OMAP generations..
-- 
2.7.4

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

Re: [PATCHv3 04/30] drm/omap: decrease min width & height

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote:
> mode_config's min_width and min_height are both set to 32, which is
> overly restrictive.
> 
> The real limits depend on whether we're configuring a crtc or a plane,
> but a limit of 8x2 is safe for both cases.

Strictly speaking it's planes in all cases, you could talk about primary or 
overlay planes instead. Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 79a4aad35e0f..fe83efbbf127
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -488,8 +488,8 @@ static int omap_modeset_init(struct drm_device *dev)
>  		priv->num_planes, priv->num_crtcs, priv->num_encoders,
>  		priv->num_connectors);
> 
> -	dev->mode_config.min_width = 32;
> -	dev->mode_config.min_height = 32;
> +	dev->mode_config.min_width = 8;
> +	dev->mode_config.min_height = 2;
> 
>  	/* note: eventually will need some cpu_is_omapXYZ() type stuff here
>  	 * to fill in these limits properly on different OMAP generations..

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 04/30] drm/omap: decrease min width & height

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 882 bytes --]

On 29/03/17 11:13, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote:
>> mode_config's min_width and min_height are both set to 32, which is
>> overly restrictive.
>>
>> The real limits depend on whether we're configuring a crtc or a plane,
>> but a limit of 8x2 is safe for both cases.
> 
> Strictly speaking it's planes in all cases, you could talk about primary or 
> overlay planes instead. Apart from that,

Hmm, well, on the HW they are different. We have CRTC limits, and we
have plane limits.

In our HW, I think we can have 1x1 planes (at least with RGB). But CRTC
minimum depends on the display used. In some cases CRTC minimum is 1x1,
but usually it's 8x1 I think.

And talking about primary or overlay planes is so legacy... With atomic,
we just have planes! =)

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCHv3 04/30] drm/omap: decrease min width & height

[Laurent Pinchart]

On Wednesday 29 Mar 2017 11:23:00 Tomi Valkeinen wrote:
> On 29/03/17 11:13, Laurent Pinchart wrote:
> > Hi Tomi,
> > 
> > Thank you for the patch.
> > 
> > On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote:
> >> mode_config's min_width and min_height are both set to 32, which is
> >> overly restrictive.
> >> 
> >> The real limits depend on whether we're configuring a crtc or a plane,
> >> but a limit of 8x2 is safe for both cases.
> > 
> > Strictly speaking it's planes in all cases, you could talk about primary
> > or overlay planes instead. Apart from that,
> 
> Hmm, well, on the HW they are different. We have CRTC limits, and we
> have plane limits.
> 
> In our HW, I think we can have 1x1 planes (at least with RGB). But CRTC
> minimum depends on the display used. In some cases CRTC minimum is 1x1,
> but usually it's 8x1 I think.
> 
> And talking about primary or overlay planes is so legacy... With atomic,
> we just have planes! =)

CRTCs and planes are KMS concepts, at the hardware level we have background 
and overlay planes, don't we ?

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 04/30] drm/omap: decrease min width & height

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1632 bytes --]



On 29/03/17 11:24, Laurent Pinchart wrote:
> On Wednesday 29 Mar 2017 11:23:00 Tomi Valkeinen wrote:
>> On 29/03/17 11:13, Laurent Pinchart wrote:
>>> Hi Tomi,
>>>
>>> Thank you for the patch.
>>>
>>> On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote:
>>>> mode_config's min_width and min_height are both set to 32, which is
>>>> overly restrictive.
>>>>
>>>> The real limits depend on whether we're configuring a crtc or a plane,
>>>> but a limit of 8x2 is safe for both cases.
>>>
>>> Strictly speaking it's planes in all cases, you could talk about primary
>>> or overlay planes instead. Apart from that,
>>
>> Hmm, well, on the HW they are different. We have CRTC limits, and we
>> have plane limits.
>>
>> In our HW, I think we can have 1x1 planes (at least with RGB). But CRTC
>> minimum depends on the display used. In some cases CRTC minimum is 1x1,
>> but usually it's 8x1 I think.
>>
>> And talking about primary or overlay planes is so legacy... With atomic,
>> we just have planes! =)
> 
> CRTCs and planes are KMS concepts, at the hardware level we have background 
> and overlay planes, don't we ?

No, on the HW level we have... I don't know what's the proper name, but
whatever matches the CRTC. The dss driver calls it overlay manager, but
that's a silly name too. It's the video timing generator, plus overlay
composition. It does have a default background color, though.

And then we have overlays, i.e. planes.

You can have the CRTC enabled without any planes. Then you'll see the
background color. If you have planes, they can be smaller than the CRTC.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCHv3 04/30] drm/omap: decrease min width & height

[Laurent Pinchart]

Hi Tomi,

On Wednesday 29 Mar 2017 11:26:45 Tomi Valkeinen wrote:
> On 29/03/17 11:24, Laurent Pinchart wrote:
> > On Wednesday 29 Mar 2017 11:23:00 Tomi Valkeinen wrote:
> >> On 29/03/17 11:13, Laurent Pinchart wrote:
> >>> On Tuesday 28 Mar 2017 16:07:50 Tomi Valkeinen wrote:
> >>>> mode_config's min_width and min_height are both set to 32, which is
> >>>> overly restrictive.
> >>>> 
> >>>> The real limits depend on whether we're configuring a crtc or a plane,
> >>>> but a limit of 8x2 is safe for both cases.
> >>> 
> >>> Strictly speaking it's planes in all cases, you could talk about primary
> >>> or overlay planes instead. Apart from that,
> >> 
> >> Hmm, well, on the HW they are different. We have CRTC limits, and we
> >> have plane limits.
> >> 
> >> In our HW, I think we can have 1x1 planes (at least with RGB). But CRTC
> >> minimum depends on the display used. In some cases CRTC minimum is 1x1,
> >> but usually it's 8x1 I think.
> >> 
> >> And talking about primary or overlay planes is so legacy... With atomic,
> >> we just have planes! =)
> > 
> > CRTCs and planes are KMS concepts, at the hardware level we have
> > background and overlay planes, don't we ?
> 
> No, on the HW level we have... I don't know what's the proper name, but
> whatever matches the CRTC. The dss driver calls it overlay manager, but
> that's a silly name too. It's the video timing generator, plus overlay
> composition. It does have a default background color, though.
> 
> And then we have overlays, i.e. planes.
> 
> You can have the CRTC enabled without any planes. Then you'll see the
> background color. If you have planes, they can be smaller than the CRTC.

The mode setting min/max width and height are used by the core when creating a 
framebuffer to validate its size. It's thus only related to planes, not to the 
CRTC.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 04/30] drm/omap: decrease min width & height

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 640 bytes --]

On 29/03/17 11:30, Laurent Pinchart wrote:

> The mode setting min/max width and height are used by the core when creating a 
> framebuffer to validate its size. It's thus only related to planes, not to the 
> CRTC.

It's also set to drm_mode_card_res, and fb's min & max are not
necessarily the same as plane's min and max. But yes, I think you're
right, it's not related to crtc as such. With legacy the fb min defines
the crtc min (I think), but possibly with atomic that's not true.

It's a bit messed up, really. We should have separate limits for crtcs,
planes and fbs. For now the mode_config is all we have.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCHv3 05/30] drm/omap: improve DPI clock selection on DRA7xx

[Tomi Valkeinen]

The clock source selection for the LCD outputs is too hardcoded at the
moment. For example, LCD3 is set to use PLL2_1, and PLL2 doesn't exist
on DRA72x SoCs.

There are quite many ways to configure the clocks, even using HDMI PLL
for LCD outputs, but enabling full configuration of the clocks is rather
tricky.

This patch improves the situation a bit by checking if the PLL about to
be used exists, and if not, tries another one.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dpi.c | 47 ++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
index e0b0c5c24c55..0f32d5d078c6 100644
--- a/drivers/gpu/drm/omapdrm/dss/dpi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
@@ -67,6 +67,42 @@ static struct dpi_data *dpi_get_data_from_pdev(struct platform_device *pdev)
 	return dev_get_drvdata(&pdev->dev);
 }
 
+static enum dss_clk_source dpi_get_clk_src_dra7xx(enum omap_channel channel)
+{
+	/*
+	 * Possible clock sources:
+	 * LCD1: FCK/PLL1_1/HDMI_PLL
+	 * LCD2: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_3)
+	 * LCD3: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_1)
+	 */
+
+	switch (channel) {
+	case OMAP_DSS_CHANNEL_LCD:
+	{
+		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_1))
+			return DSS_CLK_SRC_PLL1_1;
+	}
+	case OMAP_DSS_CHANNEL_LCD2:
+	{
+		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3))
+			return DSS_CLK_SRC_PLL1_3;
+		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_3))
+			return DSS_CLK_SRC_PLL2_3;
+	}
+	case OMAP_DSS_CHANNEL_LCD3:
+	{
+		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_1))
+			return DSS_CLK_SRC_PLL2_1;
+		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3))
+			return DSS_CLK_SRC_PLL1_3;
+	}
+	default:
+		break;
+	}
+
+	return DSS_CLK_SRC_FCK;
+}
+
 static enum dss_clk_source dpi_get_clk_src(enum omap_channel channel)
 {
 	/*
@@ -107,16 +143,7 @@ static enum dss_clk_source dpi_get_clk_src(enum omap_channel channel)
 		}
 
 	case OMAPDSS_VER_DRA7xx:
-		switch (channel) {
-		case OMAP_DSS_CHANNEL_LCD:
-			return DSS_CLK_SRC_PLL1_1;
-		case OMAP_DSS_CHANNEL_LCD2:
-			return DSS_CLK_SRC_PLL1_3;
-		case OMAP_DSS_CHANNEL_LCD3:
-			return DSS_CLK_SRC_PLL2_1;
-		default:
-			return DSS_CLK_SRC_FCK;
-		}
+		return dpi_get_clk_src_dra7xx(channel);
 
 	default:
 		return DSS_CLK_SRC_FCK;
-- 
2.7.4

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

Re: [PATCHv3 05/30] drm/omap: improve DPI clock selection on DRA7xx

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:51 Tomi Valkeinen wrote:
> The clock source selection for the LCD outputs is too hardcoded at the
> moment. For example, LCD3 is set to use PLL2_1, and PLL2 doesn't exist
> on DRA72x SoCs.
> 
> There are quite many ways to configure the clocks, even using HDMI PLL
> for LCD outputs, but enabling full configuration of the clocks is rather
> tricky.
> 
> This patch improves the situation a bit by checking if the PLL about to
> be used exists, and if not, tries another one.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dpi.c | 47 +++++++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c
> b/drivers/gpu/drm/omapdrm/dss/dpi.c index e0b0c5c24c55..0f32d5d078c6 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> @@ -67,6 +67,42 @@ static struct dpi_data *dpi_get_data_from_pdev(struct
> platform_device *pdev) return dev_get_drvdata(&pdev->dev);
>  }
> 
> +static enum dss_clk_source dpi_get_clk_src_dra7xx(enum omap_channel
> channel)
> +{
> +	/*
> +	 * Possible clock sources:
> +	 * LCD1: FCK/PLL1_1/HDMI_PLL
> +	 * LCD2: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_3)
> +	 * LCD3: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_1)
> +	 */
> +
> +	switch (channel) {
> +	case OMAP_DSS_CHANNEL_LCD:
> +	{
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_1))
> +			return DSS_CLK_SRC_PLL1_1;
> +	}

Aren't you missing break statements ?

> +	case OMAP_DSS_CHANNEL_LCD2:
> +	{
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3))
> +			return DSS_CLK_SRC_PLL1_3;
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_3))
> +			return DSS_CLK_SRC_PLL2_3;
> +	}
> +	case OMAP_DSS_CHANNEL_LCD3:
> +	{
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_1))
> +			return DSS_CLK_SRC_PLL2_1;
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3))
> +			return DSS_CLK_SRC_PLL1_3;

What happens if LCD2 is already using PLL1_3 ?

> +	}
> +	default:
> +		break;
> +	}
> +
> +	return DSS_CLK_SRC_FCK;
> +}
> +
>  static enum dss_clk_source dpi_get_clk_src(enum omap_channel channel)
>  {
>  	/*
> @@ -107,16 +143,7 @@ static enum dss_clk_source dpi_get_clk_src(enum
> omap_channel channel) }
> 
>  	case OMAPDSS_VER_DRA7xx:
> -		switch (channel) {
> -		case OMAP_DSS_CHANNEL_LCD:
> -			return DSS_CLK_SRC_PLL1_1;
> -		case OMAP_DSS_CHANNEL_LCD2:
> -			return DSS_CLK_SRC_PLL1_3;
> -		case OMAP_DSS_CHANNEL_LCD3:
> -			return DSS_CLK_SRC_PLL2_1;
> -		default:
> -			return DSS_CLK_SRC_FCK;
> -		}
> +		return dpi_get_clk_src_dra7xx(channel);
> 
>  	default:
>  		return DSS_CLK_SRC_FCK;

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 05/30] drm/omap: improve DPI clock selection on DRA7xx

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 2974 bytes --]

On 29/03/17 11:19, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Mar 2017 16:07:51 Tomi Valkeinen wrote:
>> The clock source selection for the LCD outputs is too hardcoded at the
>> moment. For example, LCD3 is set to use PLL2_1, and PLL2 doesn't exist
>> on DRA72x SoCs.
>>
>> There are quite many ways to configure the clocks, even using HDMI PLL
>> for LCD outputs, but enabling full configuration of the clocks is rather
>> tricky.
>>
>> This patch improves the situation a bit by checking if the PLL about to
>> be used exists, and if not, tries another one.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/dpi.c | 47 +++++++++++++++++++++++++++---------
>>  1 file changed, 37 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c
>> b/drivers/gpu/drm/omapdrm/dss/dpi.c index e0b0c5c24c55..0f32d5d078c6 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
>> @@ -67,6 +67,42 @@ static struct dpi_data *dpi_get_data_from_pdev(struct
>> platform_device *pdev) return dev_get_drvdata(&pdev->dev);
>>  }
>>
>> +static enum dss_clk_source dpi_get_clk_src_dra7xx(enum omap_channel
>> channel)
>> +{
>> +	/*
>> +	 * Possible clock sources:
>> +	 * LCD1: FCK/PLL1_1/HDMI_PLL
>> +	 * LCD2: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_3)
>> +	 * LCD3: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_1)
>> +	 */
>> +
>> +	switch (channel) {
>> +	case OMAP_DSS_CHANNEL_LCD:
>> +	{
>> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_1))
>> +			return DSS_CLK_SRC_PLL1_1;
>> +	}
> 
> Aren't you missing break statements ?

Huh, indeed I am. Thanks! Interesting that we've never hit that bug
(this has been in TI kernel for some time).

> 
>> +	case OMAP_DSS_CHANNEL_LCD2:
>> +	{
>> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3))
>> +			return DSS_CLK_SRC_PLL1_3;
>> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_3))
>> +			return DSS_CLK_SRC_PLL2_3;
>> +	}
>> +	case OMAP_DSS_CHANNEL_LCD3:
>> +	{
>> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_1))
>> +			return DSS_CLK_SRC_PLL2_1;
>> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3))
>> +			return DSS_CLK_SRC_PLL1_3;
> 
> What happens if LCD2 is already using PLL1_3 ?

Bad things, I guess? =)

It is a limitation with the current implementation that some
combinations are not really possible. To support all the cases, we
should somehow share the clocks, and manage the needs of the displays
using the same clocks... We have nothing for that.

So if you design a board that uses LCD2 and LCD3, and you don't have
PLL2, then the driver won't work.

Sharing the clock with two displays of exact same pixel clock possibly
works even now, but I have not tested it.

This patch improves the situation a bit, by allowing all cases where we
don't share the clocks, but it doesn't try to fix the sharing problem.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCHv3 06/30] drm/omap: Add support for render nodes

[Tomi Valkeinen]

From: Hemant Hariyani <hemanthariyani@ti.com>

Add support for render nodes in omap driver and allow required
ioctls to be accessible via render nodes.

This enables unprivileged clients to allocate resources like GEM buffers
for rendering their content into. Mode setting (KMS ioctls) is not
allowed using render nodes. These buffers are then shared with
a previleged process (e.g compositor) that has mode setting access.

An example of this use case is Android where the hardware composer is
the only master and has mode setting access. Every other client then
uses render node(e.g /dev/dri/renderD128 to allocate and use its buffers.

Signed-off-by: Hemant Hariyani <hemanthariyani@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index fe83efbbf127..ce0a1c04403d 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -629,12 +629,18 @@ static int ioctl_gem_info(struct drm_device *dev, void *data,
 }
 
 static const struct drm_ioctl_desc ioctls[DRM_COMMAND_END - DRM_COMMAND_BASE] = {
-	DRM_IOCTL_DEF_DRV(OMAP_GET_PARAM, ioctl_get_param, DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, ioctl_set_param, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
-	DRM_IOCTL_DEF_DRV(OMAP_GEM_NEW, ioctl_gem_new, DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_PREP, ioctl_gem_cpu_prep, DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_FINI, ioctl_gem_cpu_fini, DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(OMAP_GEM_INFO, ioctl_gem_info, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(OMAP_GET_PARAM, ioctl_get_param,
+			  DRM_AUTH | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, ioctl_set_param,
+			  DRM_AUTH | DRM_MASTER | DRM_ROOT_ONLY),
+	DRM_IOCTL_DEF_DRV(OMAP_GEM_NEW, ioctl_gem_new,
+			  DRM_AUTH | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_PREP, ioctl_gem_cpu_prep,
+			  DRM_AUTH | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_FINI, ioctl_gem_cpu_fini,
+			  DRM_AUTH | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(OMAP_GEM_INFO, ioctl_gem_info,
+			  DRM_AUTH | DRM_RENDER_ALLOW),
 };
 
 /*
@@ -724,7 +730,7 @@ static const struct file_operations omapdriver_fops = {
 
 static struct drm_driver omap_drm_driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM  | DRIVER_PRIME |
-		DRIVER_ATOMIC,
+		DRIVER_ATOMIC | DRIVER_RENDER,
 	.open = dev_open,
 	.lastclose = dev_lastclose,
 #ifdef CONFIG_DEBUG_FS
-- 
2.7.4

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

Re: [PATCHv3 06/30] drm/omap: Add support for render nodes

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:52 Tomi Valkeinen wrote:
> From: Hemant Hariyani <hemanthariyani@ti.com>
> 
> Add support for render nodes in omap driver and allow required
> ioctls to be accessible via render nodes.

But the OMAP DSS doesn't perform rendering... This seems an abuse of render 
nodes, I think the API should instead be implemented by the GPU driver.

> This enables unprivileged clients to allocate resources like GEM buffers
> for rendering their content into. Mode setting (KMS ioctls) is not
> allowed using render nodes. These buffers are then shared with
> a previleged process (e.g compositor) that has mode setting access.
> 
> An example of this use case is Android where the hardware composer is
> the only master and has mode setting access. Every other client then
> uses render node(e.g /dev/dri/renderD128 to allocate and use its buffers.
> 
> Signed-off-by: Hemant Hariyani <hemanthariyani@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index fe83efbbf127..ce0a1c04403d
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -629,12 +629,18 @@ static int ioctl_gem_info(struct drm_device *dev, void
> *data, }
> 
>  static const struct drm_ioctl_desc ioctls[DRM_COMMAND_END -
> DRM_COMMAND_BASE] = { -	DRM_IOCTL_DEF_DRV(OMAP_GET_PARAM, 
ioctl_get_param,
> DRM_AUTH),
> -	DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, ioctl_set_param,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), -	DRM_IOCTL_DEF_DRV(OMAP_GEM_NEW,
> ioctl_gem_new, DRM_AUTH),
> -	DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_PREP, ioctl_gem_cpu_prep, DRM_AUTH),
> -	DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_FINI, ioctl_gem_cpu_fini, DRM_AUTH),
> -	DRM_IOCTL_DEF_DRV(OMAP_GEM_INFO, ioctl_gem_info, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(OMAP_GET_PARAM, ioctl_get_param,
> +			  DRM_AUTH | DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, ioctl_set_param,
> +			  DRM_AUTH | DRM_MASTER | DRM_ROOT_ONLY),
> +	DRM_IOCTL_DEF_DRV(OMAP_GEM_NEW, ioctl_gem_new,
> +			  DRM_AUTH | DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_PREP, ioctl_gem_cpu_prep,
> +			  DRM_AUTH | DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_FINI, ioctl_gem_cpu_fini,
> +			  DRM_AUTH | DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(OMAP_GEM_INFO, ioctl_gem_info,
> +			  DRM_AUTH | DRM_RENDER_ALLOW),
>  };
> 
>  /*
> @@ -724,7 +730,7 @@ static const struct file_operations omapdriver_fops = {
> 
>  static struct drm_driver omap_drm_driver = {
>  	.driver_features = DRIVER_MODESET | DRIVER_GEM  | DRIVER_PRIME |
> -		DRIVER_ATOMIC,
> +		DRIVER_ATOMIC | DRIVER_RENDER,
>  	.open = dev_open,
>  	.lastclose = dev_lastclose,
>  #ifdef CONFIG_DEBUG_FS

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 06/30] drm/omap: Add support for render nodes

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1678 bytes --]

On 29/03/17 11:22, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Mar 2017 16:07:52 Tomi Valkeinen wrote:
>> From: Hemant Hariyani <hemanthariyani@ti.com>
>>
>> Add support for render nodes in omap driver and allow required
>> ioctls to be accessible via render nodes.
> 
> But the OMAP DSS doesn't perform rendering... This seems an abuse of render 
> nodes, I think the API should instead be implemented by the GPU driver.

I agree that the GPU use case described in the patch sounds a bit odd.
Why not allocate from the GPU driver instead. But here a particular
issue is that to get TILER buffers we need to ask them from the omapdrm.
Probably TILER should not be part of omapdrm, but then, where should it
be...

We also have writeback in DSS, which can function as a memory to memory
or capture device. That's not supported in the mainline, but we have
support in the TI kernel.

And how about a case where you have the privileged process as KMS
master, and another process wants to draw to a buffer with the CPU, and
then give the buffer to the privileged process for displaying.

So, yes, DSS is not a renderer (well, WB is kind of rendering), but
isn't it a valid use case to allocate a buffer from omapdrm?

Also, where should a buffer be allocated from, generally? I usually
think that if a buffer is going to DSS, I allocate it from omapdrm. Then
again, getting it from the source sounds ok also, i.e. from a v4l2
capture driver... But if we get it from the source, it may not be usable
by all the components in the processing pipeline (e.g, SGX requires 32
pixel aligned buffers).

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCHv3 06/30] drm/omap: Add support for render nodes

[Laurent Pinchart]

Hi Tomi,

(CC'ing Daniel and David)

On Wednesday 29 Mar 2017 11:58:23 Tomi Valkeinen wrote:
> On 29/03/17 11:22, Laurent Pinchart wrote:
> > Hi Tomi,
> > 
> > Thank you for the patch.
> > 
> > On Tuesday 28 Mar 2017 16:07:52 Tomi Valkeinen wrote:
> >> From: Hemant Hariyani <hemanthariyani@ti.com>
> >> 
> >> Add support for render nodes in omap driver and allow required
> >> ioctls to be accessible via render nodes.
> > 
> > But the OMAP DSS doesn't perform rendering... This seems an abuse of
> > render nodes, I think the API should instead be implemented by the GPU
> > driver.
> 
> I agree that the GPU use case described in the patch sounds a bit odd.
> Why not allocate from the GPU driver instead. But here a particular
> issue is that to get TILER buffers we need to ask them from the omapdrm.
> Probably TILER should not be part of omapdrm, but then, where should it
> be...
> 
> We also have writeback in DSS, which can function as a memory to memory
> or capture device. That's not supported in the mainline, but we have
> support in the TI kernel.
> 
> And how about a case where you have the privileged process as KMS
> master, and another process wants to draw to a buffer with the CPU, and
> then give the buffer to the privileged process for displaying.
> 
> So, yes, DSS is not a renderer (well, WB is kind of rendering), but
> isn't it a valid use case to allocate a buffer from omapdrm?

It could be a valid use case, but it's still an API abuse. It starts sounding 
like a DRM core issue to me. The DRIVER_RENDER flag is not documented, so its 
exact meaning isn't defined. I thought it was supposed to flag the device as a 
renderer (GPU).

commit 1793126fcebd7c18834f95d43b55e387a8803aa8
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Sun Aug 25 18:29:00 2013 +0200

    drm: implement experimental render nodes
    
    Render nodes provide an API for userspace to use non-privileged GPU
    commands without any running DRM-Master. It is useful for offscreen
    rendering, GPGPU clients, and normal render clients which do not perform
    modesetting.

    [...]

You instead use the flag as a way to enable unprivileged clients to allocate 
buffers. If that's what the flag should mean, I wonder if there would be a use 
case for *not* setting it.

> Also, where should a buffer be allocated from, generally? I usually
> think that if a buffer is going to DSS, I allocate it from omapdrm. Then
> again, getting it from the source sounds ok also, i.e. from a v4l2
> capture driver... But if we get it from the source, it may not be usable
> by all the components in the processing pipeline (e.g, SGX requires 32
> pixel aligned buffers).

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 06/30] drm/omap: Add support for render nodes

[David Herrmann]

Hi

On Wed, Mar 29, 2017 at 2:20 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Tomi,
>
> (CC'ing Daniel and David)
>
> On Wednesday 29 Mar 2017 11:58:23 Tomi Valkeinen wrote:
>> On 29/03/17 11:22, Laurent Pinchart wrote:
>> > Hi Tomi,
>> >
>> > Thank you for the patch.
>> >
>> > On Tuesday 28 Mar 2017 16:07:52 Tomi Valkeinen wrote:
>> >> From: Hemant Hariyani <hemanthariyani@ti.com>
>> >>
>> >> Add support for render nodes in omap driver and allow required
>> >> ioctls to be accessible via render nodes.
>> >
>> > But the OMAP DSS doesn't perform rendering... This seems an abuse of
>> > render nodes, I think the API should instead be implemented by the GPU
>> > driver.
>>
>> I agree that the GPU use case described in the patch sounds a bit odd.
>> Why not allocate from the GPU driver instead. But here a particular
>> issue is that to get TILER buffers we need to ask them from the omapdrm.
>> Probably TILER should not be part of omapdrm, but then, where should it
>> be...
>>
>> We also have writeback in DSS, which can function as a memory to memory
>> or capture device. That's not supported in the mainline, but we have
>> support in the TI kernel.
>>
>> And how about a case where you have the privileged process as KMS
>> master, and another process wants to draw to a buffer with the CPU, and
>> then give the buffer to the privileged process for displaying.
>>
>> So, yes, DSS is not a renderer (well, WB is kind of rendering), but
>> isn't it a valid use case to allocate a buffer from omapdrm?
>
> It could be a valid use case, but it's still an API abuse. It starts sounding
> like a DRM core issue to me. The DRIVER_RENDER flag is not documented, so its
> exact meaning isn't defined. I thought it was supposed to flag the device as a
> renderer (GPU).
>
> commit 1793126fcebd7c18834f95d43b55e387a8803aa8
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Sun Aug 25 18:29:00 2013 +0200
>
>     drm: implement experimental render nodes
>
>     Render nodes provide an API for userspace to use non-privileged GPU
>     commands without any running DRM-Master. It is useful for offscreen
>     rendering, GPGPU clients, and normal render clients which do not perform
>     modesetting.
>
>     [...]
>
> You instead use the flag as a way to enable unprivileged clients to allocate
> buffers. If that's what the flag should mean, I wonder if there would be a use
> case for *not* setting it.

Correct. You can (and should) enable all your sandboxed commands on
render nodes. That is, if a command only affects the issuing client,
then it is safe for render-nodes. If two clients have a file-context
opened on the render node, they should be unable to affect each other
(minus accounting, resource allocation, etc.).

The name is historic (I did not come up with it either, but failed at
renaming it..). The DRIVER_RENDER flag only controls whether the
render-node is actually instantiated. I will not object doing that
unconditionally. It will create some useless nodes for legacy drivers,
but we should not care.

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

Re: [PATCHv3 06/30] drm/omap: Add support for render nodes

[Laurent Pinchart]

Hi David,

On Wednesday 29 Mar 2017 14:51:48 David Herrmann wrote:
> On Wed, Mar 29, 2017 at 2:20 PM, Laurent Pinchart wrote:
> > On Wednesday 29 Mar 2017 11:58:23 Tomi Valkeinen wrote:
> >> On 29/03/17 11:22, Laurent Pinchart wrote:
> >>> On Tuesday 28 Mar 2017 16:07:52 Tomi Valkeinen wrote:
> >>>> From: Hemant Hariyani <hemanthariyani@ti.com>
> >>>> 
> >>>> Add support for render nodes in omap driver and allow required
> >>>> ioctls to be accessible via render nodes.
> >>> 
> >>> But the OMAP DSS doesn't perform rendering... This seems an abuse of
> >>> render nodes, I think the API should instead be implemented by the GPU
> >>> driver.
> >> 
> >> I agree that the GPU use case described in the patch sounds a bit odd.
> >> Why not allocate from the GPU driver instead. But here a particular
> >> issue is that to get TILER buffers we need to ask them from the omapdrm.
> >> Probably TILER should not be part of omapdrm, but then, where should it
> >> be...
> >> 
> >> We also have writeback in DSS, which can function as a memory to memory
> >> or capture device. That's not supported in the mainline, but we have
> >> support in the TI kernel.
> >> 
> >> And how about a case where you have the privileged process as KMS
> >> master, and another process wants to draw to a buffer with the CPU, and
> >> then give the buffer to the privileged process for displaying.
> >> 
> >> So, yes, DSS is not a renderer (well, WB is kind of rendering), but
> >> isn't it a valid use case to allocate a buffer from omapdrm?
> > 
> > It could be a valid use case, but it's still an API abuse. It starts
> > sounding like a DRM core issue to me. The DRIVER_RENDER flag is not
> > documented, so its exact meaning isn't defined. I thought it was supposed
> > to flag the device as a renderer (GPU).
> > 
> > commit 1793126fcebd7c18834f95d43b55e387a8803aa8
> > Author: David Herrmann <dh.herrmann@gmail.com>
> > Date:   Sun Aug 25 18:29:00 2013 +0200
> > 
> >     drm: implement experimental render nodes
> >     
> >     Render nodes provide an API for userspace to use non-privileged GPU
> >     commands without any running DRM-Master. It is useful for offscreen
> >     rendering, GPGPU clients, and normal render clients which do not
> >     perform
> >     modesetting.
> >     
> >     [...]
> > 
> > You instead use the flag as a way to enable unprivileged clients to
> > allocate buffers. If that's what the flag should mean, I wonder if there
> > would be a use case for *not* setting it.
> 
> Correct. You can (and should) enable all your sandboxed commands on
> render nodes. That is, if a command only affects the issuing client,
> then it is safe for render-nodes. If two clients have a file-context
> opened on the render node, they should be unable to affect each other
> (minus accounting, resource allocation, etc.).
> 
> The name is historic (I did not come up with it either, but failed at
> renaming it..). The DRIVER_RENDER flag only controls whether the
> render-node is actually instantiated. I will not object doing that
> unconditionally. It will create some useless nodes for legacy drivers,
> but we should not care.

Couldn't we achieve the same effect without render nodes, by allowing GEM 
object allocation on the main DRM node by unauthenticated clients ?

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 06/30] drm/omap: Add support for render nodes

[David Herrmann]

Hey

On Wed, Mar 29, 2017 at 11:42 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi David,
>
> On Wednesday 29 Mar 2017 14:51:48 David Herrmann wrote:
>> On Wed, Mar 29, 2017 at 2:20 PM, Laurent Pinchart wrote:
>> > On Wednesday 29 Mar 2017 11:58:23 Tomi Valkeinen wrote:
>> >> On 29/03/17 11:22, Laurent Pinchart wrote:
>> >>> On Tuesday 28 Mar 2017 16:07:52 Tomi Valkeinen wrote:
>> >>>> From: Hemant Hariyani <hemanthariyani@ti.com>
>> >>>>
>> >>>> Add support for render nodes in omap driver and allow required
>> >>>> ioctls to be accessible via render nodes.
>> >>>
>> >>> But the OMAP DSS doesn't perform rendering... This seems an abuse of
>> >>> render nodes, I think the API should instead be implemented by the GPU
>> >>> driver.
>> >>
>> >> I agree that the GPU use case described in the patch sounds a bit odd.
>> >> Why not allocate from the GPU driver instead. But here a particular
>> >> issue is that to get TILER buffers we need to ask them from the omapdrm.
>> >> Probably TILER should not be part of omapdrm, but then, where should it
>> >> be...
>> >>
>> >> We also have writeback in DSS, which can function as a memory to memory
>> >> or capture device. That's not supported in the mainline, but we have
>> >> support in the TI kernel.
>> >>
>> >> And how about a case where you have the privileged process as KMS
>> >> master, and another process wants to draw to a buffer with the CPU, and
>> >> then give the buffer to the privileged process for displaying.
>> >>
>> >> So, yes, DSS is not a renderer (well, WB is kind of rendering), but
>> >> isn't it a valid use case to allocate a buffer from omapdrm?
>> >
>> > It could be a valid use case, but it's still an API abuse. It starts
>> > sounding like a DRM core issue to me. The DRIVER_RENDER flag is not
>> > documented, so its exact meaning isn't defined. I thought it was supposed
>> > to flag the device as a renderer (GPU).
>> >
>> > commit 1793126fcebd7c18834f95d43b55e387a8803aa8
>> > Author: David Herrmann <dh.herrmann@gmail.com>
>> > Date:   Sun Aug 25 18:29:00 2013 +0200
>> >
>> >     drm: implement experimental render nodes
>> >
>> >     Render nodes provide an API for userspace to use non-privileged GPU
>> >     commands without any running DRM-Master. It is useful for offscreen
>> >     rendering, GPGPU clients, and normal render clients which do not
>> >     perform
>> >     modesetting.
>> >
>> >     [...]
>> >
>> > You instead use the flag as a way to enable unprivileged clients to
>> > allocate buffers. If that's what the flag should mean, I wonder if there
>> > would be a use case for *not* setting it.
>>
>> Correct. You can (and should) enable all your sandboxed commands on
>> render nodes. That is, if a command only affects the issuing client,
>> then it is safe for render-nodes. If two clients have a file-context
>> opened on the render node, they should be unable to affect each other
>> (minus accounting, resource allocation, etc.).
>>
>> The name is historic (I did not come up with it either, but failed at
>> renaming it..). The DRIVER_RENDER flag only controls whether the
>> render-node is actually instantiated. I will not object doing that
>> unconditionally. It will create some useless nodes for legacy drivers,
>> but we should not care.
>
> Couldn't we achieve the same effect without render nodes, by allowing GEM
> object allocation on the main DRM node by unauthenticated clients ?

Using a different inode makes sure you can sand-box the node. That is,
you can now easily mknod a render node in containers and sandboxes
without risk of exposing any other APIs.

I guess you could achieve something similar by careful selection of
which privs to pass to the container. But we preferred a clean cut
back then.

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

Re: [PATCHv3 06/30] drm/omap: Add support for render nodes

[Daniel Vetter]

On Thu, Mar 30, 2017 at 08:44:25AM +0200, David Herrmann wrote:
> Hey
> 
> On Wed, Mar 29, 2017 at 11:42 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > Hi David,
> >
> > On Wednesday 29 Mar 2017 14:51:48 David Herrmann wrote:
> >> On Wed, Mar 29, 2017 at 2:20 PM, Laurent Pinchart wrote:
> >> > On Wednesday 29 Mar 2017 11:58:23 Tomi Valkeinen wrote:
> >> >> On 29/03/17 11:22, Laurent Pinchart wrote:
> >> >>> On Tuesday 28 Mar 2017 16:07:52 Tomi Valkeinen wrote:
> >> >>>> From: Hemant Hariyani <hemanthariyani@ti.com>
> >> >>>>
> >> >>>> Add support for render nodes in omap driver and allow required
> >> >>>> ioctls to be accessible via render nodes.
> >> >>>
> >> >>> But the OMAP DSS doesn't perform rendering... This seems an abuse of
> >> >>> render nodes, I think the API should instead be implemented by the GPU
> >> >>> driver.
> >> >>
> >> >> I agree that the GPU use case described in the patch sounds a bit odd.
> >> >> Why not allocate from the GPU driver instead. But here a particular
> >> >> issue is that to get TILER buffers we need to ask them from the omapdrm.
> >> >> Probably TILER should not be part of omapdrm, but then, where should it
> >> >> be...
> >> >>
> >> >> We also have writeback in DSS, which can function as a memory to memory
> >> >> or capture device. That's not supported in the mainline, but we have
> >> >> support in the TI kernel.
> >> >>
> >> >> And how about a case where you have the privileged process as KMS
> >> >> master, and another process wants to draw to a buffer with the CPU, and
> >> >> then give the buffer to the privileged process for displaying.
> >> >>
> >> >> So, yes, DSS is not a renderer (well, WB is kind of rendering), but
> >> >> isn't it a valid use case to allocate a buffer from omapdrm?
> >> >
> >> > It could be a valid use case, but it's still an API abuse. It starts
> >> > sounding like a DRM core issue to me. The DRIVER_RENDER flag is not
> >> > documented, so its exact meaning isn't defined. I thought it was supposed
> >> > to flag the device as a renderer (GPU).
> >> >
> >> > commit 1793126fcebd7c18834f95d43b55e387a8803aa8
> >> > Author: David Herrmann <dh.herrmann@gmail.com>
> >> > Date:   Sun Aug 25 18:29:00 2013 +0200
> >> >
> >> >     drm: implement experimental render nodes
> >> >
> >> >     Render nodes provide an API for userspace to use non-privileged GPU
> >> >     commands without any running DRM-Master. It is useful for offscreen
> >> >     rendering, GPGPU clients, and normal render clients which do not
> >> >     perform
> >> >     modesetting.
> >> >
> >> >     [...]
> >> >
> >> > You instead use the flag as a way to enable unprivileged clients to
> >> > allocate buffers. If that's what the flag should mean, I wonder if there
> >> > would be a use case for *not* setting it.
> >>
> >> Correct. You can (and should) enable all your sandboxed commands on
> >> render nodes. That is, if a command only affects the issuing client,
> >> then it is safe for render-nodes. If two clients have a file-context
> >> opened on the render node, they should be unable to affect each other
> >> (minus accounting, resource allocation, etc.).
> >>
> >> The name is historic (I did not come up with it either, but failed at
> >> renaming it..). The DRIVER_RENDER flag only controls whether the
> >> render-node is actually instantiated. I will not object doing that
> >> unconditionally. It will create some useless nodes for legacy drivers,
> >> but we should not care.
> >
> > Couldn't we achieve the same effect without render nodes, by allowing GEM
> > object allocation on the main DRM node by unauthenticated clients ?
> 
> Using a different inode makes sure you can sand-box the node. That is,
> you can now easily mknod a render node in containers and sandboxes
> without risk of exposing any other APIs.
> 
> I guess you could achieve something similar by careful selection of
> which privs to pass to the container. But we preferred a clean cut
> back then.

btw would be lovely if you folks could review the render node docs and
make sure they reflect all the nuances discussed here properly:

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#render-nodes

And if not, please send a patch.

Thanks, Daniel
-- 
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

[PATCHv3 07/30] drm/omap: fix HDMI sync polarities

[Tomi Valkeinen]

While implementing writeback support, odd behavior of WBDELAYCOUNT was
observed with the combination of WB capture and HDMI. The result of the
debugging was that the HDMI sync polarities are not set correctly.

The current code sets the sync polarities going from HDMI WP to DISPC
according to the video mode used, which seems to work normally fine, but
causes problems with WB as WB expects the syncs to be active-high.

This patch changes the HDMI sync polarities so that the DISPC always
gets active-high syncs from HDMI WP, and the HDMI core gets sync
polarities according to the used video mode.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/hdmi_wp.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
index b783d5a0750e..597ec9d87d1d 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
@@ -147,15 +147,17 @@ void hdmi_wp_video_config_interface(struct hdmi_wp_data *wp,
 				    struct videomode *vm)
 {
 	u32 r;
-	bool vsync_pol, hsync_pol;
+	bool vsync_inv, hsync_inv;
 	DSSDBG("Enter hdmi_wp_video_config_interface\n");
 
-	vsync_pol = !!(vm->flags & DISPLAY_FLAGS_VSYNC_HIGH);
-	hsync_pol = !!(vm->flags & DISPLAY_FLAGS_HSYNC_HIGH);
+	vsync_inv = !!(vm->flags & DISPLAY_FLAGS_VSYNC_LOW);
+	hsync_inv = !!(vm->flags & DISPLAY_FLAGS_HSYNC_LOW);
 
 	r = hdmi_read_reg(wp->base, HDMI_WP_VIDEO_CFG);
-	r = FLD_MOD(r, vsync_pol, 7, 7);
-	r = FLD_MOD(r, hsync_pol, 6, 6);
+	r = FLD_MOD(r, 1, 7, 7);	/* VSYNC_POL to dispc active high */
+	r = FLD_MOD(r, 1, 6, 6);	/* HSYNC_POL to dispc active high */
+	r = FLD_MOD(r, vsync_inv, 5, 5);	/* CORE_VSYNC_INV */
+	r = FLD_MOD(r, hsync_inv, 4, 4);	/* CORE_HSYNC_INV */
 	r = FLD_MOD(r, !!(vm->flags & DISPLAY_FLAGS_INTERLACED), 3, 3);
 	r = FLD_MOD(r, 1, 1, 0); /* HDMI_TIMING_MASTER_24BIT */
 	hdmi_write_reg(wp->base, HDMI_WP_VIDEO_CFG, r);
-- 
2.7.4

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

Re: [PATCHv3 07/30] drm/omap: fix HDMI sync polarities

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:53 Tomi Valkeinen wrote:
> While implementing writeback support, odd behavior of WBDELAYCOUNT was
> observed with the combination of WB capture and HDMI. The result of the
> debugging was that the HDMI sync polarities are not set correctly.
> 
> The current code sets the sync polarities going from HDMI WP to DISPC
> according to the video mode used, which seems to work normally fine, but
> causes problems with WB as WB expects the syncs to be active-high.
> 
> This patch changes the HDMI sync polarities so that the DISPC always
> gets active-high syncs from HDMI WP, and the HDMI core gets sync
> polarities according to the used video mode.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi_wp.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
> b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c index b783d5a0750e..597ec9d87d1d
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
> @@ -147,15 +147,17 @@ void hdmi_wp_video_config_interface(struct
> hdmi_wp_data *wp, struct videomode *vm)
>  {
>  	u32 r;
> -	bool vsync_pol, hsync_pol;
> +	bool vsync_inv, hsync_inv;
>  	DSSDBG("Enter hdmi_wp_video_config_interface\n");
> 
> -	vsync_pol = !!(vm->flags & DISPLAY_FLAGS_VSYNC_HIGH);
> -	hsync_pol = !!(vm->flags & DISPLAY_FLAGS_HSYNC_HIGH);
> +	vsync_inv = !!(vm->flags & DISPLAY_FLAGS_VSYNC_LOW);
> +	hsync_inv = !!(vm->flags & DISPLAY_FLAGS_HSYNC_LOW);
> 
>  	r = hdmi_read_reg(wp->base, HDMI_WP_VIDEO_CFG);
> -	r = FLD_MOD(r, vsync_pol, 7, 7);
> -	r = FLD_MOD(r, hsync_pol, 6, 6);
> +	r = FLD_MOD(r, 1, 7, 7);	/* VSYNC_POL to dispc active high */
> +	r = FLD_MOD(r, 1, 6, 6);	/* HSYNC_POL to dispc active high */
> +	r = FLD_MOD(r, vsync_inv, 5, 5);	/* CORE_VSYNC_INV */
> +	r = FLD_MOD(r, hsync_inv, 4, 4);	/* CORE_HSYNC_INV */
>  	r = FLD_MOD(r, !!(vm->flags & DISPLAY_FLAGS_INTERLACED), 3, 3);
>  	r = FLD_MOD(r, 1, 1, 0); /* HDMI_TIMING_MASTER_24BIT */
>  	hdmi_write_reg(wp->base, HDMI_WP_VIDEO_CFG, r);

-- 
Regards,

Laurent Pinchart

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

[PATCHv3 08/30] drm/omap: add omapdss-base.ko

[Tomi Valkeinen]

We are working towards enabling omapdss6, which will consists of a new
dss, dispc and dpi drivers. omapdss6 will be a new module. The panel,
encoder and omapdrm will need to use either the current omapdss driver
or the new omapdss6 driver, depending on the platform.

This will be implemented with a common base module and function
pointers.

This patch adds a skeleton omapdss-base.ko module, to which we'll be
moving common dss functionality like registration of the panels.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/Kconfig  | 4 ++++
 drivers/gpu/drm/omapdrm/dss/Makefile | 4 ++++
 drivers/gpu/drm/omapdrm/dss/base.c   | 6 ++++++
 3 files changed, 14 insertions(+)
 create mode 100644 drivers/gpu/drm/omapdrm/dss/base.c

diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
index d1fa730c7d54..f53adb944a0d 100644
--- a/drivers/gpu/drm/omapdrm/dss/Kconfig
+++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
@@ -1,8 +1,12 @@
 config OMAP2_DSS_INIT
 	bool
 
+config OMAP_DSS_BASE
+	tristate
+
 menuconfig OMAP2_DSS
         tristate "OMAP2+ Display Subsystem support"
+	select OMAP_DSS_BASE
 	select VIDEOMODE_HELPERS
 	select OMAP2_DSS_INIT
 	select HDMI
diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile
index b651ec9751e6..c7d070168152 100644
--- a/drivers/gpu/drm/omapdrm/dss/Makefile
+++ b/drivers/gpu/drm/omapdrm/dss/Makefile
@@ -1,4 +1,8 @@
 obj-$(CONFIG_OMAP2_DSS_INIT) += omapdss-boot-init.o
+
+obj-$(CONFIG_OMAP_DSS_BASE) += omapdss-base.o
+omapdss-base-y := base.o
+
 obj-$(CONFIG_OMAP2_DSS) += omapdss.o
 # Core DSS files
 omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o \
diff --git a/drivers/gpu/drm/omapdrm/dss/base.c b/drivers/gpu/drm/omapdrm/dss/base.c
new file mode 100644
index 000000000000..943030a7523c
--- /dev/null
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -0,0 +1,6 @@
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
+MODULE_DESCRIPTION("OMAP Display Subsystem Base");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[PATCHv3 09/30] drm/omap: move dss_initialized to omapdss-base

[Tomi Valkeinen]

omapdss_is_initialized() is used to find out if omapdss has been probed
successfully. This patch moves the related code to the common
omapdss-base module, so that the same support will be there for both
omapdss and omapdss6.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/base.c    | 14 ++++++++++++++
 drivers/gpu/drm/omapdrm/dss/dss.c     | 12 ++----------
 drivers/gpu/drm/omapdrm/dss/omapdss.h |  2 ++
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/base.c b/drivers/gpu/drm/omapdrm/dss/base.c
index 943030a7523c..03942097b9f1 100644
--- a/drivers/gpu/drm/omapdrm/dss/base.c
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -1,6 +1,20 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 
+static bool dss_initialized;
+
+void omapdss_set_is_initialized(bool set)
+{
+	dss_initialized = set;
+}
+EXPORT_SYMBOL(omapdss_set_is_initialized);
+
+bool omapdss_is_initialized(void)
+{
+	return dss_initialized;
+}
+EXPORT_SYMBOL(omapdss_is_initialized);
+
 MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
 MODULE_DESCRIPTION("OMAP Display Subsystem Base");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
index 4e72d2fefb4d..5eb2f1260547 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss.c
@@ -117,14 +117,6 @@ static const char * const dss_generic_clk_source_names[] = {
 	[DSS_CLK_SRC_HDMI_PLL]	= "HDMI PLL",
 };
 
-static bool dss_initialized;
-
-bool omapdss_is_initialized(void)
-{
-	return dss_initialized;
-}
-EXPORT_SYMBOL(omapdss_is_initialized);
-
 static inline void dss_write_reg(const struct dss_reg idx, u32 val)
 {
 	__raw_writel(val, dss.base + idx.idx);
@@ -1266,7 +1258,7 @@ static int dss_bind(struct device *dev)
 
 	pm_set_vt_switch(0);
 
-	dss_initialized = true;
+	omapdss_set_is_initialized(true);
 
 	return 0;
 
@@ -1290,7 +1282,7 @@ static void dss_unbind(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 
-	dss_initialized = false;
+	omapdss_set_is_initialized(false);
 
 	component_unbind_all(&pdev->dev, NULL);
 
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 5b3b961127bd..b644b12345c3 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -844,6 +844,8 @@ omapdss_of_get_first_endpoint(const struct device_node *parent);
 struct omap_dss_device *
 omapdss_of_find_source_for_first_ep(struct device_node *node);
 
+void omapdss_set_is_initialized(bool set);
+
 u32 dispc_read_irqstatus(void);
 void dispc_clear_irqstatus(u32 mask);
 u32 dispc_read_irqenable(void);
-- 
2.7.4

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

[PATCHv3 10/30] drm/omap: output: use dev_err instead of DSSERR

[Tomi Valkeinen]

We don't have omapdss's custom error printing functions in the common
omapdss-base module, to which we want to move output.c.

This patch changes output.c to use dev_err instead of DSSERR so that it
doesn't depend on DSSERR.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/output.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
index a901af5a9bc3..0858958677a1 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -35,14 +35,15 @@ int omapdss_output_set_device(struct omap_dss_device *out,
 	mutex_lock(&output_lock);
 
 	if (out->dst) {
-		DSSERR("output already has device %s connected to it\n",
+		dev_err(out->dev,
+			"output already has device %s connected to it\n",
 			out->dst->name);
 		r = -EINVAL;
 		goto err;
 	}
 
 	if (out->output_type != dssdev->type) {
-		DSSERR("output type and display type don't match\n");
+		dev_err(out->dev, "output type and display type don't match\n");
 		r = -EINVAL;
 		goto err;
 	}
@@ -67,14 +68,16 @@ int omapdss_output_unset_device(struct omap_dss_device *out)
 	mutex_lock(&output_lock);
 
 	if (!out->dst) {
-		DSSERR("output doesn't have a device connected to it\n");
+		dev_err(out->dev,
+			"output doesn't have a device connected to it\n");
 		r = -EINVAL;
 		goto err;
 	}
 
 	if (out->dst->state != OMAP_DSS_DISPLAY_DISABLED) {
-		DSSERR("device %s is not disabled, cannot unset device\n",
-				out->dst->name);
+		dev_err(out->dev,
+			"device %s is not disabled, cannot unset device\n",
+			out->dst->name);
 		r = -EINVAL;
 		goto err;
 	}
-- 
2.7.4

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

Re: [PATCHv3 10/30] drm/omap: output: use dev_err instead of DSSERR

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:56 Tomi Valkeinen wrote:
> We don't have omapdss's custom error printing functions in the common
> omapdss-base module, to which we want to move output.c.
> 
> This patch changes output.c to use dev_err instead of DSSERR so that it
> doesn't depend on DSSERR.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/output.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/output.c
> b/drivers/gpu/drm/omapdrm/dss/output.c index a901af5a9bc3..0858958677a1
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/output.c
> +++ b/drivers/gpu/drm/omapdrm/dss/output.c
> @@ -35,14 +35,15 @@ int omapdss_output_set_device(struct omap_dss_device
> *out, mutex_lock(&output_lock);
> 
>  	if (out->dst) {
> -		DSSERR("output already has device %s connected to it\n",
> +		dev_err(out->dev,
> +			"output already has device %s connected to it\n",
>  			out->dst->name);
>  		r = -EINVAL;
>  		goto err;
>  	}
> 
>  	if (out->output_type != dssdev->type) {
> -		DSSERR("output type and display type don't match\n");
> +		dev_err(out->dev, "output type and display type don't 
match\n");
>  		r = -EINVAL;
>  		goto err;
>  	}
> @@ -67,14 +68,16 @@ int omapdss_output_unset_device(struct omap_dss_device
> *out) mutex_lock(&output_lock);
> 
>  	if (!out->dst) {
> -		DSSERR("output doesn't have a device connected to it\n");
> +		dev_err(out->dev,
> +			"output doesn't have a device connected to it\n");
>  		r = -EINVAL;
>  		goto err;
>  	}
> 
>  	if (out->dst->state != OMAP_DSS_DISPLAY_DISABLED) {
> -		DSSERR("device %s is not disabled, cannot unset device\n",
> -				out->dst->name);
> +		dev_err(out->dev,
> +			"device %s is not disabled, cannot unset device\n",
> +			out->dst->name);
>  		r = -EINVAL;
>  		goto err;
>  	}

-- 
Regards,

Laurent Pinchart

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

[PATCHv3 11/30] drm/omap: display: don't use dsi_get_pixel_size()

[Tomi Valkeinen]

display.c uses dsi_get_pixel_size() which is implemented in the DSI
driver, and we won't have that in the omapdss-base module, to which we
want to move display.c

This patch changes display.c not to use dsi_get_pixel_size(). The call
can be replaced with a simple check for OMAP_DSS_DSI_FMT_RGB565.

We can also make dsi_get_pixel_size() static as it's no longer used
outside dsi.c.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/display.c | 6 +++---
 drivers/gpu/drm/omapdrm/dss/dsi.c     | 2 +-
 drivers/gpu/drm/omapdrm/dss/dss.h     | 8 --------
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/display.c b/drivers/gpu/drm/omapdrm/dss/display.c
index 425a5a8dff8b..333ba284ef6d 100644
--- a/drivers/gpu/drm/omapdrm/dss/display.c
+++ b/drivers/gpu/drm/omapdrm/dss/display.c
@@ -55,10 +55,10 @@ int omapdss_default_get_recommended_bpp(struct omap_dss_device *dssdev)
 		else
 			return 16;
 	case OMAP_DISPLAY_TYPE_DSI:
-		if (dsi_get_pixel_size(dssdev->panel.dsi_pix_fmt) > 16)
-			return 24;
-		else
+		if (dssdev->panel.dsi_pix_fmt == OMAP_DSS_DSI_FMT_RGB565)
 			return 16;
+		else
+			return 24;
 	case OMAP_DISPLAY_TYPE_VENC:
 	case OMAP_DISPLAY_TYPE_SDI:
 	case OMAP_DISPLAY_TYPE_HDMI:
diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 5e51a5649efb..f50d6fc0d92e 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -527,7 +527,7 @@ static inline int wait_for_bit_change(struct platform_device *dsidev,
 	return !value;
 }
 
-u8 dsi_get_pixel_size(enum omap_dss_dsi_pixel_format fmt)
+static u8 dsi_get_pixel_size(enum omap_dss_dsi_pixel_format fmt)
 {
 	switch (fmt) {
 	case OMAP_DSS_DSI_FMT_RGB888:
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
index 78f6fc75948b..4d568ac76826 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss.h
@@ -315,15 +315,7 @@ void dsi_uninit_platform_driver(void);
 void dsi_dump_clocks(struct seq_file *s);
 
 void dsi_irq_handler(void);
-u8 dsi_get_pixel_size(enum omap_dss_dsi_pixel_format fmt);
 
-#else
-static inline u8 dsi_get_pixel_size(enum omap_dss_dsi_pixel_format fmt)
-{
-	WARN(1, "%s: DSI not compiled in, returning pixel_size as 0\n",
-	     __func__);
-	return 0;
-}
 #endif
 
 /* DPI */
-- 
2.7.4

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

[PATCHv3 12/30] drm/omap: move display, dss-of, output to omapdss-base

[Tomi Valkeinen]

This patch moves the common parts of omapdss to omapdss-base so that
both the current omapdss driver and the new omapdss6 driver can use
them.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/Makefile  | 6 +++---
 drivers/gpu/drm/omapdrm/dss/display.c | 2 --
 drivers/gpu/drm/omapdrm/dss/dss-of.c  | 3 ++-
 drivers/gpu/drm/omapdrm/dss/dss.h     | 4 ----
 drivers/gpu/drm/omapdrm/dss/omapdss.h | 3 +++
 drivers/gpu/drm/omapdrm/dss/output.c  | 1 -
 6 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile
index c7d070168152..75ec30f231c7 100644
--- a/drivers/gpu/drm/omapdrm/dss/Makefile
+++ b/drivers/gpu/drm/omapdrm/dss/Makefile
@@ -1,12 +1,12 @@
 obj-$(CONFIG_OMAP2_DSS_INIT) += omapdss-boot-init.o
 
 obj-$(CONFIG_OMAP_DSS_BASE) += omapdss-base.o
-omapdss-base-y := base.o
+omapdss-base-y := base.o display.o dss-of.o output.o
 
 obj-$(CONFIG_OMAP2_DSS) += omapdss.o
 # Core DSS files
-omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o \
-	output.o dss-of.o pll.o video-pll.o
+omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o \
+	pll.o video-pll.o
 omapdss-$(CONFIG_OMAP2_DSS_DPI) += dpi.o
 omapdss-$(CONFIG_OMAP2_DSS_RFBI) += rfbi.o
 omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o
diff --git a/drivers/gpu/drm/omapdrm/dss/display.c b/drivers/gpu/drm/omapdrm/dss/display.c
index 333ba284ef6d..e567ff68b216 100644
--- a/drivers/gpu/drm/omapdrm/dss/display.c
+++ b/drivers/gpu/drm/omapdrm/dss/display.c
@@ -29,8 +29,6 @@
 #include <linux/of.h>
 
 #include "omapdss.h"
-#include "dss.h"
-#include "dss_features.h"
 
 void omapdss_default_get_resolution(struct omap_dss_device *dssdev,
 			u16 *xres, u16 *yres)
diff --git a/drivers/gpu/drm/omapdrm/dss/dss-of.c b/drivers/gpu/drm/omapdrm/dss/dss-of.c
index dfd4e9621e3b..b46606b0f014 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss-of.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss-of.c
@@ -19,7 +19,6 @@
 #include <linux/seq_file.h>
 
 #include "omapdss.h"
-#include "dss.h"
 
 struct device_node *
 omapdss_of_get_next_port(const struct device_node *parent,
@@ -110,6 +109,7 @@ struct device_node *dss_of_port_get_parent_device(struct device_node *port)
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(dss_of_port_get_parent_device);
 
 u32 dss_of_port_get_port_number(struct device_node *port)
 {
@@ -122,6 +122,7 @@ u32 dss_of_port_get_port_number(struct device_node *port)
 
 	return reg;
 }
+EXPORT_SYMBOL_GPL(dss_of_port_get_port_number);
 
 static struct device_node *omapdss_of_get_remote_port(const struct device_node *node)
 {
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
index 4d568ac76826..b0551f0090b4 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss.h
@@ -253,10 +253,6 @@ struct dss_pll *dss_video_pll_init(struct platform_device *pdev, int id,
 	struct regulator *regulator);
 void dss_video_pll_uninit(struct dss_pll *pll);
 
-/* dss-of */
-struct device_node *dss_of_port_get_parent_device(struct device_node *port);
-u32 dss_of_port_get_port_number(struct device_node *port);
-
 #if defined(CONFIG_OMAP2_DSS_DEBUGFS)
 void dss_debug_dump_clocks(struct seq_file *s);
 #endif
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index b644b12345c3..dab367ae5ca1 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -846,6 +846,9 @@ omapdss_of_find_source_for_first_ep(struct device_node *node);
 
 void omapdss_set_is_initialized(bool set);
 
+struct device_node *dss_of_port_get_parent_device(struct device_node *port);
+u32 dss_of_port_get_port_number(struct device_node *port);
+
 u32 dispc_read_irqstatus(void);
 void dispc_clear_irqstatus(u32 mask);
 u32 dispc_read_irqenable(void);
diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
index 0858958677a1..2b999dc48621 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -22,7 +22,6 @@
 #include <linux/of.h>
 
 #include "omapdss.h"
-#include "dss.h"
 
 static LIST_HEAD(output_list);
 static DEFINE_MUTEX(output_lock);
-- 
2.7.4

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

[PATCHv3 13/30] drm/omap: move dispc related dss-feat funcs to dispc

[Tomi Valkeinen]

omapdrm still uses a few non-dispc functions: dss_feat_get_num_mgrs(),
dss_feat_get_num_ovls() and  dss_feat_get_supported_color_modes(). We
want to provide omapdrm a single dispc_ops function pointer struct so
that omapdrm will use either the current omapdss or the new omapdss6
driver depending on the platform.

Those three functions are really dispc functions, but are located in the
dss_features.c for legacy reasons.

This patch adds similar functionss to the dispc, and changes omapdrm to
use those new functions. Underneath the functions still call the
functions from dss_feature.c.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c        | 18 ++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/dss_features.c |  3 ---
 drivers/gpu/drm/omapdrm/dss/dss_features.h |  4 ++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h      |  5 +++++
 drivers/gpu/drm/omapdrm/omap_drv.c         |  4 ++--
 drivers/gpu/drm/omapdrm/omap_irq.c         |  2 +-
 drivers/gpu/drm/omapdrm/omap_plane.c       |  2 +-
 7 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index d956e6266368..53d08dc4ef5a 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -1135,6 +1135,18 @@ static u32 dispc_ovl_get_burst_size(enum omap_plane plane)
 	return unit * 8;
 }
 
+enum omap_color_mode dispc_ovl_get_color_modes(enum omap_plane plane)
+{
+	return dss_feat_get_supported_color_modes(plane);
+}
+EXPORT_SYMBOL(dispc_ovl_get_color_modes);
+
+int dispc_get_num_ovls(void)
+{
+	return dss_feat_get_num_ovls();
+}
+EXPORT_SYMBOL(dispc_get_num_ovls);
+
 static void dispc_mgr_enable_cpr(enum omap_channel channel, bool enable)
 {
 	if (channel == OMAP_DSS_CHANNEL_DIGIT)
@@ -2967,6 +2979,12 @@ void dispc_pck_free_enable(bool enable)
 	REG_FLD_MOD(DISPC_CONTROL, enable ? 1 : 0, 27, 27);
 }
 
+int dispc_get_num_mgrs(void)
+{
+	return dss_feat_get_num_mgrs();
+}
+EXPORT_SYMBOL(dispc_get_num_mgrs);
+
 static void dispc_mgr_enable_fifohandcheck(enum omap_channel channel, bool enable)
 {
 	mgr_fld_write(channel, DISPC_MGR_FLD_FIFOHANDCHECK, enable);
diff --git a/drivers/gpu/drm/omapdrm/dss/dss_features.c b/drivers/gpu/drm/omapdrm/dss/dss_features.c
index ee5b93ce2763..26c29332b8e3 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss_features.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss_features.c
@@ -774,13 +774,11 @@ int dss_feat_get_num_mgrs(void)
 {
 	return omap_current_dss_features->num_mgrs;
 }
-EXPORT_SYMBOL(dss_feat_get_num_mgrs);
 
 int dss_feat_get_num_ovls(void)
 {
 	return omap_current_dss_features->num_ovls;
 }
-EXPORT_SYMBOL(dss_feat_get_num_ovls);
 
 unsigned long dss_feat_get_param_min(enum dss_range_param param)
 {
@@ -806,7 +804,6 @@ enum omap_color_mode dss_feat_get_supported_color_modes(enum omap_plane plane)
 {
 	return omap_current_dss_features->supported_color_modes[plane];
 }
-EXPORT_SYMBOL(dss_feat_get_supported_color_modes);
 
 enum omap_overlay_caps dss_feat_get_overlay_caps(enum omap_plane plane)
 {
diff --git a/drivers/gpu/drm/omapdrm/dss/dss_features.h b/drivers/gpu/drm/omapdrm/dss/dss_features.h
index bb4b7f0e642b..bcec68ba8db9 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss_features.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss_features.h
@@ -104,4 +104,8 @@ void dss_features_init(enum omapdss_version version);
 enum omap_display_type dss_feat_get_supported_displays(enum omap_channel channel);
 enum omap_dss_output_id dss_feat_get_supported_outputs(enum omap_channel channel);
 
+int dss_feat_get_num_mgrs(void);
+int dss_feat_get_num_ovls(void);
+enum omap_color_mode dss_feat_get_supported_color_modes(enum omap_plane plane);
+
 #endif
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index dab367ae5ca1..4d3999c18fa2 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -860,6 +860,9 @@ void dispc_free_irq(void *dev_id);
 int dispc_runtime_get(void);
 void dispc_runtime_put(void);
 
+int dispc_get_num_ovls(void);
+int dispc_get_num_mgrs(void);
+
 void dispc_mgr_enable(enum omap_channel channel, bool enable);
 u32 dispc_mgr_get_vsync_irq(enum omap_channel channel);
 u32 dispc_mgr_get_framedone_irq(enum omap_channel channel);
@@ -872,6 +875,7 @@ void dispc_mgr_set_timings(enum omap_channel channel,
 		const struct videomode *vm);
 void dispc_mgr_setup(enum omap_channel channel,
 		const struct omap_overlay_manager_info *info);
+enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum omap_channel channel);
 u32 dispc_mgr_gamma_size(enum omap_channel channel);
 void dispc_mgr_set_gamma(enum omap_channel channel,
 			 const struct drm_color_lut *lut,
@@ -883,6 +887,7 @@ void dispc_ovl_set_channel_out(enum omap_plane plane,
 		enum omap_channel channel);
 int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
 		bool replication, const struct videomode *vm, bool mem_to_mem);
+enum omap_color_mode dispc_ovl_get_color_modes(enum omap_plane plane);
 
 enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum omap_channel channel);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index ce0a1c04403d..db0b485ef6c2 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -320,8 +320,8 @@ static int omap_modeset_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_dss_device *dssdev = NULL;
-	int num_ovls = dss_feat_get_num_ovls();
-	int num_mgrs = dss_feat_get_num_mgrs();
+	int num_ovls = dispc_get_num_ovls();
+	int num_mgrs = dispc_get_num_mgrs();
 	int num_crtcs;
 	int i, id = 0;
 	int ret;
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index 59f21add6f19..a3fd6e8266c8 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -249,7 +249,7 @@ static const u32 omap_underflow_irqs[] = {
 int omap_drm_irq_install(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
-	unsigned int num_mgrs = dss_feat_get_num_mgrs();
+	unsigned int num_mgrs = dispc_get_num_mgrs();
 	unsigned int max_planes;
 	unsigned int i;
 	int ret;
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 386d90af70f7..1067695af401 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -352,7 +352,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 
 	omap_plane->nformats = omap_framebuffer_get_formats(
 			omap_plane->formats, ARRAY_SIZE(omap_plane->formats),
-			dss_feat_get_supported_color_modes(id));
+			dispc_ovl_get_color_modes(id));
 	omap_plane->id = id;
 	omap_plane->name = plane_names[id];
 
-- 
2.7.4

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

[PATCHv3 14/30] drm/omap: add dispc_ops

[Tomi Valkeinen]

We want to change the dispc API from plain functions to a struct with
functions pointers, so that omapdrm can call either omapdss or omapdss6
depending on the platform.

This patch adds 'struct dispc_ops' and adds functions to omapdss-base
to set and get the ops.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/base.c    | 13 +++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h | 50 +++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/base.c b/drivers/gpu/drm/omapdrm/dss/base.c
index 03942097b9f1..7dd447e6c4d7 100644
--- a/drivers/gpu/drm/omapdrm/dss/base.c
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -2,6 +2,7 @@
 #include <linux/module.h>
 
 static bool dss_initialized;
+static const struct dispc_ops *ops;
 
 void omapdss_set_is_initialized(bool set)
 {
@@ -15,6 +16,18 @@ bool omapdss_is_initialized(void)
 }
 EXPORT_SYMBOL(omapdss_is_initialized);
 
+void dispc_set_ops(const struct dispc_ops *o)
+{
+	ops = o;
+}
+EXPORT_SYMBOL(dispc_set_ops);
+
+const struct dispc_ops *dispc_get_ops(void)
+{
+	return ops;
+}
+EXPORT_SYMBOL(dispc_get_ops);
+
 MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
 MODULE_DESCRIPTION("OMAP Display Subsystem Base");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 4d3999c18fa2..a7c2981fa322 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -929,4 +929,54 @@ int dss_mgr_register_framedone_handler(enum omap_channel channel,
 void dss_mgr_unregister_framedone_handler(enum omap_channel channel,
 		void (*handler)(void *), void *data);
 
+/* dispc ops */
+
+struct dispc_ops {
+	u32 (*read_irqstatus)(void);
+	void (*clear_irqstatus)(u32 mask);
+	u32 (*read_irqenable)(void);
+	void (*write_irqenable)(u32 mask);
+
+	int (*request_irq)(irq_handler_t handler, void *dev_id);
+	void (*free_irq)(void *dev_id);
+
+	int (*runtime_get)(void);
+	void (*runtime_put)(void);
+
+	int (*get_num_ovls)(void);
+	int (*get_num_mgrs)(void);
+
+	void (*mgr_enable)(enum omap_channel channel, bool enable);
+	bool (*mgr_is_enabled)(enum omap_channel channel);
+	u32 (*mgr_get_vsync_irq)(enum omap_channel channel);
+	u32 (*mgr_get_framedone_irq)(enum omap_channel channel);
+	u32 (*mgr_get_sync_lost_irq)(enum omap_channel channel);
+	bool (*mgr_go_busy)(enum omap_channel channel);
+	void (*mgr_go)(enum omap_channel channel);
+	void (*mgr_set_lcd_config)(enum omap_channel channel,
+			const struct dss_lcd_mgr_config *config);
+	void (*mgr_set_timings)(enum omap_channel channel,
+			const struct videomode *vm);
+	void (*mgr_setup)(enum omap_channel channel,
+			const struct omap_overlay_manager_info *info);
+	enum omap_dss_output_id (*mgr_get_supported_outputs)(enum omap_channel channel);
+	u32 (*mgr_gamma_size)(enum omap_channel channel);
+	void (*mgr_set_gamma)(enum omap_channel channel,
+		const struct drm_color_lut *lut,
+		unsigned int length);
+
+	int (*ovl_enable)(enum omap_plane plane, bool enable);
+	bool (*ovl_enabled)(enum omap_plane plane);
+	void (*ovl_set_channel_out)(enum omap_plane plane,
+			enum omap_channel channel);
+	int (*ovl_setup)(enum omap_plane plane, const struct omap_overlay_info *oi,
+			bool replication, const struct videomode *vm,
+			bool mem_to_mem);
+
+	enum omap_color_mode (*ovl_get_color_modes)(enum omap_plane plane);
+};
+
+void dispc_set_ops(const struct dispc_ops *o);
+const struct dispc_ops *dispc_get_ops(void);
+
 #endif /* __OMAP_DRM_DSS_H */
-- 
2.7.4

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

[PATCHv3 15/30] drm/omap: fill dispc_ops

[Tomi Valkeinen]

This patch changes the current omapdss driver to fill a dispc_ops struct
and set it to omapdss-base.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c | 40 +++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 53d08dc4ef5a..f871f69dab1a 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -4359,6 +4359,42 @@ static void dispc_errata_i734_wa(void)
 	REG_FLD_MOD(DISPC_CONFIG, gatestate, 8, 4);
 }
 
+static const struct dispc_ops dispc_ops = {
+	.read_irqstatus = dispc_read_irqstatus,
+	.clear_irqstatus = dispc_clear_irqstatus,
+	.read_irqenable = dispc_read_irqenable,
+	.write_irqenable = dispc_write_irqenable,
+
+	.request_irq = dispc_request_irq,
+	.free_irq = dispc_free_irq,
+
+	.runtime_get = dispc_runtime_get,
+	.runtime_put = dispc_runtime_put,
+
+	.get_num_ovls = dispc_get_num_ovls,
+	.get_num_mgrs = dispc_get_num_mgrs,
+
+	.mgr_enable = dispc_mgr_enable,
+	.mgr_is_enabled = dispc_mgr_is_enabled,
+	.mgr_get_vsync_irq = dispc_mgr_get_vsync_irq,
+	.mgr_get_framedone_irq = dispc_mgr_get_framedone_irq,
+	.mgr_get_sync_lost_irq = dispc_mgr_get_sync_lost_irq,
+	.mgr_go_busy = dispc_mgr_go_busy,
+	.mgr_go = dispc_mgr_go,
+	.mgr_set_lcd_config = dispc_mgr_set_lcd_config,
+	.mgr_set_timings = dispc_mgr_set_timings,
+	.mgr_setup = dispc_mgr_setup,
+	.mgr_get_supported_outputs = dispc_mgr_get_supported_outputs,
+	.mgr_gamma_size = dispc_mgr_gamma_size,
+	.mgr_set_gamma = dispc_mgr_set_gamma,
+
+	.ovl_enable = dispc_ovl_enable,
+	.ovl_enabled = dispc_ovl_enabled,
+	.ovl_set_channel_out = dispc_ovl_set_channel_out,
+	.ovl_setup = dispc_ovl_setup,
+	.ovl_get_color_modes = dispc_ovl_get_color_modes,
+};
+
 /* DISPC HW IP initialisation */
 static int dispc_bind(struct device *dev, struct device *master, void *data)
 {
@@ -4431,6 +4467,8 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
 
 	dispc_runtime_put();
 
+	dispc_set_ops(&dispc_ops);
+
 	dss_debugfs_create_file("dispc", dispc_dump_regs);
 
 	return 0;
@@ -4443,6 +4481,8 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
 static void dispc_unbind(struct device *dev, struct device *master,
 			       void *data)
 {
+	dispc_set_ops(NULL);
+
 	pm_runtime_disable(dev);
 
 	dispc_errata_i734_wa_fini();
-- 
2.7.4

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

[PATCHv3 16/30] drm/omap: use dispc_ops

[Tomi Valkeinen]

Change omapdrm to get dispc_ops and use that to call the dispc functions
instead or direct function calls.

The change is very straightforward.

The only problem was in omap_crtc_init() which calls pipe2vbl(crtc), and
at that point of time the crtc->dev link, which is used to get the
dispc_ops, has not been set up yet. This patch makes omap_crtc_init()
skip the call to pipe2vbl() and instead calls
dispc_ops->mgr_get_vsync_irq() directly.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c  | 36 +++++++++++++++++++++++-------------
 drivers/gpu/drm/omapdrm/omap_drv.c   | 10 ++++++----
 drivers/gpu/drm/omapdrm/omap_drv.h   |  2 ++
 drivers/gpu/drm/omapdrm/omap_irq.c   | 33 +++++++++++++++++----------------
 drivers/gpu/drm/omapdrm/omap_plane.c | 15 +++++++++------
 5 files changed, 57 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 49fc61963af4..1db96b077ae8 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -107,10 +107,12 @@ static struct omap_dss_device *omap_crtc_output[8];
 static int omap_crtc_dss_connect(enum omap_channel channel,
 		struct omap_dss_device *dst)
 {
+	const struct dispc_ops *dispc_ops = dispc_get_ops();
+
 	if (omap_crtc_output[channel])
 		return -EINVAL;
 
-	if ((dispc_mgr_get_supported_outputs(channel) & dst->id) == 0)
+	if ((dispc_ops->mgr_get_supported_outputs(channel) & dst->id) == 0)
 		return -EINVAL;
 
 	omap_crtc_output[channel] = dst;
@@ -134,6 +136,7 @@ static void omap_crtc_dss_start_update(enum omap_channel channel)
 static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 {
 	struct drm_device *dev = crtc->dev;
+	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	enum omap_channel channel = omap_crtc->channel;
 	struct omap_irq_wait *wait;
@@ -144,7 +147,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 		return;
 
 	if (omap_crtc_output[channel]->output_type == OMAP_DISPLAY_TYPE_HDMI) {
-		dispc_mgr_enable(channel, enable);
+		priv->dispc_ops->mgr_enable(channel, enable);
 		omap_crtc->enabled = enable;
 		return;
 	}
@@ -157,8 +160,8 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 		omap_crtc->ignore_digit_sync_lost = true;
 	}
 
-	framedone_irq = dispc_mgr_get_framedone_irq(channel);
-	vsync_irq = dispc_mgr_get_vsync_irq(channel);
+	framedone_irq = priv->dispc_ops->mgr_get_framedone_irq(channel);
+	vsync_irq = priv->dispc_ops->mgr_get_vsync_irq(channel);
 
 	if (enable) {
 		wait = omap_irq_wait_init(dev, vsync_irq, 1);
@@ -178,7 +181,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 			wait = omap_irq_wait_init(dev, vsync_irq, 2);
 	}
 
-	dispc_mgr_enable(channel, enable);
+	priv->dispc_ops->mgr_enable(channel, enable);
 	omap_crtc->enabled = enable;
 
 	ret = omap_irq_wait(dev, wait, msecs_to_jiffies(100));
@@ -198,9 +201,9 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 static int omap_crtc_dss_enable(enum omap_channel channel)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[channel];
+	struct omap_drm_private *priv = omap_crtc->base.dev->dev_private;
 
-	dispc_mgr_set_timings(omap_crtc->channel,
-			&omap_crtc->vm);
+	priv->dispc_ops->mgr_set_timings(omap_crtc->channel, &omap_crtc->vm);
 	omap_crtc_set_enabled(&omap_crtc->base, true);
 
 	return 0;
@@ -225,8 +228,10 @@ static void omap_crtc_dss_set_lcd_config(enum omap_channel channel,
 		const struct dss_lcd_mgr_config *config)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[channel];
+	struct omap_drm_private *priv = omap_crtc->base.dev->dev_private;
+
 	DBG("%s", omap_crtc->name);
-	dispc_mgr_set_lcd_config(omap_crtc->channel, config);
+	priv->dispc_ops->mgr_set_lcd_config(omap_crtc->channel, config);
 }
 
 static int omap_crtc_dss_register_framedone(
@@ -274,6 +279,8 @@ void omap_crtc_error_irq(struct drm_crtc *crtc, uint32_t irqstatus)
 void omap_crtc_vblank_irq(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = omap_crtc->base.dev;
+	struct omap_drm_private *priv = dev->dev_private;
 	bool pending;
 
 	spin_lock(&crtc->dev->event_lock);
@@ -281,7 +288,7 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
 	 * If the dispc is busy we're racing the flush operation. Try again on
 	 * the next vblank interrupt.
 	 */
-	if (dispc_mgr_go_busy(omap_crtc->channel)) {
+	if (priv->dispc_ops->mgr_go_busy(omap_crtc->channel)) {
 		spin_unlock(&crtc->dev->event_lock);
 		return;
 	}
@@ -307,6 +314,7 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
 
 static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
 {
+	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	struct omap_overlay_manager_info info;
 
@@ -317,7 +325,7 @@ static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
 	info.partial_alpha_enabled = false;
 	info.cpr_enable = false;
 
-	dispc_mgr_setup(omap_crtc->channel, &info);
+	priv->dispc_ops->mgr_setup(omap_crtc->channel, &info);
 }
 
 /* -----------------------------------------------------------------------------
@@ -401,6 +409,7 @@ static void omap_crtc_atomic_begin(struct drm_crtc *crtc,
 static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 				   struct drm_crtc_state *old_crtc_state)
 {
+	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	int ret;
 
@@ -414,7 +423,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 			length = crtc->state->gamma_lut->length /
 				sizeof(*lut);
 		}
-		dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
+		priv->dispc_ops->mgr_set_gamma(omap_crtc->channel, lut, length);
 	}
 
 	omap_crtc_write_crtc_properties(crtc);
@@ -429,7 +438,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 	WARN_ON(ret != 0);
 
 	spin_lock_irq(&crtc->dev->event_lock);
-	dispc_mgr_go(omap_crtc->channel);
+	priv->dispc_ops->mgr_go(omap_crtc->channel);
 
 	WARN_ON(omap_crtc->pending);
 	omap_crtc->pending = true;
@@ -542,6 +551,7 @@ void omap_crtc_pre_uninit(void)
 struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 		struct drm_plane *plane, enum omap_channel channel, int id)
 {
+	struct omap_drm_private *priv = dev->dev_private;
 	struct drm_crtc *crtc = NULL;
 	struct omap_crtc *omap_crtc;
 	int ret;
@@ -575,7 +585,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	 * extracted with dispc_mgr_gamma_size(). If it returns 0
 	 * gamma table is not supprted.
 	 */
-	if (dispc_mgr_gamma_size(channel)) {
+	if (priv->dispc_ops->mgr_gamma_size(channel)) {
 		uint gamma_lut_size = 256;
 
 		drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size);
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index db0b485ef6c2..c7dbf30a61fa 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -93,7 +93,7 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
 	struct drm_atomic_state *old_state = commit->state;
 
 	/* Apply the atomic update. */
-	dispc_runtime_get();
+	priv->dispc_ops->runtime_get();
 
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
 
@@ -117,7 +117,7 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
 
 	drm_atomic_helper_cleanup_planes(dev, old_state);
 
-	dispc_runtime_put();
+	priv->dispc_ops->runtime_put();
 
 	drm_atomic_state_put(old_state);
 
@@ -320,8 +320,8 @@ static int omap_modeset_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_dss_device *dssdev = NULL;
-	int num_ovls = dispc_get_num_ovls();
-	int num_mgrs = dispc_get_num_mgrs();
+	int num_ovls = priv->dispc_ops->get_num_ovls();
+	int num_mgrs = priv->dispc_ops->get_num_mgrs();
 	int num_crtcs;
 	int i, id = 0;
 	int ret;
@@ -782,6 +782,8 @@ static int pdev_probe(struct platform_device *pdev)
 		goto err_disconnect_dssdevs;
 	}
 
+	priv->dispc_ops = dispc_get_ops();
+
 	priv->omaprev = pdata->omaprev;
 	priv->wq = alloc_ordered_workqueue("omapdrm", 0);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 9098ea138269..3cb7bf259670 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -59,6 +59,8 @@ int omap_irq_wait(struct drm_device *dev, struct omap_irq_wait *wait,
 struct omap_drm_private {
 	uint32_t omaprev;
 
+	const struct dispc_ops *dispc_ops;
+
 	unsigned int num_crtcs;
 	struct drm_crtc *crtcs[8];
 
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index a3fd6e8266c8..26a3c06aa14d 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -40,8 +40,8 @@ static void omap_irq_update(struct drm_device *dev)
 
 	DBG("irqmask=%08x", irqmask);
 
-	dispc_write_irqenable(irqmask);
-	dispc_read_irqenable();        /* flush posted write */
+	priv->dispc_ops->write_irqenable(irqmask);
+	priv->dispc_ops->read_irqenable();        /* flush posted write */
 }
 
 static void omap_irq_wait_handler(struct omap_irq_wait *wait)
@@ -111,7 +111,7 @@ int omap_irq_enable_vblank(struct drm_crtc *crtc)
 	DBG("dev=%p, crtc=%u", dev, channel);
 
 	spin_lock_irqsave(&priv->wait_lock, flags);
-	priv->irq_mask |= dispc_mgr_get_vsync_irq(channel);
+	priv->irq_mask |= priv->dispc_ops->mgr_get_vsync_irq(channel);
 	omap_irq_update(dev);
 	spin_unlock_irqrestore(&priv->wait_lock, flags);
 
@@ -137,7 +137,7 @@ void omap_irq_disable_vblank(struct drm_crtc *crtc)
 	DBG("dev=%p, crtc=%u", dev, channel);
 
 	spin_lock_irqsave(&priv->wait_lock, flags);
-	priv->irq_mask &= ~dispc_mgr_get_vsync_irq(channel);
+	priv->irq_mask &= ~priv->dispc_ops->mgr_get_vsync_irq(channel);
 	omap_irq_update(dev);
 	spin_unlock_irqrestore(&priv->wait_lock, flags);
 }
@@ -200,9 +200,9 @@ static irqreturn_t omap_irq_handler(int irq, void *arg)
 	unsigned int id;
 	u32 irqstatus;
 
-	irqstatus = dispc_read_irqstatus();
-	dispc_clear_irqstatus(irqstatus);
-	dispc_read_irqstatus();        /* flush posted write */
+	irqstatus = priv->dispc_ops->read_irqstatus();
+	priv->dispc_ops->clear_irqstatus(irqstatus);
+	priv->dispc_ops->read_irqstatus();        /* flush posted write */
 
 	VERB("irqs: %08x", irqstatus);
 
@@ -210,12 +210,12 @@ static irqreturn_t omap_irq_handler(int irq, void *arg)
 		struct drm_crtc *crtc = priv->crtcs[id];
 		enum omap_channel channel = omap_crtc_channel(crtc);
 
-		if (irqstatus & dispc_mgr_get_vsync_irq(channel)) {
+		if (irqstatus & priv->dispc_ops->mgr_get_vsync_irq(channel)) {
 			drm_handle_vblank(dev, id);
 			omap_crtc_vblank_irq(crtc);
 		}
 
-		if (irqstatus & dispc_mgr_get_sync_lost_irq(channel))
+		if (irqstatus & priv->dispc_ops->mgr_get_sync_lost_irq(channel))
 			omap_crtc_error_irq(crtc, irqstatus);
 	}
 
@@ -249,7 +249,7 @@ static const u32 omap_underflow_irqs[] = {
 int omap_drm_irq_install(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
-	unsigned int num_mgrs = dispc_get_num_mgrs();
+	unsigned int num_mgrs = priv->dispc_ops->get_num_mgrs();
 	unsigned int max_planes;
 	unsigned int i;
 	int ret;
@@ -267,13 +267,13 @@ int omap_drm_irq_install(struct drm_device *dev)
 	}
 
 	for (i = 0; i < num_mgrs; ++i)
-		priv->irq_mask |= dispc_mgr_get_sync_lost_irq(i);
+		priv->irq_mask |= priv->dispc_ops->mgr_get_sync_lost_irq(i);
 
-	dispc_runtime_get();
-	dispc_clear_irqstatus(0xffffffff);
-	dispc_runtime_put();
+	priv->dispc_ops->runtime_get();
+	priv->dispc_ops->clear_irqstatus(0xffffffff);
+	priv->dispc_ops->runtime_put();
 
-	ret = dispc_request_irq(omap_irq_handler, dev);
+	ret = priv->dispc_ops->request_irq(omap_irq_handler, dev);
 	if (ret < 0)
 		return ret;
 
@@ -284,6 +284,7 @@ int omap_drm_irq_install(struct drm_device *dev)
 
 void omap_drm_irq_uninstall(struct drm_device *dev)
 {
+	struct omap_drm_private *priv = dev->dev_private;
 	unsigned long irqflags;
 	int i;
 
@@ -304,5 +305,5 @@ void omap_drm_irq_uninstall(struct drm_device *dev)
 		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
 	}
 
-	dispc_free_irq(dev);
+	priv->dispc_ops->free_irq(dev);
 }
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 1067695af401..bdd74692e0cd 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -76,6 +76,7 @@ static void omap_plane_cleanup_fb(struct drm_plane *plane,
 static void omap_plane_atomic_update(struct drm_plane *plane,
 				     struct drm_plane_state *old_state)
 {
+	struct omap_drm_private *priv = plane->dev->dev_private;
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct drm_plane_state *state = plane->state;
 	struct omap_plane_state *omap_state = to_omap_plane_state(state);
@@ -123,25 +124,26 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 	DBG("%d,%d %pad %pad", info.pos_x, info.pos_y,
 			&info.paddr, &info.p_uv_addr);
 
-	dispc_ovl_set_channel_out(omap_plane->id,
+	priv->dispc_ops->ovl_set_channel_out(omap_plane->id,
 				  omap_crtc_channel(state->crtc));
 
 	/* and finally, update omapdss: */
-	ret = dispc_ovl_setup(omap_plane->id, &info, false,
+	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info, false,
 			      omap_crtc_timings(state->crtc), false);
 	if (ret) {
 		dev_err(plane->dev->dev, "Failed to setup plane %s\n",
 			omap_plane->name);
-		dispc_ovl_enable(omap_plane->id, false);
+		priv->dispc_ops->ovl_enable(omap_plane->id, false);
 		return;
 	}
 
-	dispc_ovl_enable(omap_plane->id, true);
+	priv->dispc_ops->ovl_enable(omap_plane->id, true);
 }
 
 static void omap_plane_atomic_disable(struct drm_plane *plane,
 				      struct drm_plane_state *old_state)
 {
+	struct omap_drm_private *priv = plane->dev->dev_private;
 	struct omap_plane_state *omap_state = to_omap_plane_state(plane->state);
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 
@@ -149,7 +151,7 @@ static void omap_plane_atomic_disable(struct drm_plane *plane,
 	omap_state->zorder = plane->type == DRM_PLANE_TYPE_PRIMARY
 			   ? 0 : omap_plane->id;
 
-	dispc_ovl_enable(omap_plane->id, false);
+	priv->dispc_ops->ovl_enable(omap_plane->id, false);
 }
 
 static int omap_plane_atomic_check(struct drm_plane *plane,
@@ -340,6 +342,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 		int id, enum drm_plane_type type,
 		u32 possible_crtcs)
 {
+	struct omap_drm_private *priv = dev->dev_private;
 	struct drm_plane *plane;
 	struct omap_plane *omap_plane;
 	int ret;
@@ -352,7 +355,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 
 	omap_plane->nformats = omap_framebuffer_get_formats(
 			omap_plane->formats, ARRAY_SIZE(omap_plane->formats),
-			dispc_ovl_get_color_modes(id));
+			priv->dispc_ops->ovl_get_color_modes(id));
 	omap_plane->id = id;
 	omap_plane->name = plane_names[id];
 
-- 
2.7.4

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

[PATCHv3 17/30] drm/omap: remove all EXPORT_SYMBOLs from dispc.c

[Tomi Valkeinen]

omapdrm now uses dispc_ops instead of direct function calls so we can
remove all EXPORT_SYMBOLs from dispc. Most of the functions can also be
made static, but a few are used outside dispc.c.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 84 +++++++++++++----------------------
 drivers/gpu/drm/omapdrm/dss/dss.h     |  3 ++
 drivers/gpu/drm/omapdrm/dss/omapdss.h | 42 ------------------
 3 files changed, 33 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index f871f69dab1a..7104cffde176 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -306,6 +306,10 @@ static unsigned long dispc_mgr_pclk_rate(enum omap_channel channel);
 static unsigned long dispc_plane_pclk_rate(enum omap_plane plane);
 static unsigned long dispc_plane_lclk_rate(enum omap_plane plane);
 
+static void dispc_clear_irqstatus(u32 mask);
+static bool dispc_mgr_is_enabled(enum omap_channel channel);
+static void dispc_clear_irqstatus(u32 mask);
+
 static inline void dispc_write_reg(const u16 idx, u32 val)
 {
 	__raw_writel(val, dispc.base + idx);
@@ -581,7 +585,6 @@ int dispc_runtime_get(void)
 	WARN_ON(r < 0);
 	return r < 0 ? r : 0;
 }
-EXPORT_SYMBOL(dispc_runtime_get);
 
 void dispc_runtime_put(void)
 {
@@ -592,54 +595,48 @@ void dispc_runtime_put(void)
 	r = pm_runtime_put_sync(&dispc.pdev->dev);
 	WARN_ON(r < 0 && r != -ENOSYS);
 }
-EXPORT_SYMBOL(dispc_runtime_put);
 
-u32 dispc_mgr_get_vsync_irq(enum omap_channel channel)
+static u32 dispc_mgr_get_vsync_irq(enum omap_channel channel)
 {
 	return mgr_desc[channel].vsync_irq;
 }
-EXPORT_SYMBOL(dispc_mgr_get_vsync_irq);
 
-u32 dispc_mgr_get_framedone_irq(enum omap_channel channel)
+static u32 dispc_mgr_get_framedone_irq(enum omap_channel channel)
 {
 	if (channel == OMAP_DSS_CHANNEL_DIGIT && dispc.feat->no_framedone_tv)
 		return 0;
 
 	return mgr_desc[channel].framedone_irq;
 }
-EXPORT_SYMBOL(dispc_mgr_get_framedone_irq);
 
-u32 dispc_mgr_get_sync_lost_irq(enum omap_channel channel)
+static u32 dispc_mgr_get_sync_lost_irq(enum omap_channel channel)
 {
 	return mgr_desc[channel].sync_lost_irq;
 }
-EXPORT_SYMBOL(dispc_mgr_get_sync_lost_irq);
 
 u32 dispc_wb_get_framedone_irq(void)
 {
 	return DISPC_IRQ_FRAMEDONEWB;
 }
 
-void dispc_mgr_enable(enum omap_channel channel, bool enable)
+static void dispc_mgr_enable(enum omap_channel channel, bool enable)
 {
 	mgr_fld_write(channel, DISPC_MGR_FLD_ENABLE, enable);
 	/* flush posted write */
 	mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
 }
-EXPORT_SYMBOL(dispc_mgr_enable);
 
 static bool dispc_mgr_is_enabled(enum omap_channel channel)
 {
 	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
 }
 
-bool dispc_mgr_go_busy(enum omap_channel channel)
+static bool dispc_mgr_go_busy(enum omap_channel channel)
 {
 	return mgr_fld_read(channel, DISPC_MGR_FLD_GO) == 1;
 }
-EXPORT_SYMBOL(dispc_mgr_go_busy);
 
-void dispc_mgr_go(enum omap_channel channel)
+static void dispc_mgr_go(enum omap_channel channel)
 {
 	WARN_ON(!dispc_mgr_is_enabled(channel));
 	WARN_ON(dispc_mgr_go_busy(channel));
@@ -648,7 +645,6 @@ void dispc_mgr_go(enum omap_channel channel)
 
 	mgr_fld_write(channel, DISPC_MGR_FLD_GO, 1);
 }
-EXPORT_SYMBOL(dispc_mgr_go);
 
 bool dispc_wb_go_busy(void)
 {
@@ -997,7 +993,7 @@ static void dispc_ovl_configure_burst_type(enum omap_plane plane,
 		REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), 0, 29, 29);
 }
 
-void dispc_ovl_set_channel_out(enum omap_plane plane, enum omap_channel channel)
+static void dispc_ovl_set_channel_out(enum omap_plane plane, enum omap_channel channel)
 {
 	int shift;
 	u32 val;
@@ -1057,7 +1053,6 @@ void dispc_ovl_set_channel_out(enum omap_plane plane, enum omap_channel channel)
 	}
 	dispc_write_reg(DISPC_OVL_ATTRIBUTES(plane), val);
 }
-EXPORT_SYMBOL(dispc_ovl_set_channel_out);
 
 static enum omap_channel dispc_ovl_get_channel_out(enum omap_plane plane)
 {
@@ -1135,17 +1130,15 @@ static u32 dispc_ovl_get_burst_size(enum omap_plane plane)
 	return unit * 8;
 }
 
-enum omap_color_mode dispc_ovl_get_color_modes(enum omap_plane plane)
+static enum omap_color_mode dispc_ovl_get_color_modes(enum omap_plane plane)
 {
 	return dss_feat_get_supported_color_modes(plane);
 }
-EXPORT_SYMBOL(dispc_ovl_get_color_modes);
 
-int dispc_get_num_ovls(void)
+static int dispc_get_num_ovls(void)
 {
 	return dss_feat_get_num_ovls();
 }
-EXPORT_SYMBOL(dispc_get_num_ovls);
 
 static void dispc_mgr_enable_cpr(enum omap_channel channel, bool enable)
 {
@@ -2829,9 +2822,8 @@ static int dispc_ovl_setup_common(enum omap_plane plane,
 	return 0;
 }
 
-int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
-		bool replication, const struct videomode *vm,
-		bool mem_to_mem)
+static int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
+		bool replication, const struct videomode *vm, bool mem_to_mem)
 {
 	int r;
 	enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
@@ -2853,7 +2845,6 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
 
 	return r;
 }
-EXPORT_SYMBOL(dispc_ovl_setup);
 
 int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
 		bool mem_to_mem, const struct videomode *vm)
@@ -2923,7 +2914,7 @@ int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
 	return r;
 }
 
-int dispc_ovl_enable(enum omap_plane plane, bool enable)
+static int dispc_ovl_enable(enum omap_plane plane, bool enable)
 {
 	DSSDBG("dispc_enable_plane %d, %d\n", plane, enable);
 
@@ -2931,19 +2922,16 @@ int dispc_ovl_enable(enum omap_plane plane, bool enable)
 
 	return 0;
 }
-EXPORT_SYMBOL(dispc_ovl_enable);
 
-bool dispc_ovl_enabled(enum omap_plane plane)
+static bool dispc_ovl_enabled(enum omap_plane plane)
 {
 	return REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0);
 }
-EXPORT_SYMBOL(dispc_ovl_enabled);
 
-enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum omap_channel channel)
+static enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum omap_channel channel)
 {
 	return dss_feat_get_supported_outputs(channel);
 }
-EXPORT_SYMBOL(dispc_mgr_get_supported_outputs);
 
 void dispc_wb_enable(bool enable)
 {
@@ -2979,11 +2967,10 @@ void dispc_pck_free_enable(bool enable)
 	REG_FLD_MOD(DISPC_CONTROL, enable ? 1 : 0, 27, 27);
 }
 
-int dispc_get_num_mgrs(void)
+static int dispc_get_num_mgrs(void)
 {
 	return dss_feat_get_num_mgrs();
 }
-EXPORT_SYMBOL(dispc_get_num_mgrs);
 
 static void dispc_mgr_enable_fifohandcheck(enum omap_channel channel, bool enable)
 {
@@ -3033,7 +3020,7 @@ static void dispc_mgr_enable_alpha_fixed_zorder(enum omap_channel ch,
 		REG_FLD_MOD(DISPC_CONFIG, enable, 19, 19);
 }
 
-void dispc_mgr_setup(enum omap_channel channel,
+static void dispc_mgr_setup(enum omap_channel channel,
 		const struct omap_overlay_manager_info *info)
 {
 	dispc_mgr_set_default_color(channel, info->default_color);
@@ -3046,7 +3033,6 @@ void dispc_mgr_setup(enum omap_channel channel,
 		dispc_mgr_set_cpr_coef(channel, &info->cpr_coefs);
 	}
 }
-EXPORT_SYMBOL(dispc_mgr_setup);
 
 static void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines)
 {
@@ -3107,7 +3093,7 @@ static void dispc_mgr_enable_stallmode(enum omap_channel channel, bool enable)
 	mgr_fld_write(channel, DISPC_MGR_FLD_STALLMODE, enable);
 }
 
-void dispc_mgr_set_lcd_config(enum omap_channel channel,
+static void dispc_mgr_set_lcd_config(enum omap_channel channel,
 		const struct dss_lcd_mgr_config *config)
 {
 	dispc_mgr_set_io_pad_mode(config->io_pad_mode);
@@ -3123,7 +3109,6 @@ void dispc_mgr_set_lcd_config(enum omap_channel channel,
 
 	dispc_mgr_set_lcd_type_tft(channel);
 }
-EXPORT_SYMBOL(dispc_mgr_set_lcd_config);
 
 static bool _dispc_mgr_size_ok(u16 width, u16 height)
 {
@@ -3254,7 +3239,7 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel,
 }
 
 /* change name to mode? */
-void dispc_mgr_set_timings(enum omap_channel channel,
+static void dispc_mgr_set_timings(enum omap_channel channel,
 			   const struct videomode *vm)
 {
 	unsigned xtot, ytot;
@@ -3301,7 +3286,6 @@ void dispc_mgr_set_timings(enum omap_channel channel,
 
 	dispc_mgr_set_size(channel, t.hactive, t.vactive);
 }
-EXPORT_SYMBOL(dispc_mgr_set_timings);
 
 static void dispc_mgr_set_lcd_divisor(enum omap_channel channel, u16 lck_div,
 		u16 pck_div)
@@ -3781,25 +3765,22 @@ int dispc_mgr_get_clock_div(enum omap_channel channel,
 	return 0;
 }
 
-u32 dispc_read_irqstatus(void)
+static u32 dispc_read_irqstatus(void)
 {
 	return dispc_read_reg(DISPC_IRQSTATUS);
 }
-EXPORT_SYMBOL(dispc_read_irqstatus);
 
-void dispc_clear_irqstatus(u32 mask)
+static void dispc_clear_irqstatus(u32 mask)
 {
 	dispc_write_reg(DISPC_IRQSTATUS, mask);
 }
-EXPORT_SYMBOL(dispc_clear_irqstatus);
 
-u32 dispc_read_irqenable(void)
+static u32 dispc_read_irqenable(void)
 {
 	return dispc_read_reg(DISPC_IRQENABLE);
 }
-EXPORT_SYMBOL(dispc_read_irqenable);
 
-void dispc_write_irqenable(u32 mask)
+static void dispc_write_irqenable(u32 mask)
 {
 	u32 old_mask = dispc_read_reg(DISPC_IRQENABLE);
 
@@ -3808,7 +3789,6 @@ void dispc_write_irqenable(u32 mask)
 
 	dispc_write_reg(DISPC_IRQENABLE, mask);
 }
-EXPORT_SYMBOL(dispc_write_irqenable);
 
 void dispc_enable_sidle(void)
 {
@@ -3820,7 +3800,7 @@ void dispc_disable_sidle(void)
 	REG_FLD_MOD(DISPC_SYSCONFIG, 1, 4, 3);	/* SIDLEMODE: no idle */
 }
 
-u32 dispc_mgr_gamma_size(enum omap_channel channel)
+static u32 dispc_mgr_gamma_size(enum omap_channel channel)
 {
 	const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
 
@@ -3829,7 +3809,6 @@ u32 dispc_mgr_gamma_size(enum omap_channel channel)
 
 	return gdesc->len;
 }
-EXPORT_SYMBOL(dispc_mgr_gamma_size);
 
 static void dispc_mgr_write_gamma_table(enum omap_channel channel)
 {
@@ -3874,7 +3853,7 @@ static const struct drm_color_lut dispc_mgr_gamma_default_lut[] = {
 	{ .red = U16_MAX, .green = U16_MAX, .blue = U16_MAX, },
 };
 
-void dispc_mgr_set_gamma(enum omap_channel channel,
+static void dispc_mgr_set_gamma(enum omap_channel channel,
 			 const struct drm_color_lut *lut,
 			 unsigned int length)
 {
@@ -3920,7 +3899,6 @@ void dispc_mgr_set_gamma(enum omap_channel channel,
 	if (dispc.is_enabled)
 		dispc_mgr_write_gamma_table(channel);
 }
-EXPORT_SYMBOL(dispc_mgr_set_gamma);
 
 static int dispc_init_gamma_tables(void)
 {
@@ -4167,7 +4145,7 @@ static irqreturn_t dispc_irq_handler(int irq, void *arg)
 	return dispc.user_handler(irq, dispc.user_data);
 }
 
-int dispc_request_irq(irq_handler_t handler, void *dev_id)
+static int dispc_request_irq(irq_handler_t handler, void *dev_id)
 {
 	int r;
 
@@ -4189,16 +4167,14 @@ int dispc_request_irq(irq_handler_t handler, void *dev_id)
 
 	return r;
 }
-EXPORT_SYMBOL(dispc_request_irq);
 
-void dispc_free_irq(void *dev_id)
+static void dispc_free_irq(void *dev_id)
 {
 	devm_free_irq(&dispc.pdev->dev, dispc.irq, &dispc);
 
 	dispc.user_handler = NULL;
 	dispc.user_data = NULL;
 }
-EXPORT_SYMBOL(dispc_free_irq);
 
 /*
  * Workaround for errata i734 in DSS dispc
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
index b0551f0090b4..9f8937ae35b4 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss.h
@@ -337,6 +337,9 @@ int dispc_init_platform_driver(void) __init;
 void dispc_uninit_platform_driver(void);
 void dispc_dump_clocks(struct seq_file *s);
 
+int dispc_runtime_get(void);
+void dispc_runtime_put(void);
+
 void dispc_enable_sidle(void);
 void dispc_disable_sidle(void);
 
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index a7c2981fa322..3e61f4b2d1ce 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -849,48 +849,6 @@ void omapdss_set_is_initialized(bool set);
 struct device_node *dss_of_port_get_parent_device(struct device_node *port);
 u32 dss_of_port_get_port_number(struct device_node *port);
 
-u32 dispc_read_irqstatus(void);
-void dispc_clear_irqstatus(u32 mask);
-u32 dispc_read_irqenable(void);
-void dispc_write_irqenable(u32 mask);
-
-int dispc_request_irq(irq_handler_t handler, void *dev_id);
-void dispc_free_irq(void *dev_id);
-
-int dispc_runtime_get(void);
-void dispc_runtime_put(void);
-
-int dispc_get_num_ovls(void);
-int dispc_get_num_mgrs(void);
-
-void dispc_mgr_enable(enum omap_channel channel, bool enable);
-u32 dispc_mgr_get_vsync_irq(enum omap_channel channel);
-u32 dispc_mgr_get_framedone_irq(enum omap_channel channel);
-u32 dispc_mgr_get_sync_lost_irq(enum omap_channel channel);
-bool dispc_mgr_go_busy(enum omap_channel channel);
-void dispc_mgr_go(enum omap_channel channel);
-void dispc_mgr_set_lcd_config(enum omap_channel channel,
-		const struct dss_lcd_mgr_config *config);
-void dispc_mgr_set_timings(enum omap_channel channel,
-		const struct videomode *vm);
-void dispc_mgr_setup(enum omap_channel channel,
-		const struct omap_overlay_manager_info *info);
-enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum omap_channel channel);
-u32 dispc_mgr_gamma_size(enum omap_channel channel);
-void dispc_mgr_set_gamma(enum omap_channel channel,
-			 const struct drm_color_lut *lut,
-			 unsigned int length);
-
-int dispc_ovl_enable(enum omap_plane plane, bool enable);
-bool dispc_ovl_enabled(enum omap_plane plane);
-void dispc_ovl_set_channel_out(enum omap_plane plane,
-		enum omap_channel channel);
-int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
-		bool replication, const struct videomode *vm, bool mem_to_mem);
-enum omap_color_mode dispc_ovl_get_color_modes(enum omap_plane plane);
-
-enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum omap_channel channel);
-
 struct dss_mgr_ops {
 	int (*connect)(enum omap_channel channel,
 		struct omap_dss_device *dst);
-- 
2.7.4

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

[PATCHv3 18/30] drm/omap: remove unused dispc_wb_enable & dispc_wb_is_enabled

[Tomi Valkeinen]

Remove two unused WB functions.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c | 10 ----------
 drivers/gpu/drm/omapdrm/dss/dss.h   |  2 --
 2 files changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 7104cffde176..e9d98046a97f 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -2933,16 +2933,6 @@ static enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum omap_channel
 	return dss_feat_get_supported_outputs(channel);
 }
 
-void dispc_wb_enable(bool enable)
-{
-	dispc_ovl_enable(OMAP_DSS_WB, enable);
-}
-
-bool dispc_wb_is_enabled(void)
-{
-	return dispc_ovl_enabled(OMAP_DSS_WB);
-}
-
 static void dispc_lcd_enable_signal_polarity(bool act_high)
 {
 	if (!dss_has_feature(FEAT_LCDENABLEPOL))
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
index 9f8937ae35b4..ab9f6afbf477 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss.h
@@ -373,8 +373,6 @@ void dispc_set_tv_pclk(unsigned long pclk);
 u32 dispc_wb_get_framedone_irq(void);
 bool dispc_wb_go_busy(void);
 void dispc_wb_go(void);
-void dispc_wb_enable(bool enable);
-bool dispc_wb_is_enabled(void);
 void dispc_wb_set_channel_in(enum dss_writeback_channel channel);
 int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
 		bool mem_to_mem, const struct videomode *vm);
-- 
2.7.4

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

Re: [PATCHv3 18/30] drm/omap: remove unused dispc_wb_enable & dispc_wb_is_enabled

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:04 Tomi Valkeinen wrote:
> Remove two unused WB functions.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c | 10 ----------
>  drivers/gpu/drm/omapdrm/dss/dss.h   |  2 --
>  2 files changed, 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> b/drivers/gpu/drm/omapdrm/dss/dispc.c index 7104cffde176..e9d98046a97f
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -2933,16 +2933,6 @@ static enum omap_dss_output_id
> dispc_mgr_get_supported_outputs(enum omap_channel return
> dss_feat_get_supported_outputs(channel);
>  }
> 
> -void dispc_wb_enable(bool enable)
> -{
> -	dispc_ovl_enable(OMAP_DSS_WB, enable);
> -}
> -
> -bool dispc_wb_is_enabled(void)
> -{
> -	return dispc_ovl_enabled(OMAP_DSS_WB);
> -}
> -
>  static void dispc_lcd_enable_signal_polarity(bool act_high)
>  {
>  	if (!dss_has_feature(FEAT_LCDENABLEPOL))
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h
> b/drivers/gpu/drm/omapdrm/dss/dss.h index 9f8937ae35b4..ab9f6afbf477 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
> @@ -373,8 +373,6 @@ void dispc_set_tv_pclk(unsigned long pclk);
>  u32 dispc_wb_get_framedone_irq(void);
>  bool dispc_wb_go_busy(void);
>  void dispc_wb_go(void);
> -void dispc_wb_enable(bool enable);
> -bool dispc_wb_is_enabled(void);
>  void dispc_wb_set_channel_in(enum dss_writeback_channel channel);
>  int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
>  		bool mem_to_mem, const struct videomode *vm);

-- 
Regards,

Laurent Pinchart

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

[PATCHv3 19/30] drm/omap: fix replication logic

[Tomi Valkeinen]

DSS uses "replication logic" to convert color components from smaller
bit widths to bigger bit widths. Without replication logic, the color
component would be shifted and the least significant bits would be left
at 0, whereas with replication logic, the least significat bits will be
filled with the most significant bit.

For some reason omapdrm leaves replication logic always to off, which,
in the most common case, means that showing 16-bit RGB colors on 24-bit
display gives a bit darker colors than expected.

The reset value of the replication logic bit is enabled, and it should
always be set, as there's no practical reason to disable it. This patch
removes the option to disable it from omapdrm, and always sets it to
enabled in dispc.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 7 ++++---
 drivers/gpu/drm/omapdrm/dss/omapdss.h | 3 +--
 drivers/gpu/drm/omapdrm/omap_plane.c  | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index e9d98046a97f..83241052df6b 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -2823,11 +2823,12 @@ static int dispc_ovl_setup_common(enum omap_plane plane,
 }
 
 static int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
-		bool replication, const struct videomode *vm, bool mem_to_mem)
+		const struct videomode *vm, bool mem_to_mem)
 {
 	int r;
 	enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
 	enum omap_channel channel;
+	const bool replication = true;
 
 	channel = dispc_ovl_get_channel_out(plane);
 
@@ -2854,7 +2855,7 @@ int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
 	enum omap_plane plane = OMAP_DSS_WB;
 	const int pos_x = 0, pos_y = 0;
 	const u8 zorder = 0, global_alpha = 0;
-	const bool replication = false;
+	const bool replication = true;
 	bool truncation;
 	int in_width = vm->hactive;
 	int in_height = vm->vactive;
@@ -4288,7 +4289,7 @@ static void dispc_errata_i734_wa(void)
 
 	/* Setup and enable GFX plane */
 	dispc_ovl_set_channel_out(OMAP_DSS_GFX, OMAP_DSS_CHANNEL_LCD);
-	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, false, &i734.vm, false);
+	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, &i734.vm, false);
 	dispc_ovl_enable(OMAP_DSS_GFX, true);
 
 	/* Set up and enable display manager for LCD1 */
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 3e61f4b2d1ce..4545f1824e66 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -928,8 +928,7 @@ struct dispc_ops {
 	void (*ovl_set_channel_out)(enum omap_plane plane,
 			enum omap_channel channel);
 	int (*ovl_setup)(enum omap_plane plane, const struct omap_overlay_info *oi,
-			bool replication, const struct videomode *vm,
-			bool mem_to_mem);
+			const struct videomode *vm, bool mem_to_mem);
 
 	enum omap_color_mode (*ovl_get_color_modes)(enum omap_plane plane);
 };
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index bdd74692e0cd..78a92422ede1 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -128,7 +128,7 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 				  omap_crtc_channel(state->crtc));
 
 	/* and finally, update omapdss: */
-	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info, false,
+	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info,
 			      omap_crtc_timings(state->crtc), false);
 	if (ret) {
 		dev_err(plane->dev->dev, "Failed to setup plane %s\n",
-- 
2.7.4

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

Re: [PATCHv3 19/30] drm/omap: fix replication logic

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:05 Tomi Valkeinen wrote:
> DSS uses "replication logic" to convert color components from smaller
> bit widths to bigger bit widths. Without replication logic, the color
> component would be shifted and the least significant bits would be left
> at 0, whereas with replication logic, the least significat bits will be
> filled with the most significant bit.
> 
> For some reason omapdrm leaves replication logic always to off, which,
> in the most common case, means that showing 16-bit RGB colors on 24-bit
> display gives a bit darker colors than expected.
> 
> The reset value of the replication logic bit is enabled, and it should
> always be set, as there's no practical reason to disable it. This patch
> removes the option to disable it from omapdrm, and always sets it to
> enabled in dispc.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c   | 7 ++++---
>  drivers/gpu/drm/omapdrm/dss/omapdss.h | 3 +--
>  drivers/gpu/drm/omapdrm/omap_plane.c  | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> b/drivers/gpu/drm/omapdrm/dss/dispc.c index e9d98046a97f..83241052df6b
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -2823,11 +2823,12 @@ static int dispc_ovl_setup_common(enum omap_plane
> plane, }
> 
>  static int dispc_ovl_setup(enum omap_plane plane, const struct
> omap_overlay_info *oi, -		bool replication, const struct 
videomode *vm,
> bool mem_to_mem)
> +		const struct videomode *vm, bool mem_to_mem)
>  {
>  	int r;
>  	enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
>  	enum omap_channel channel;
> +	const bool replication = true;
> 
>  	channel = dispc_ovl_get_channel_out(plane);
> 
> @@ -2854,7 +2855,7 @@ int dispc_wb_setup(const struct
> omap_dss_writeback_info *wi, enum omap_plane plane = OMAP_DSS_WB;
>  	const int pos_x = 0, pos_y = 0;
>  	const u8 zorder = 0, global_alpha = 0;
> -	const bool replication = false;
> +	const bool replication = true;
>  	bool truncation;
>  	int in_width = vm->hactive;
>  	int in_height = vm->vactive;
> @@ -4288,7 +4289,7 @@ static void dispc_errata_i734_wa(void)
> 
>  	/* Setup and enable GFX plane */
>  	dispc_ovl_set_channel_out(OMAP_DSS_GFX, OMAP_DSS_CHANNEL_LCD);
> -	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, false, &i734.vm, false);
> +	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, &i734.vm, false);
>  	dispc_ovl_enable(OMAP_DSS_GFX, true);
> 
>  	/* Set up and enable display manager for LCD1 */
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h index 3e61f4b2d1ce..4545f1824e66
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -928,8 +928,7 @@ struct dispc_ops {
>  	void (*ovl_set_channel_out)(enum omap_plane plane,
>  			enum omap_channel channel);
>  	int (*ovl_setup)(enum omap_plane plane, const struct omap_overlay_info
> *oi, -			bool replication, const struct videomode *vm,
> -			bool mem_to_mem);
> +			const struct videomode *vm, bool mem_to_mem);
> 
>  	enum omap_color_mode (*ovl_get_color_modes)(enum omap_plane plane);
>  };
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index bdd74692e0cd..78a92422ede1
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -128,7 +128,7 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane, omap_crtc_channel(state->crtc));
> 
>  	/* and finally, update omapdss: */
> -	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info, false,
> +	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info,
>  			      omap_crtc_timings(state->crtc), false);
>  	if (ret) {
>  		dev_err(plane->dev->dev, "Failed to setup plane %s\n",

-- 
Regards,

Laurent Pinchart

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

[PATCHv3 20/30] drm/omap: dss: Functions to check components in the display/output list

[Tomi Valkeinen]

From: Peter Ujfalusi <peter.ujfalusi@ti.com>

The functions can be used to check a component (by it's of_node) if it is
part of the omapdss display or output list. If the component is found, it
means that the driver is loaded.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/display.c | 18 ++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h |  3 +++
 drivers/gpu/drm/omapdrm/dss/output.c  | 13 +++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/display.c b/drivers/gpu/drm/omapdrm/dss/display.c
index e567ff68b216..94c012e0584b 100644
--- a/drivers/gpu/drm/omapdrm/dss/display.c
+++ b/drivers/gpu/drm/omapdrm/dss/display.c
@@ -132,6 +132,24 @@ void omapdss_unregister_display(struct omap_dss_device *dssdev)
 }
 EXPORT_SYMBOL(omapdss_unregister_display);
 
+bool omapdss_component_is_display(struct device_node *node)
+{
+	struct omap_dss_device *dssdev;
+	bool found = false;
+
+	mutex_lock(&panel_list_mutex);
+	list_for_each_entry(dssdev, &panel_list, panel_list) {
+		if (dssdev->dev->of_node == node) {
+			found = true;
+			goto out;
+		}
+	}
+out:
+	mutex_unlock(&panel_list_mutex);
+	return found;
+}
+EXPORT_SYMBOL(omapdss_component_is_display);
+
 struct omap_dss_device *omap_dss_get_device(struct omap_dss_device *dssdev)
 {
 	if (!try_module_get(dssdev->owner))
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 4545f1824e66..7e1feb935137 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -936,4 +936,7 @@ struct dispc_ops {
 void dispc_set_ops(const struct dispc_ops *o);
 const struct dispc_ops *dispc_get_ops(void);
 
+bool omapdss_component_is_display(struct device_node *node);
+bool omapdss_component_is_output(struct device_node *node);
+
 #endif /* __OMAP_DRM_DSS_H */
diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
index 2b999dc48621..655c5d73eac9 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -107,6 +107,19 @@ void omapdss_unregister_output(struct omap_dss_device *out)
 }
 EXPORT_SYMBOL(omapdss_unregister_output);
 
+bool omapdss_component_is_output(struct device_node *node)
+{
+	struct omap_dss_device *out;
+
+	list_for_each_entry(out, &output_list, list) {
+		if (out->dev->of_node == node)
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(omapdss_component_is_output);
+
 struct omap_dss_device *omap_dss_get_output(enum omap_dss_output_id id)
 {
 	struct omap_dss_device *out;
-- 
2.7.4

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

[PATCHv3 21/30] drm/omap: dss: Support for detecting display stack readiness

[Tomi Valkeinen]

From: Peter Ujfalusi <peter.ujfalusi@ti.com>

When omapdss is loaded (all core components are in place) create a list of
devices used in the display graph. This list later can be used by omapdrm
via the omapdss_stack_is_ready() function to check that these components
are loaded. Based on this information, omapdrm can defer probe in case when
the omapdss stack is not ready yet.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/base.c    | 107 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/dss.c     |   1 +
 drivers/gpu/drm/omapdrm/dss/omapdss.h |   3 +
 3 files changed, 111 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/base.c b/drivers/gpu/drm/omapdrm/dss/base.c
index 7dd447e6c4d7..13e91faaf7a6 100644
--- a/drivers/gpu/drm/omapdrm/dss/base.c
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -1,9 +1,21 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/list.h>
+#include "omapdss.h"
 
 static bool dss_initialized;
 static const struct dispc_ops *ops;
 
+static struct list_head omapdss_comp_list;
+
+struct omapdss_comp_node {
+	struct list_head list;
+	struct device_node *node;
+	bool dss_core_component;
+};
+
 void omapdss_set_is_initialized(bool set)
 {
 	dss_initialized = set;
@@ -28,6 +40,101 @@ const struct dispc_ops *dispc_get_ops(void)
 }
 EXPORT_SYMBOL(dispc_get_ops);
 
+static bool omapdss_list_contains(const struct device_node *node)
+{
+	struct omapdss_comp_node *comp;
+
+	list_for_each_entry(comp, &omapdss_comp_list, list) {
+		if (comp->node == node)
+			return true;
+	}
+
+	return false;
+}
+
+static void omapdss_walk_device(struct device *dev, struct device_node *node,
+				bool dss_core)
+{
+	struct device_node *n;
+	struct omapdss_comp_node *comp = devm_kzalloc(dev, sizeof(*comp),
+						      GFP_KERNEL);
+
+	if (comp) {
+		comp->node = node;
+		comp->dss_core_component = dss_core;
+		list_add(&comp->list, &omapdss_comp_list);
+	}
+
+	/*
+	 * of_graph_get_remote_port_parent() prints an error if there is no
+	 * port/ports node. To avoid that, check first that there's the node.
+	 */
+	n = of_get_child_by_name(node, "ports");
+	if (!n)
+		n = of_get_child_by_name(node, "port");
+	if (!n)
+		return;
+
+	of_node_put(n);
+
+	n = NULL;
+	while ((n = of_graph_get_next_endpoint(node, n)) != NULL) {
+		struct device_node *pn = of_graph_get_remote_port_parent(n);
+
+		if (!pn)
+			continue;
+
+		if (!of_device_is_available(pn) || omapdss_list_contains(pn)) {
+			of_node_put(pn);
+			continue;
+		}
+
+		omapdss_walk_device(dev, pn, false);
+	}
+}
+
+void omapdss_gather_components(struct device *dev)
+{
+	struct device_node *child;
+
+	INIT_LIST_HEAD(&omapdss_comp_list);
+
+	omapdss_walk_device(dev, dev->of_node, true);
+
+	for_each_available_child_of_node(dev->of_node, child) {
+		if (!of_find_property(child, "compatible", NULL))
+			continue;
+
+		omapdss_walk_device(dev, child, true);
+	}
+}
+EXPORT_SYMBOL(omapdss_gather_components);
+
+static bool omapdss_component_is_loaded(struct omapdss_comp_node *comp)
+{
+	if (comp->dss_core_component)
+		return true;
+	if (omapdss_component_is_display(comp->node))
+		return true;
+	if (omapdss_component_is_output(comp->node))
+		return true;
+
+	return false;
+}
+
+bool omapdss_stack_is_ready(void)
+{
+	struct omapdss_comp_node *comp;
+
+	list_for_each_entry(comp, &omapdss_comp_list, list) {
+		if (!omapdss_component_is_loaded(comp))
+			return false;
+	}
+
+	return true;
+}
+EXPORT_SYMBOL(omapdss_stack_is_ready);
+
 MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
 MODULE_DESCRIPTION("OMAP Display Subsystem Base");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
index 5eb2f1260547..ceb483650f8c 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss.c
@@ -1258,6 +1258,7 @@ static int dss_bind(struct device *dev)
 
 	pm_set_vt_switch(0);
 
+	omapdss_gather_components(dev);
 	omapdss_set_is_initialized(true);
 
 	return 0;
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 7e1feb935137..d5c369bd565c 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -939,4 +939,7 @@ const struct dispc_ops *dispc_get_ops(void);
 bool omapdss_component_is_display(struct device_node *node);
 bool omapdss_component_is_output(struct device_node *node);
 
+bool omapdss_stack_is_ready(void);
+void omapdss_gather_components(struct device *dev);
+
 #endif /* __OMAP_DRM_DSS_H */
-- 
2.7.4

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

[PATCHv3 22/30] drm/omap: Use omapdss_stack_is_ready() to check that the display stack is up

[Tomi Valkeinen]

From: Peter Ujfalusi <peter.ujfalusi@ti.com>

Instead of 'guessing' based on aliases of the status of the DSS drivers,
use the new interface to check that all needed drivers are loaded.
In this way we can be sure that all needed drivers are loaded so it is
safe to continue the probing of omapdrm.
This method will allow the omapdrm to be probed 'headless', without
outputs.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index c7dbf30a61fa..ad8d16cf819c 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -250,7 +250,9 @@ static int omap_connect_dssdevs(void)
 {
 	int r;
 	struct omap_dss_device *dssdev = NULL;
-	bool no_displays = true;
+
+	if (!omapdss_stack_is_ready())
+		return -EPROBE_DEFER;
 
 	for_each_dss_dev(dssdev) {
 		r = dssdev->driver->connect(dssdev);
@@ -260,14 +262,9 @@ static int omap_connect_dssdevs(void)
 		} else if (r) {
 			dev_warn(dssdev->dev, "could not connect display: %s\n",
 				dssdev->name);
-		} else {
-			no_displays = false;
 		}
 	}
 
-	if (no_displays)
-		return -EPROBE_DEFER;
-
 	return 0;
 
 cleanup:
-- 
2.7.4

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

[PATCHv3 23/30] drm/omap: fix plane update warning when crtc is disabled

[Tomi Valkeinen]

If omap_plane_atomic_update() is called when the crtc is disabled, and
the timings are zero, we'll see the following warning:

omapdss DISPC error: cannot calculate scaling settings: pclk is zero
omapdrm omapdrm.0: Failed to setup plane vid2

It shouldn't cause any issues, as the crtc is disabled so the plane is
not used by the HW.

To remove the warning, check whether the crtc is enabled at the
beginning of omap_plane_atomic_update(), and bail out if not.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 78a92422ede1..27c1902415de 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -86,6 +86,11 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 
 	DBG("%s, crtc=%p fb=%p", omap_plane->name, state->crtc, state->fb);
 
+	if (!state->crtc->state->enable) {
+		priv->dispc_ops->ovl_enable(omap_plane->id, false);
+		return;
+	}
+
 	memset(&info, 0, sizeof(info));
 	info.rotation_type = OMAP_DSS_ROT_DMA;
 	info.rotation = OMAP_DSS_ROT_0;
-- 
2.7.4

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

Re: [PATCHv3 23/30] drm/omap: fix plane update warning when crtc is disabled

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:09 Tomi Valkeinen wrote:
> If omap_plane_atomic_update() is called when the crtc is disabled, and
> the timings are zero, we'll see the following warning:
> 
> omapdss DISPC error: cannot calculate scaling settings: pclk is zero
> omapdrm omapdrm.0: Failed to setup plane vid2
> 
> It shouldn't cause any issues, as the crtc is disabled so the plane is
> not used by the HW.
> 
> To remove the warning, check whether the crtc is enabled at the
> beginning of omap_plane_atomic_update(), and bail out if not.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_plane.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index 78a92422ede1..27c1902415de
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -86,6 +86,11 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane,
> 
>  	DBG("%s, crtc=%p fb=%p", omap_plane->name, state->crtc, state->fb);
> 
> +	if (!state->crtc->state->enable) {
> +		priv->dispc_ops->ovl_enable(omap_plane->id, false);
> +		return;
> +	}

I'm a bit puzzled, why is omap_plane_atomic_update() called for a plane on a 
disabled CRTC ?

>  	memset(&info, 0, sizeof(info));
>  	info.rotation_type = OMAP_DSS_ROT_DMA;
>  	info.rotation = OMAP_DSS_ROT_0;

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 23/30] drm/omap: fix plane update warning when crtc is disabled

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1579 bytes --]

On 29/03/17 13:30, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Mar 2017 16:08:09 Tomi Valkeinen wrote:
>> If omap_plane_atomic_update() is called when the crtc is disabled, and
>> the timings are zero, we'll see the following warning:
>>
>> omapdss DISPC error: cannot calculate scaling settings: pclk is zero
>> omapdrm omapdrm.0: Failed to setup plane vid2
>>
>> It shouldn't cause any issues, as the crtc is disabled so the plane is
>> not used by the HW.
>>
>> To remove the warning, check whether the crtc is enabled at the
>> beginning of omap_plane_atomic_update(), and bail out if not.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/omap_plane.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
>> b/drivers/gpu/drm/omapdrm/omap_plane.c index 78a92422ede1..27c1902415de
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
>> @@ -86,6 +86,11 @@ static void omap_plane_atomic_update(struct drm_plane
>> *plane,
>>
>>  	DBG("%s, crtc=%p fb=%p", omap_plane->name, state->crtc, state->fb);
>>
>> +	if (!state->crtc->state->enable) {
>> +		priv->dispc_ops->ovl_enable(omap_plane->id, false);
>> +		return;
>> +	}
> 
> I'm a bit puzzled, why is omap_plane_atomic_update() called for a plane on a 
> disabled CRTC ?

I don't remember, and I can't reproduce this on 4.12. Possibly something
has changed since 4.4...

I'll drop this.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCHv3 24/30] drm/omap: display: Add displays in sorted order to the panel_list

[Tomi Valkeinen]

From: Peter Ujfalusi <peter.ujfalusi@ti.com>

Keep the panel_list ordered according to aliases. The DRM connectors will
be created following the panel_list. By keeping the list ordered the DRM
connectors will be created in the same order regardless of the driver
probe order.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/display.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/display.c b/drivers/gpu/drm/omapdrm/dss/display.c
index 94c012e0584b..26cb59be045e 100644
--- a/drivers/gpu/drm/omapdrm/dss/display.c
+++ b/drivers/gpu/drm/omapdrm/dss/display.c
@@ -83,6 +83,7 @@ static int disp_num_counter;
 int omapdss_register_display(struct omap_dss_device *dssdev)
 {
 	struct omap_dss_driver *drv = dssdev->driver;
+	struct list_head *cur;
 	int id;
 
 	/*
@@ -118,7 +119,14 @@ int omapdss_register_display(struct omap_dss_device *dssdev)
 		drv->get_timings = omapdss_default_get_timings;
 
 	mutex_lock(&panel_list_mutex);
-	list_add_tail(&dssdev->panel_list, &panel_list);
+	list_for_each(cur, &panel_list) {
+		struct omap_dss_device *ldev = list_entry(cur,
+							 struct omap_dss_device,
+							 panel_list);
+		if (strcmp(ldev->alias, dssdev->alias) > 0)
+			break;
+	}
+	list_add_tail(&dssdev->panel_list, cur);
 	mutex_unlock(&panel_list_mutex);
 	return 0;
 }
-- 
2.7.4

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

Re: [PATCHv3 24/30] drm/omap: display: Add displays in sorted order to the panel_list

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:10 Tomi Valkeinen wrote:
> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> Keep the panel_list ordered according to aliases. The DRM connectors will
> be created following the panel_list. By keeping the list ordered the DRM
> connectors will be created in the same order regardless of the driver
> probe order.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/display.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/display.c
> b/drivers/gpu/drm/omapdrm/dss/display.c index 94c012e0584b..26cb59be045e
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/display.c
> +++ b/drivers/gpu/drm/omapdrm/dss/display.c
> @@ -83,6 +83,7 @@ static int disp_num_counter;
>  int omapdss_register_display(struct omap_dss_device *dssdev)
>  {
>  	struct omap_dss_driver *drv = dssdev->driver;
> +	struct list_head *cur;
>  	int id;
> 
>  	/*
> @@ -118,7 +119,14 @@ int omapdss_register_display(struct omap_dss_device
> *dssdev) drv->get_timings = omapdss_default_get_timings;
> 
>  	mutex_lock(&panel_list_mutex);
> -	list_add_tail(&dssdev->panel_list, &panel_list);
> +	list_for_each(cur, &panel_list) {

How about using list_for_each_entry ?

> +		struct omap_dss_device *ldev = list_entry(cur,
> +							 struct 
omap_dss_device,
> +							 panel_list);
> +		if (strcmp(ldev->alias, dssdev->alias) > 0)
> +			break;
> +	}
> +	list_add_tail(&dssdev->panel_list, cur);

What you want to do here is add the new element before cur. list_add_tail() 
does that, but that's not very explicit (at least to me) from the function 
name. I would have used list_add(new, cur->prev), but that's up to you.

>  	mutex_unlock(&panel_list_mutex);
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 24/30] drm/omap: display: Add displays in sorted order to the panel_list

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 2462 bytes --]

On 29/03/17 13:08, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Mar 2017 16:08:10 Tomi Valkeinen wrote:
>> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>
>> Keep the panel_list ordered according to aliases. The DRM connectors will
>> be created following the panel_list. By keeping the list ordered the DRM
>> connectors will be created in the same order regardless of the driver
>> probe order.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/display.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/display.c
>> b/drivers/gpu/drm/omapdrm/dss/display.c index 94c012e0584b..26cb59be045e
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/display.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/display.c
>> @@ -83,6 +83,7 @@ static int disp_num_counter;
>>  int omapdss_register_display(struct omap_dss_device *dssdev)
>>  {
>>  	struct omap_dss_driver *drv = dssdev->driver;
>> +	struct list_head *cur;
>>  	int id;
>>
>>  	/*
>> @@ -118,7 +119,14 @@ int omapdss_register_display(struct omap_dss_device
>> *dssdev) drv->get_timings = omapdss_default_get_timings;
>>
>>  	mutex_lock(&panel_list_mutex);
>> -	list_add_tail(&dssdev->panel_list, &panel_list);
>> +	list_for_each(cur, &panel_list) {
> 
> How about using list_for_each_entry ?

We need the list iterator after the loop to add the new node. Where
would list_for_each_entry()'s 'pos' point to, if there's nothing in the
list yet? I think list_for_each_entry()'s iterator is supposed to be
used only inside the loop.

>> +		struct omap_dss_device *ldev = list_entry(cur,
>> +							 struct 
> omap_dss_device,
>> +							 panel_list);
>> +		if (strcmp(ldev->alias, dssdev->alias) > 0)
>> +			break;
>> +	}
>> +	list_add_tail(&dssdev->panel_list, cur);
> 
> What you want to do here is add the new element before cur. list_add_tail() 
> does that, but that's not very explicit (at least to me) from the function 
> name. I would have used list_add(new, cur->prev), but that's up to you.

list_add_tail's documentation says "Insert a new entry before the
specified head". So while I agree that "tail" is not what comes to my
mind from the description, it sounds like this is exactly the case where
list_add_tail is to be used...

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCHv3 25/30] drm/omap: poll only connectors where the connect/disconnect can be checked

[Tomi Valkeinen]

From: Peter Ujfalusi <peter.ujfalusi@ti.com>

When the connector associated detect callback is not provided, we can not
detect if the display is connected or disconnected. These displays does not
support hot plug, they are always connected. Let DRM know that connectors
w/o detect callback should not be polled.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_connector.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index f90e2d22c5ec..50d2b641c28b 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -229,13 +229,11 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 				connector_type);
 	drm_connector_helper_add(connector, &omap_connector_helper_funcs);
 
-#if 0 /* enable when dss2 supports hotplug */
-	if (dssdev->caps & OMAP_DSS_DISPLAY_CAP_HPD)
-		connector->polled = 0;
-	else
-#endif
+	if (dssdev->driver->detect)
 		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
-				DRM_CONNECTOR_POLL_DISCONNECT;
+				    DRM_CONNECTOR_POLL_DISCONNECT;
+	else
+		connector->polled = 0;
 
 	connector->interlace_allowed = 1;
 	connector->doublescan_allowed = 0;
-- 
2.7.4

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

Re: [PATCHv3 25/30] drm/omap: poll only connectors where the connect/disconnect can be checked

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:11 Tomi Valkeinen wrote:
> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> When the connector associated detect callback is not provided, we can not
> detect if the display is connected or disconnected. These displays does not

s/does/do/

> support hot plug, they are always connected. Let DRM know that connectors
> w/o detect callback should not be polled.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/omap_connector.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c
> b/drivers/gpu/drm/omapdrm/omap_connector.c index f90e2d22c5ec..50d2b641c28b
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -229,13 +229,11 @@ struct drm_connector *omap_connector_init(struct
> drm_device *dev, connector_type);
>  	drm_connector_helper_add(connector, &omap_connector_helper_funcs);
> 
> -#if 0 /* enable when dss2 supports hotplug */
> -	if (dssdev->caps & OMAP_DSS_DISPLAY_CAP_HPD)
> -		connector->polled = 0;
> -	else
> -#endif
> +	if (dssdev->driver->detect)
>  		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
> -				DRM_CONNECTOR_POLL_DISCONNECT;
> +				    DRM_CONNECTOR_POLL_DISCONNECT;
> +	else
> +		connector->polled = 0;
> 
>  	connector->interlace_allowed = 1;
>  	connector->doublescan_allowed = 0;

-- 
Regards,

Laurent Pinchart

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

[PATCHv3 26/30] drm/omap: displays: panel-dpi: Support for handling backlight devices

[Tomi Valkeinen]

From: Peter Ujfalusi <peter.ujfalusi@ti.com>

The associated backlight device can be configured via DT by providing the
phandle to the device.

If the backlight device is configured, the driver can manage the backligt
along with the panel's power state, iow it can turn on the backlight when
the panel is enabled and turn it off when the panel is disabled.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 37 ++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index 38003208d9ca..51fc78eb212c 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -16,6 +16,7 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/regulator/consumer.h>
+#include <linux/backlight.h>
 
 #include <video/omap-panel-data.h>
 #include <video/of_display_timing.h>
@@ -30,6 +31,8 @@ struct panel_drv_data {
 
 	struct videomode vm;
 
+	struct backlight_device *backlight;
+
 	/* used for non-DT boot, to be removed */
 	int backlight_gpio;
 
@@ -97,6 +100,11 @@ static int panel_dpi_enable(struct omap_dss_device *dssdev)
 	if (gpio_is_valid(ddata->backlight_gpio))
 		gpio_set_value_cansleep(ddata->backlight_gpio, 1);
 
+	if (ddata->backlight) {
+		ddata->backlight->props.power = FB_BLANK_UNBLANK;
+		backlight_update_status(ddata->backlight);
+	}
+
 	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
 
 	return 0;
@@ -113,6 +121,11 @@ static void panel_dpi_disable(struct omap_dss_device *dssdev)
 	if (gpio_is_valid(ddata->backlight_gpio))
 		gpio_set_value_cansleep(ddata->backlight_gpio, 0);
 
+	if (ddata->backlight) {
+		ddata->backlight->props.power = FB_BLANK_POWERDOWN;
+		backlight_update_status(ddata->backlight);
+	}
+
 	gpiod_set_value_cansleep(ddata->enable_gpio, 0);
 	regulator_disable(ddata->vcc_supply);
 
@@ -209,6 +222,7 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct device_node *node = pdev->dev.of_node;
+	struct device_node *bl_node;
 	struct omap_dss_device *in;
 	int r;
 	struct display_timing timing;
@@ -236,10 +250,19 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
 
 	ddata->backlight_gpio = -ENOENT;
 
+	bl_node = of_parse_phandle(node, "backlight", 0);
+	if (bl_node) {
+		ddata->backlight = of_find_backlight_by_node(bl_node);
+		of_node_put(bl_node);
+
+		if (!ddata->backlight)
+			return -EPROBE_DEFER;
+	}
+
 	r = of_get_display_timing(node, "panel-timing", &timing);
 	if (r) {
 		dev_err(&pdev->dev, "failed to get video timing\n");
-		return r;
+		goto free_backlight;
 	}
 
 	videomode_from_timing(&timing, &ddata->vm);
@@ -247,12 +270,19 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
 	in = omapdss_of_find_source_for_first_ep(node);
 	if (IS_ERR(in)) {
 		dev_err(&pdev->dev, "failed to find video source\n");
-		return PTR_ERR(in);
+		r = PTR_ERR(in);
+		goto free_backlight;
 	}
 
 	ddata->in = in;
 
 	return 0;
+
+free_backlight:
+	if (ddata->backlight)
+		put_device(&ddata->backlight->dev);
+
+	return r;
 }
 
 static int panel_dpi_probe(struct platform_device *pdev)
@@ -321,6 +351,9 @@ static int __exit panel_dpi_remove(struct platform_device *pdev)
 
 	omap_dss_put_device(in);
 
+	if (ddata->backlight)
+		put_device(&ddata->backlight->dev);
+
 	return 0;
 }
 
-- 
2.7.4

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

Re: [PATCHv3 26/30] drm/omap: displays: panel-dpi: Support for handling backlight devices

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:12 Tomi Valkeinen wrote:
> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> The associated backlight device can be configured via DT by providing the
> phandle to the device.
> 
> If the backlight device is configured, the driver can manage the backligt
> along with the panel's power state, iow it can turn on the backlight when
> the panel is enabled and turn it off when the panel is disabled.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 37 ++++++++++++++++++++++--

The backlight property is missing from the DT bindings.

>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c index
> 38003208d9ca..51fc78eb212c 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> @@ -16,6 +16,7 @@
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/backlight.h>
> 
>  #include <video/omap-panel-data.h>
>  #include <video/of_display_timing.h>
> @@ -30,6 +31,8 @@ struct panel_drv_data {
> 
>  	struct videomode vm;
> 
> +	struct backlight_device *backlight;
> +
>  	/* used for non-DT boot, to be removed */
>  	int backlight_gpio;
> 
> @@ -97,6 +100,11 @@ static int panel_dpi_enable(struct omap_dss_device
> *dssdev) if (gpio_is_valid(ddata->backlight_gpio))
>  		gpio_set_value_cansleep(ddata->backlight_gpio, 1);
> 
> +	if (ddata->backlight) {
> +		ddata->backlight->props.power = FB_BLANK_UNBLANK;
> +		backlight_update_status(ddata->backlight);
> +	}
> +
>  	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> 
>  	return 0;
> @@ -113,6 +121,11 @@ static void panel_dpi_disable(struct omap_dss_device
> *dssdev) if (gpio_is_valid(ddata->backlight_gpio))
>  		gpio_set_value_cansleep(ddata->backlight_gpio, 0);
> 
> +	if (ddata->backlight) {
> +		ddata->backlight->props.power = FB_BLANK_POWERDOWN;
> +		backlight_update_status(ddata->backlight);
> +	}
> +
>  	gpiod_set_value_cansleep(ddata->enable_gpio, 0);
>  	regulator_disable(ddata->vcc_supply);
> 
> @@ -209,6 +222,7 @@ static int panel_dpi_probe_of(struct platform_device
> *pdev) {
>  	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
>  	struct device_node *node = pdev->dev.of_node;
> +	struct device_node *bl_node;
>  	struct omap_dss_device *in;
>  	int r;
>  	struct display_timing timing;
> @@ -236,10 +250,19 @@ static int panel_dpi_probe_of(struct platform_device
> *pdev)
> 
>  	ddata->backlight_gpio = -ENOENT;
> 
> +	bl_node = of_parse_phandle(node, "backlight", 0);
> +	if (bl_node) {
> +		ddata->backlight = of_find_backlight_by_node(bl_node);
> +		of_node_put(bl_node);
> +
> +		if (!ddata->backlight)
> +			return -EPROBE_DEFER;
> +	}
> +
>  	r = of_get_display_timing(node, "panel-timing", &timing);
>  	if (r) {
>  		dev_err(&pdev->dev, "failed to get video timing\n");
> -		return r;
> +		goto free_backlight;
>  	}
> 
>  	videomode_from_timing(&timing, &ddata->vm);
> @@ -247,12 +270,19 @@ static int panel_dpi_probe_of(struct platform_device
> *pdev) in = omapdss_of_find_source_for_first_ep(node);
>  	if (IS_ERR(in)) {
>  		dev_err(&pdev->dev, "failed to find video source\n");
> -		return PTR_ERR(in);
> +		r = PTR_ERR(in);
> +		goto free_backlight;
>  	}
> 
>  	ddata->in = in;
> 
>  	return 0;
> +
> +free_backlight:

I would name this error_free_backlight to clearly show it's an error label, or 
just error as there's a single one.

> +	if (ddata->backlight)
> +		put_device(&ddata->backlight->dev);
> +
> +	return r;
>  }
> 
>  static int panel_dpi_probe(struct platform_device *pdev)
> @@ -321,6 +351,9 @@ static int __exit panel_dpi_remove(struct
> platform_device *pdev)
> 
>  	omap_dss_put_device(in);
> 
> +	if (ddata->backlight)
> +		put_device(&ddata->backlight->dev);
> +
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart

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

[PATCHv3 27/30] drm/omap: dispc: improve debug print of display flags

[Tomi Valkeinen]

Instead of printing 0/1 for display flags like vsync high/low, use a
tri-state print (-1/0/1) to indicate the "undefined" state.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 83241052df6b..2e6a71dbc25d 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -3229,6 +3229,16 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel,
 	}
 }
 
+static int vm_flag_to_int(enum display_flags flags, enum display_flags high,
+	enum display_flags low)
+{
+	if (flags & high)
+		return 1;
+	if (flags & low)
+		return -1;
+	return 0;
+}
+
 /* change name to mode? */
 static void dispc_mgr_set_timings(enum omap_channel channel,
 			   const struct videomode *vm)
@@ -3258,11 +3268,11 @@ static void dispc_mgr_set_timings(enum omap_channel channel,
 			t.hsync_len, t.hfront_porch, t.hback_porch,
 			t.vsync_len, t.vfront_porch, t.vback_porch);
 		DSSDBG("vsync_level %d hsync_level %d data_pclk_edge %d de_level %d sync_pclk_edge %d\n",
-			!!(t.flags & DISPLAY_FLAGS_VSYNC_HIGH),
-			!!(t.flags & DISPLAY_FLAGS_HSYNC_HIGH),
-			!!(t.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE),
-			!!(t.flags & DISPLAY_FLAGS_DE_HIGH),
-			!!(t.flags & DISPLAY_FLAGS_SYNC_POSEDGE));
+			vm_flag_to_int(t.flags, DISPLAY_FLAGS_VSYNC_HIGH, DISPLAY_FLAGS_VSYNC_LOW),
+			vm_flag_to_int(t.flags, DISPLAY_FLAGS_HSYNC_HIGH, DISPLAY_FLAGS_HSYNC_LOW),
+			vm_flag_to_int(t.flags, DISPLAY_FLAGS_PIXDATA_POSEDGE, DISPLAY_FLAGS_PIXDATA_NEGEDGE),
+			vm_flag_to_int(t.flags, DISPLAY_FLAGS_DE_HIGH, DISPLAY_FLAGS_DE_LOW),
+			vm_flag_to_int(t.flags, DISPLAY_FLAGS_SYNC_POSEDGE, DISPLAY_FLAGS_SYNC_NEGEDGE));
 
 		DSSDBG("hsync %luHz, vsync %luHz\n", ht, vt);
 	} else {
-- 
2.7.4

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

Re: [PATCHv3 27/30] drm/omap: dispc: improve debug print of display flags

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:13 Tomi Valkeinen wrote:
> Instead of printing 0/1 for display flags like vsync high/low, use a
> tri-state print (-1/0/1) to indicate the "undefined" state.

Even better would be to use high/low/unknown (or H/L/U or any similar set of 
values).

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> b/drivers/gpu/drm/omapdrm/dss/dispc.c index 83241052df6b..2e6a71dbc25d
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -3229,6 +3229,16 @@ static void _dispc_mgr_set_lcd_timings(enum
> omap_channel channel, }
>  }
> 
> +static int vm_flag_to_int(enum display_flags flags, enum display_flags
> high, +	enum display_flags low)
> +{
> +	if (flags & high)
> +		return 1;
> +	if (flags & low)
> +		return -1;
> +	return 0;
> +}
> +
>  /* change name to mode? */
>  static void dispc_mgr_set_timings(enum omap_channel channel,
>  			   const struct videomode *vm)
> @@ -3258,11 +3268,11 @@ static void dispc_mgr_set_timings(enum omap_channel
> channel, t.hsync_len, t.hfront_porch, t.hback_porch,
>  			t.vsync_len, t.vfront_porch, t.vback_porch);
>  		DSSDBG("vsync_level %d hsync_level %d data_pclk_edge %d 
de_level %d
> sync_pclk_edge %d\n", -			!!(t.flags & 
DISPLAY_FLAGS_VSYNC_HIGH),
> -			!!(t.flags & DISPLAY_FLAGS_HSYNC_HIGH),
> -			!!(t.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE),
> -			!!(t.flags & DISPLAY_FLAGS_DE_HIGH),
> -			!!(t.flags & DISPLAY_FLAGS_SYNC_POSEDGE));
> +			vm_flag_to_int(t.flags, DISPLAY_FLAGS_VSYNC_HIGH,
> DISPLAY_FLAGS_VSYNC_LOW), +			vm_flag_to_int(t.flags,
> DISPLAY_FLAGS_HSYNC_HIGH, DISPLAY_FLAGS_HSYNC_LOW),
> +			vm_flag_to_int(t.flags, DISPLAY_FLAGS_PIXDATA_POSEDGE,
> DISPLAY_FLAGS_PIXDATA_NEGEDGE), +			
vm_flag_to_int(t.flags,
> DISPLAY_FLAGS_DE_HIGH, DISPLAY_FLAGS_DE_LOW), +			
vm_flag_to_int(t.flags,
> DISPLAY_FLAGS_SYNC_POSEDGE, DISPLAY_FLAGS_SYNC_NEGEDGE));
> 
>  		DSSDBG("hsync %luHz, vsync %luHz\n", ht, vt);
>  	} else {

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 27/30] drm/omap: dispc: improve debug print of display flags

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 664 bytes --]

On 29/03/17 12:00, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Mar 2017 16:08:13 Tomi Valkeinen wrote:
>> Instead of printing 0/1 for display flags like vsync high/low, use a
>> tri-state print (-1/0/1) to indicate the "undefined" state.
> 
> Even better would be to use high/low/unknown (or H/L/U or any similar set of 
> values).

Hmm I don't know... high/low/unknown would make the debug print much
longer, whereas H/L/U doesn't mean anything unless you check it from the
driver code. Well, probably -1/0/1 is not exactly clear either, but it's
the shortest and clearest option I have in mind.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCHv3 28/30] drm/omap: fix display SYNC/DE flags

[Tomi Valkeinen]

At the moment VSYNC/HSYNC/DE high/low flags set by the panel/encoder
drivers get lost when the videotimings are translated to DRM's
videomode, as DRM's mode does not have corresponding flags.

DRM has bus-flags for this purpose, and while it lacks a few flags at
the moment, it should be used here. However, until we rewrite omapdrm's
device model, using bus-flags is rather difficult.

As a short term fix, this patch makes sure that every time the videomode
is set in omap_crtc_mode_set_nofb(), the driver asks for the SYNC/DE
flags from the panel/encoder drivers, and thus we get the correct flags
into use.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_connector.c |  2 --
 drivers/gpu/drm/omapdrm/omap_crtc.c      | 28 +++++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 50d2b641c28b..d62c035f050f 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -146,8 +146,6 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 	int r, ret = MODE_BAD;
 
 	drm_display_mode_to_videomode(mode, &vm);
-	vm.flags |= DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE |
-		    DISPLAY_FLAGS_SYNC_NEGEDGE;
 	mode->vrefresh = drm_mode_vrefresh(mode);
 
 	/*
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 1db96b077ae8..606ef807741a 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -373,6 +373,11 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
+	struct omap_drm_private *priv = crtc->dev->dev_private;
+	const u32 flags_mask = DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_DE_LOW |
+		DISPLAY_FLAGS_PIXDATA_POSEDGE | DISPLAY_FLAGS_PIXDATA_NEGEDGE |
+		DISPLAY_FLAGS_SYNC_POSEDGE | DISPLAY_FLAGS_SYNC_NEGEDGE;
+	int i;
 
 	DBG("%s: set mode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x",
 	    omap_crtc->name, mode->base.id, mode->name,
@@ -382,9 +387,26 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	    mode->type, mode->flags);
 
 	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
-	omap_crtc->vm.flags |= DISPLAY_FLAGS_DE_HIGH |
-			       DISPLAY_FLAGS_PIXDATA_POSEDGE |
-			       DISPLAY_FLAGS_SYNC_NEGEDGE;
+
+	for (i = 0; i < priv->num_encoders; ++i) {
+		struct drm_encoder *encoder = priv->encoders[i];
+
+		if (encoder->crtc == crtc) {
+			struct omap_dss_device *dssdev;
+
+			dssdev = omap_encoder_get_dssdev(encoder);
+
+			if (dssdev) {
+				struct videomode vm = {0};
+
+				dssdev->driver->get_timings(dssdev, &vm);
+
+				omap_crtc->vm.flags |= vm.flags & flags_mask;
+			}
+
+			break;
+		}
+	}
 }
 
 static int omap_crtc_atomic_check(struct drm_crtc *crtc,
-- 
2.7.4

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

Re: [PATCHv3 28/30] drm/omap: fix display SYNC/DE flags

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:14 Tomi Valkeinen wrote:
> At the moment VSYNC/HSYNC/DE high/low flags set by the panel/encoder
> drivers get lost when the videotimings are translated to DRM's
> videomode, as DRM's mode does not have corresponding flags.
> 
> DRM has bus-flags for this purpose, and while it lacks a few flags at
> the moment, it should be used here. However, until we rewrite omapdrm's
> device model, using bus-flags is rather difficult.
> 
> As a short term fix, this patch makes sure that every time the videomode
> is set in omap_crtc_mode_set_nofb(), the driver asks for the SYNC/DE
> flags from the panel/encoder drivers, and thus we get the correct flags
> into use.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_connector.c |  2 --
>  drivers/gpu/drm/omapdrm/omap_crtc.c      | 28 +++++++++++++++++++++++++---
>  2 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c
> b/drivers/gpu/drm/omapdrm/omap_connector.c index 50d2b641c28b..d62c035f050f
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -146,8 +146,6 @@ static int omap_connector_mode_valid(struct
> drm_connector *connector, int r, ret = MODE_BAD;
> 
>  	drm_display_mode_to_videomode(mode, &vm);
> -	vm.flags |= DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE |
> -		    DISPLAY_FLAGS_SYNC_NEGEDGE;

The dssdrv->get_timings() call below will return a video mode with polarity 
flags set, which will then be compared to vm. As you don't set the vm flags 
anymore, won't the comparison fail ?

>  	mode->vrefresh = drm_mode_vrefresh(mode);
> 
>  	/*
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 1db96b077ae8..606ef807741a
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -373,6 +373,11 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc
> *crtc) {
>  	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>  	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> +	struct omap_drm_private *priv = crtc->dev->dev_private;
> +	const u32 flags_mask = DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_DE_LOW |
> +		DISPLAY_FLAGS_PIXDATA_POSEDGE | DISPLAY_FLAGS_PIXDATA_NEGEDGE 
|
> +		DISPLAY_FLAGS_SYNC_POSEDGE | DISPLAY_FLAGS_SYNC_NEGEDGE;
> +	int i;

i only takes positive values, you can make it an unsigned int.

> 
>  	DBG("%s: set mode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x",
>  	    omap_crtc->name, mode->base.id, mode->name,
> @@ -382,9 +387,26 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc
> *crtc) mode->type, mode->flags);
> 
>  	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
> -	omap_crtc->vm.flags |= DISPLAY_FLAGS_DE_HIGH |
> -			       DISPLAY_FLAGS_PIXDATA_POSEDGE |
> -			       DISPLAY_FLAGS_SYNC_NEGEDGE;
> +

Could you add a HACK/FIXME comment to explain how this should be fixed 
properly ? I want to make sure we won't forget to remove this hack.

> +	for (i = 0; i < priv->num_encoders; ++i) {
> +		struct drm_encoder *encoder = priv->encoders[i];
> +
> +		if (encoder->crtc == crtc) {
> +			struct omap_dss_device *dssdev;
> +
> +			dssdev = omap_encoder_get_dssdev(encoder);
> +
> +			if (dssdev) {
> +				struct videomode vm = {0};
> +
> +				dssdev->driver->get_timings(dssdev, &vm);
> +
> +				omap_crtc->vm.flags |= vm.flags & flags_mask;
> +			}
> +
> +			break;
> +		}
> +	}
>  }
> 
>  static int omap_crtc_atomic_check(struct drm_crtc *crtc,

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 28/30] drm/omap: fix display SYNC/DE flags

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 3907 bytes --]

On 29/03/17 11:58, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Mar 2017 16:08:14 Tomi Valkeinen wrote:
>> At the moment VSYNC/HSYNC/DE high/low flags set by the panel/encoder
>> drivers get lost when the videotimings are translated to DRM's
>> videomode, as DRM's mode does not have corresponding flags.
>>
>> DRM has bus-flags for this purpose, and while it lacks a few flags at
>> the moment, it should be used here. However, until we rewrite omapdrm's
>> device model, using bus-flags is rather difficult.
>>
>> As a short term fix, this patch makes sure that every time the videomode
>> is set in omap_crtc_mode_set_nofb(), the driver asks for the SYNC/DE
>> flags from the panel/encoder drivers, and thus we get the correct flags
>> into use.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/omap_connector.c |  2 --
>>  drivers/gpu/drm/omapdrm/omap_crtc.c      | 28 +++++++++++++++++++++++++---
>>  2 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c
>> b/drivers/gpu/drm/omapdrm/omap_connector.c index 50d2b641c28b..d62c035f050f
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
>> @@ -146,8 +146,6 @@ static int omap_connector_mode_valid(struct
>> drm_connector *connector, int r, ret = MODE_BAD;
>>
>>  	drm_display_mode_to_videomode(mode, &vm);
>> -	vm.flags |= DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE |
>> -		    DISPLAY_FLAGS_SYNC_NEGEDGE;
> 
> The dssdrv->get_timings() call below will return a video mode with polarity 
> flags set, which will then be compared to vm. As you don't set the vm flags 
> anymore, won't the comparison fail ?

Hmm, true. We never go to that code path, though, as we have
check_timings in all the drivers, except panel-dsi-cs.c (but there's a
patch adding it). And the panels don't check the flags, only things like
resolution or porches.

I think we can just require all drivers to have check_timings. It's
hiding the problem a bit, but should work fine until this can be solved
properly.

Or actually, perhaps as a quick fix I should change the check to ignore
the flags for now.

>>  	mode->vrefresh = drm_mode_vrefresh(mode);
>>
>>  	/*
>> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
>> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 1db96b077ae8..606ef807741a
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
>> @@ -373,6 +373,11 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc
>> *crtc) {
>>  	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>>  	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
>> +	struct omap_drm_private *priv = crtc->dev->dev_private;
>> +	const u32 flags_mask = DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_DE_LOW |
>> +		DISPLAY_FLAGS_PIXDATA_POSEDGE | DISPLAY_FLAGS_PIXDATA_NEGEDGE 
> |
>> +		DISPLAY_FLAGS_SYNC_POSEDGE | DISPLAY_FLAGS_SYNC_NEGEDGE;
>> +	int i;
> 
> i only takes positive values, you can make it an unsigned int.

Yep.

>>
>>  	DBG("%s: set mode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x",
>>  	    omap_crtc->name, mode->base.id, mode->name,
>> @@ -382,9 +387,26 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc
>> *crtc) mode->type, mode->flags);
>>
>>  	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
>> -	omap_crtc->vm.flags |= DISPLAY_FLAGS_DE_HIGH |
>> -			       DISPLAY_FLAGS_PIXDATA_POSEDGE |
>> -			       DISPLAY_FLAGS_SYNC_NEGEDGE;
>> +
> 
> Could you add a HACK/FIXME comment to explain how this should be fixed 
> properly ? I want to make sure we won't forget to remove this hack.

I'll add a comment. I don't know exactly how this should be fixed, but
at least I can explain the hack there.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCHv3 29/30] drm/omap: use drm_atomic_helper_shutdown()

[Tomi Valkeinen]

Use drm_atomic_helper_shutdown() to ensure that all crtcs are disabled
when unloading the driver.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Daniel Vetter <daniel@ffwll.ch>

---
 drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index ad8d16cf819c..7b917c0c1a27 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -865,6 +865,8 @@ static int pdev_remove(struct platform_device *pdev)
 	if (priv->fbdev)
 		omap_fbdev_free(ddev);
 
+	drm_atomic_helper_shutdown(ddev);
+
 	drm_mode_config_cleanup(ddev);
 
 	omap_drm_irq_uninstall(ddev);
-- 
2.7.4

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

Re: [PATCHv3 29/30] drm/omap: use drm_atomic_helper_shutdown()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:15 Tomi Valkeinen wrote:
> Use drm_atomic_helper_shutdown() to ensure that all crtcs are disabled
> when unloading the driver.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> 
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index ad8d16cf819c..7b917c0c1a27
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -865,6 +865,8 @@ static int pdev_remove(struct platform_device *pdev)
>  	if (priv->fbdev)
>  		omap_fbdev_free(ddev);
> 
> +	drm_atomic_helper_shutdown(ddev);
> +

Can the hardware still be enabled at this point ? If pdev_remove() is called 
we shouldn't have any application holding the device open (otherwise we'll 
crash miserably), and the fbdev compatibility layer is disabled on the 
previous line.

>  	drm_mode_config_cleanup(ddev);
> 
>  	omap_drm_irq_uninstall(ddev);

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 29/30] drm/omap: use drm_atomic_helper_shutdown()

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1339 bytes --]

On 29/03/17 11:49, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Mar 2017 16:08:15 Tomi Valkeinen wrote:
>> Use drm_atomic_helper_shutdown() to ensure that all crtcs are disabled
>> when unloading the driver.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>
>> ---
>>  drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
>> b/drivers/gpu/drm/omapdrm/omap_drv.c index ad8d16cf819c..7b917c0c1a27
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> @@ -865,6 +865,8 @@ static int pdev_remove(struct platform_device *pdev)
>>  	if (priv->fbdev)
>>  		omap_fbdev_free(ddev);
>>
>> +	drm_atomic_helper_shutdown(ddev);
>> +
> 
> Can the hardware still be enabled at this point ? If pdev_remove() is called 
> we shouldn't have any application holding the device open (otherwise we'll 
> crash miserably), and the fbdev compatibility layer is disabled on the 
> previous line.

The hw is disabled before drm_atomic_helper_shutdown() is called only if
you have fbdev. And fbdev on all the connectors. If that's not the case,
we need drm_atomic_helper_shutdown() to disable the hw.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCHv3 29/30] drm/omap: use drm_atomic_helper_shutdown()

[Laurent Pinchart]

Hi Tomi,

On Wednesday 29 Mar 2017 12:08:19 Tomi Valkeinen wrote:
> On 29/03/17 11:49, Laurent Pinchart wrote:
> > On Tuesday 28 Mar 2017 16:08:15 Tomi Valkeinen wrote:
> >> Use drm_atomic_helper_shutdown() to ensure that all crtcs are disabled
> >> when unloading the driver.
> >> 
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >> Cc: Daniel Vetter <daniel@ffwll.ch>
> >> 
> >> ---
> >> 
> >>  drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> >> b/drivers/gpu/drm/omapdrm/omap_drv.c index ad8d16cf819c..7b917c0c1a27
> >> 100644
> >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> >> @@ -865,6 +865,8 @@ static int pdev_remove(struct platform_device *pdev)
> >> 
> >>  	if (priv->fbdev)
> >>  	
> >>  		omap_fbdev_free(ddev);
> >> 
> >> +	drm_atomic_helper_shutdown(ddev);
> >> +
> > 
> > Can the hardware still be enabled at this point ? If pdev_remove() is
> > called we shouldn't have any application holding the device open
> > (otherwise we'll crash miserably), and the fbdev compatibility layer is
> > disabled on the previous line.
> 
> The hw is disabled before drm_atomic_helper_shutdown() is called only if
> you have fbdev. And fbdev on all the connectors. If that's not the case,
> we need drm_atomic_helper_shutdown() to disable the hw.

But if you have no fbdev, as userspace must not hold the device node open 
here, isn't the device disabled already ?

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 29/30] drm/omap: use drm_atomic_helper_shutdown()

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 501 bytes --]

On 29/03/17 12:11, Laurent Pinchart wrote:

>> The hw is disabled before drm_atomic_helper_shutdown() is called only if
>> you have fbdev. And fbdev on all the connectors. If that's not the case,
>> we need drm_atomic_helper_shutdown() to disable the hw.
> 
> But if you have no fbdev, as userspace must not hold the device node open 
> here, isn't the device disabled already ?

Unfortunately not. See the "drm/atomic: Introduce
drm_atomic_helper_shutdown" thread on dri-devel.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCHv3 30/30] drm/omap: fix crash on module unload

[Tomi Valkeinen]

When unloading omapdrm we get a NULL pointer deref in
omap_drm_irq_uninstall(). This is caused by:

967dd48417874dd25491a4e933648f394a64f70f ("drm: remove
drm_vblank_no_hw_counter assignment from driver code")

We shut down all the crtcs at unload time before calling
omap_drm_irq_uninstall, so the code in omap_drm_irq_uninstall() is
unnecessary and can be removed.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/omapdrm/omap_irq.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index 26a3c06aa14d..115104cdcc59 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -285,25 +285,11 @@ int omap_drm_irq_install(struct drm_device *dev)
 void omap_drm_irq_uninstall(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
-	unsigned long irqflags;
-	int i;
 
 	if (!dev->irq_enabled)
 		return;
 
 	dev->irq_enabled = false;
 
-	/* Wake up any waiters so they don't hang. */
-	if (dev->num_crtcs) {
-		spin_lock_irqsave(&dev->vbl_lock, irqflags);
-		for (i = 0; i < dev->num_crtcs; i++) {
-			wake_up(&dev->vblank[i].queue);
-			dev->vblank[i].enabled = false;
-			dev->vblank[i].last =
-				dev->driver->get_vblank_counter(dev, i);
-		}
-		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
-	}
-
 	priv->dispc_ops->free_irq(dev);
 }
-- 
2.7.4

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

Re: [PATCHv3 30/30] drm/omap: fix crash on module unload

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:16 Tomi Valkeinen wrote:
> When unloading omapdrm we get a NULL pointer deref in
> omap_drm_irq_uninstall(). This is caused by:
> 
> 967dd48417874dd25491a4e933648f394a64f70f ("drm: remove
> drm_vblank_no_hw_counter assignment from driver code")
> 
> We shut down all the crtcs at unload time before calling
> omap_drm_irq_uninstall, so the code in omap_drm_irq_uninstall() is
> unnecessary and can be removed.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/omap_irq.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c
> b/drivers/gpu/drm/omapdrm/omap_irq.c index 26a3c06aa14d..115104cdcc59
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_irq.c
> +++ b/drivers/gpu/drm/omapdrm/omap_irq.c
> @@ -285,25 +285,11 @@ int omap_drm_irq_install(struct drm_device *dev)
>  void omap_drm_irq_uninstall(struct drm_device *dev)
>  {
>  	struct omap_drm_private *priv = dev->dev_private;
> -	unsigned long irqflags;
> -	int i;
> 
>  	if (!dev->irq_enabled)
>  		return;
> 
>  	dev->irq_enabled = false;
> 
> -	/* Wake up any waiters so they don't hang. */
> -	if (dev->num_crtcs) {
> -		spin_lock_irqsave(&dev->vbl_lock, irqflags);
> -		for (i = 0; i < dev->num_crtcs; i++) {
> -			wake_up(&dev->vblank[i].queue);
> -			dev->vblank[i].enabled = false;
> -			dev->vblank[i].last =
> -				dev->driver->get_vblank_counter(dev, i);
> -		}
> -		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> -	}
> -
>  	priv->dispc_ops->free_irq(dev);
>  }

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 00/30] drm/omap: miscallaneous improvements

[Laurent Pinchart]

Hi Tomi,

On Tuesday 28 Mar 2017 16:07:46 Tomi Valkeinen wrote:
> This is the third revision of this series. Note that this series depends on
> "drm/atomic: Introduce drm_atomic_helper_shutdown" which has not yet been
> merged to drm-next.

I've reviewed all patches but the omapdss-base split. While it doesn't look 
bad to me, it's hard to judge whether the code is correctly architectured 
without seeing the omapdss6 driver.

> The main changes in v3:
> 
> - improve variable names in 'work-around for errata i886'
> - drop 'Init fbdev emulation only when we have displays'
> - drop 'Create fbdev emulation only for the first DRM connector'
> - add 'use drm_atomic_helper_shutdown'
> - change 'fix crash on module unload' based on drm_atomic_helper_shutdown
> change.
> 
> The main changes in v2:
> 
> - Interrupt based HPD removed, as there's a race issue which needs to be
> fixed
> - Added patch to fix module unload crash, introduced in drm-next
> - Drop changes affecting userspace APIs
> 
> 
> Hemant Hariyani (1):
>   drm/omap: Add support for render nodes
> 
> Peter Ujfalusi (6):
>   drm/omap: dss: Functions to check components in the display/output
>     list
>   drm/omap: dss: Support for detecting display stack readiness
>   drm/omap: Use omapdss_stack_is_ready() to check that the display stack
>     is up
>   drm/omap: display: Add displays in sorted order to the panel_list
>   drm/omap: poll only connectors where the connect/disconnect can be
>     checked
>   drm/omap: displays: panel-dpi: Support for handling backlight devices
> 
> Tomi Valkeinen (23):
>   drm/omap: work-around for errata i886
>   drm/omap: refactor CRTC HW property setup
>   drm/omap: remove divider constraint from hsdiv
>   drm/omap: decrease min width & height
>   drm/omap: improve DPI clock selection on DRA7xx
>   drm/omap: fix HDMI sync polarities
>   drm/omap: add omapdss-base.ko
>   drm/omap: move dss_initialized to omapdss-base
>   drm/omap: output: use dev_err instead of DSSERR
>   drm/omap: display: don't use dsi_get_pixel_size()
>   drm/omap: move display, dss-of, output to omapdss-base
>   drm/omap: move dispc related dss-feat funcs to dispc
>   drm/omap: add dispc_ops
>   drm/omap: fill dispc_ops
>   drm/omap: use dispc_ops
>   drm/omap: remove all EXPORT_SYMBOLs from dispc.c
>   drm/omap: remove unused dispc_wb_enable & dispc_wb_is_enabled
>   drm/omap: fix replication logic
>   drm/omap: fix plane update warning when crtc is disabled
>   drm/omap: dispc: improve debug print of display flags
>   drm/omap: fix display SYNC/DE flags
>   drm/omap: use drm_atomic_helper_shutdown()
>   drm/omap: fix crash on module unload
> 
>  drivers/gpu/drm/omapdrm/displays/panel-dpi.c |  37 +++++-
>  drivers/gpu/drm/omapdrm/dss/Kconfig          |   4 +
>  drivers/gpu/drm/omapdrm/dss/Makefile         |   8 +-
>  drivers/gpu/drm/omapdrm/dss/base.c           | 140 +++++++++++++++++++++++
>  drivers/gpu/drm/omapdrm/dss/dispc.c          | 165 +++++++++++++++---------
>  drivers/gpu/drm/omapdrm/dss/display.c        |  36 +++++-
>  drivers/gpu/drm/omapdrm/dss/dpi.c            |  55 ++++++---
>  drivers/gpu/drm/omapdrm/dss/dsi.c            |   2 +-
>  drivers/gpu/drm/omapdrm/dss/dss-of.c         |   3 +-
>  drivers/gpu/drm/omapdrm/dss/dss.c            |  13 +--
>  drivers/gpu/drm/omapdrm/dss/dss.h            |  17 +--
>  drivers/gpu/drm/omapdrm/dss/dss_features.c   |   3 -
>  drivers/gpu/drm/omapdrm/dss/dss_features.h   |   4 +
>  drivers/gpu/drm/omapdrm/dss/hdmi_wp.c        |  12 +-
>  drivers/gpu/drm/omapdrm/dss/omapdss.h        |  95 +++++++++------
>  drivers/gpu/drm/omapdrm/dss/output.c         |  27 ++++-
>  drivers/gpu/drm/omapdrm/dss/pll.c            |  17 +--
>  drivers/gpu/drm/omapdrm/omap_connector.c     |  12 +-
>  drivers/gpu/drm/omapdrm/omap_crtc.c          |  87 ++++++++++----
>  drivers/gpu/drm/omapdrm/omap_drv.c           |  45 +++++---
>  drivers/gpu/drm/omapdrm/omap_drv.h           |   2 +
>  drivers/gpu/drm/omapdrm/omap_irq.c           |  47 +++-----
>  drivers/gpu/drm/omapdrm/omap_plane.c         |  20 +++-
>  23 files changed, 588 insertions(+), 263 deletions(-)
>  create mode 100644 drivers/gpu/drm/omapdrm/dss/base.c

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv3 00/30] drm/omap: miscallaneous improvements

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1205 bytes --]

On 29/03/17 15:09, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Tuesday 28 Mar 2017 16:07:46 Tomi Valkeinen wrote:
>> This is the third revision of this series. Note that this series depends on
>> "drm/atomic: Introduce drm_atomic_helper_shutdown" which has not yet been
>> merged to drm-next.
> 
> I've reviewed all patches but the omapdss-base split. While it doesn't look 
> bad to me, it's hard to judge whether the code is correctly architectured 
> without seeing the omapdss6 driver.

Thanks. I think time will tell if it's correctly done, but I'm quite
confident that it can't mess up much.

It consists really of two parts: separating the common base stuff into a
separate module, and the dispc_ops. While the common module could use
some cleanup, I think it makes sense. The dispc_ops really just change
from calling functions directly to use function pointers.

You can find dss6 driver from

https://git.ti.com/ti-linux-kernel/ti-linux-kernel/trees/ti-linux-4.9.y/drivers/gpu/drm/omapdrm/dss

The dss6 and dispc6 files. But it's really just a smaller, cleaner
(well, the driver is not clean yet) version of dss5. Conceptually the
same, different registers.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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