Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH V4 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Tomi Valkeinen @ 2012-08-20  8:42 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345211692.3158.160.camel@deskari>

[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]

On Fri, 2012-08-17 at 16:54 +0300, Tomi Valkeinen wrote:
> On Thu, 2012-08-16 at 16:48 +0530, Chandrabhanu Mahapatra wrote:
> > All the cpu_is checks have been moved to dss_init_features function providing a
> > much more generic and cleaner interface. The OMAP version and revision specific
> > initializations in various functions are cleaned and the necessary data are
> > moved to dss_features structure which is local to dss.c.
> > 
> > Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> 
> > +static int __init dss_init_features(struct device *dev)
> > +{
> > +	dss.feat = devm_kzalloc(dev, sizeof(*dss.feat), GFP_KERNEL);
> > +	if (!dss.feat) {
> > +		dev_err(dev, "Failed to allocate local DSS Features\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	if (cpu_is_omap24xx())
> > +		dss.feat = &omap24xx_dss_features;
> > +	else if (cpu_is_omap34xx())
> > +		dss.feat = &omap34xx_dss_features;
> > +	else if (cpu_is_omap3630())
> > +		dss.feat = &omap3630_dss_features;
> > +	else if (cpu_is_omap44xx())
> > +		dss.feat = &omap44xx_dss_features;
> > +	else
> > +		return -ENODEV;
> > +
> > +	return 0;
> > +}
> 
> This is not correct (and same problem in dispc). You allocate the feat
> struct and assign the pointer to dss.feat, but then overwrite dss.feat
> pointer with the pointer to omap24xx_dss_features (which is freed
> later). You need to memcpy it.
> 
> I also get a crash on omap3 overo board when loading omapdss:

The crash happens because dss_get_clocks uses the feat stuff, but the
dss_init_features is only called later. Did you test this? I can't see
how that can work on any board.

Also, in the current place you have dss_init_features call, in case
there's an error you can't just return, you need to release the
resources. The same problem is in the dispc.c.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 01/10] video: exynos_dp: Change aux transaction failures
From: Jingoo Han @ 2012-08-20  9:02 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-2-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> 
> This patch adds the function name to aux transaction failure messages
> so we can tell which transaction is failing. It also changes the level
> of Aux Transaction fail messages from error to debug. We retry the
> transactions a few times and will report errors if warranted outside of
> this function.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
> ---


Acked-by: Jingoo Han <jg1.han@samsung.com>

It looks good.



>  drivers/video/exynos/exynos_dp_reg.c |   21 ++++++++++++++-------
>  1 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
> index ce401c8..a121bed 100644
> --- a/drivers/video/exynos/exynos_dp_reg.c
> +++ b/drivers/video/exynos/exynos_dp_reg.c
> @@ -471,7 +471,8 @@ int exynos_dp_write_byte_to_dpcd(struct exynos_dp_device *dp,
>  		if (retval = 0)
>  			break;
>  		else
> -			dev_err(dp->dev, "Aux Transaction fail!\n");
> +			dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> +				__func__);
>  	}
> 
>  	return retval;
> @@ -511,7 +512,8 @@ int exynos_dp_read_byte_from_dpcd(struct exynos_dp_device *dp,
>  		if (retval = 0)
>  			break;
>  		else
> -			dev_err(dp->dev, "Aux Transaction fail!\n");
> +			dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> +				__func__);
>  	}
> 
>  	/* Read data buffer */
> @@ -575,7 +577,8 @@ int exynos_dp_write_bytes_to_dpcd(struct exynos_dp_device *dp,
>  			if (retval = 0)
>  				break;
>  			else
> -				dev_err(dp->dev, "Aux Transaction fail!\n");
> +				dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> +					__func__);
>  		}
> 
>  		start_offset += cur_data_count;
> @@ -632,7 +635,8 @@ int exynos_dp_read_bytes_from_dpcd(struct exynos_dp_device *dp,
>  			if (retval = 0)
>  				break;
>  			else
> -				dev_err(dp->dev, "Aux Transaction fail!\n");
> +				dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> +					__func__);
>  		}
> 
>  		for (cur_data_idx = 0; cur_data_idx < cur_data_count;
> @@ -677,7 +681,7 @@ int exynos_dp_select_i2c_device(struct exynos_dp_device *dp,
>  	/* Start AUX transaction */
>  	retval = exynos_dp_start_aux_transaction(dp);
>  	if (retval != 0)
> -		dev_err(dp->dev, "Aux Transaction fail!\n");
> +		dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
> 
>  	return retval;
>  }
> @@ -717,7 +721,8 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
>  		if (retval = 0)
>  			break;
>  		else
> -			dev_err(dp->dev, "Aux Transaction fail!\n");
> +			dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> +				__func__);
>  	}
> 
>  	/* Read data */
> @@ -777,7 +782,9 @@ int exynos_dp_read_bytes_from_i2c(struct exynos_dp_device *dp,
>  				if (retval = 0)
>  					break;
>  				else
> -					dev_err(dp->dev, "Aux Transaction fail!\n");
> +					dev_dbg(dp->dev,
> +						"%s: Aux Transaction fail!\n",
> +						__func__);
>  			}
>  			/* Check if Rx sends defer */
>  			reg = readl(dp->reg_base + EXYNOS_DP_AUX_RX_COMM);
> --
> 1.7.7.3


^ permalink raw reply

* Re: [PATCH 02/10] video: exynos_dp: Check DPCD return codes
From: Jingoo Han @ 2012-08-20  9:04 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-3-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:

> 
> Add return code checks to the DPCD transactions in the SW link training.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Mandeep Singh Baines <msb@chromium.org>
> ---
>  drivers/video/exynos/exynos_dp_core.c |   65 ++++++++++++++++++++++-----------
>  1 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index c6c016a..1836e33 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -258,8 +258,9 @@ static void exynos_dp_set_lane_lane_pre_emphasis(struct exynos_dp_device *dp,
>  	}
>  }
> 
> -static void exynos_dp_link_start(struct exynos_dp_device *dp)
> +static int exynos_dp_link_start(struct exynos_dp_device *dp)
>  {
> +	int ret;
>  	u8 buf[5];
>  	int lane;
>  	int lane_count;
> @@ -273,8 +274,10 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
>  		dp->link_train.cr_loop[lane] = 0;
> 
>  	/* Set sink to D0 (Sink Not Ready) mode. */
> -	exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
> +	ret = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
>  				DPCD_SET_POWER_STATE_D0);
> +	if (ret)
> +		return ret;

Could you use 'retval' instead of 'ret'?



> 
>  	/* Set link rate and count as you want to establish*/
>  	exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
> @@ -283,8 +286,10 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
>  	/* Setup RX configuration */
>  	buf[0] = dp->link_train.link_rate;
>  	buf[1] = dp->link_train.lane_count;
> -	exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_LINK_BW_SET,
> +	ret = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_LINK_BW_SET,
>  				2, buf);
> +	if (ret)
> +		return ret;
> 
>  	/* Set TX pre-emphasis to minimum */
>  	for (lane = 0; lane < lane_count; lane++)
> @@ -303,9 +308,13 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
>  	for (lane = 0; lane < lane_count; lane++)
>  		buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
>  			    DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
> -	exynos_dp_write_bytes_to_dpcd(dp,
> +	ret = exynos_dp_write_bytes_to_dpcd(dp,
>  		DPCD_ADDR_TRAINING_LANE0_SET,
>  		lane_count, buf);
> +	if (ret)
> +		return ret;
> +
> +	return ret;
>  }
> 
>  static unsigned char exynos_dp_get_lane_status(u8 link_status[6], int lane)
> @@ -475,6 +484,7 @@ static int exynos_dp_check_max_cr_loop(struct exynos_dp_device *dp,
> 
>  static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>  {
> +	int ret;
>  	u8 data;
>  	u8 link_status[6];
>  	int lane;
> @@ -488,8 +498,11 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
> 
>  	usleep_range(100, 101);
> 
> -	exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
> +	ret = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
>  				6, link_status);
> +	if (ret)
> +		return ret;
> +
>  	lane_count = dp->link_train.lane_count;
> 
>  	if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
> @@ -503,18 +516,22 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
> 
>  		buf[0] = DPCD_SCRAMBLING_DISABLED |
>  			 DPCD_TRAINING_PATTERN_2;
> -		exynos_dp_write_byte_to_dpcd(dp,
> +		ret = exynos_dp_write_byte_to_dpcd(dp,
>  			DPCD_ADDR_TRAINING_PATTERN_SET,
>  			buf[0]);
> +		if (ret)
> +			return ret;
> 
>  		for (lane = 0; lane < lane_count; lane++) {
>  			exynos_dp_set_lane_link_training(dp,
>  				dp->link_train.training_lane[lane],
>  				lane);
>  			buf[lane] = dp->link_train.training_lane[lane];
> -			exynos_dp_write_byte_to_dpcd(dp,
> +			ret = exynos_dp_write_byte_to_dpcd(dp,
>  				DPCD_ADDR_TRAINING_LANE0_SET + lane,
>  				buf[lane]);
> +			if (ret)
> +				return ret;
>  		}
>  		dp->link_train.lt_state = EQUALIZER_TRAINING;
>  	} else {
> @@ -551,18 +568,21 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>  					dp->link_train.training_lane[lane],
>  					lane);
>  				buf[lane] = dp->link_train.training_lane[lane];
> -				exynos_dp_write_byte_to_dpcd(dp,
> +				ret = exynos_dp_write_byte_to_dpcd(dp,
>  					DPCD_ADDR_TRAINING_LANE0_SET + lane,
>  					buf[lane]);
> +				if (ret)
> +					return ret;
>  			}
>  		}
>  	}
> 
> -	return 0;
> +	return ret;
>  }
> 
>  static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>  {
> +	int ret;
>  	u8 link_status[6];
>  	int lane;
>  	int lane_count;
> @@ -573,8 +593,10 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
> 
>  	usleep_range(400, 401);
> 
> -	exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
> +	ret = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
>  				6, link_status);
> +	if (ret)
> +		return ret;
>  	lane_count = dp->link_train.lane_count;
> 
>  	if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
> @@ -614,9 +636,11 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>  						dp->link_train.training_lane[lane],
>  						lane);
>  					buf[lane] = dp->link_train.training_lane[lane];
> -					exynos_dp_write_byte_to_dpcd(dp,
> +					ret = exynos_dp_write_byte_to_dpcd(dp,
>  						DPCD_ADDR_TRAINING_LANE0_SET + lane,
>  						buf[lane]);
> +					if (ret)
> +						return ret;
>  				}
>  			}
>  		}
> @@ -624,7 +648,7 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>  		exynos_dp_reduce_link_rate(dp);
>  	}
> 
> -	return 0;
> +	return ret;
>  }
> 
>  static void exynos_dp_get_max_rx_bandwidth(struct exynos_dp_device *dp,
> @@ -692,28 +716,25 @@ static void exynos_dp_init_training(struct exynos_dp_device *dp,
> 
>  static int exynos_dp_sw_link_training(struct exynos_dp_device *dp)
>  {
> -	int retval = 0;
> -	int training_finished;
> +	int ret = 0, training_finished = 0;
> 
>  	/* Turn off unnecessary lane */
>  	if (dp->link_train.lane_count = 1)
>  		exynos_dp_set_analog_power_down(dp, CH1_BLOCK, 1);
> 
> -	training_finished = 0;
> -
>  	dp->link_train.lt_state = START;
> 
>  	/* Process here */
> -	while (!training_finished) {
> +	while (!ret && !training_finished) {
>  		switch (dp->link_train.lt_state) {
>  		case START:
> -			exynos_dp_link_start(dp);
> +			ret = exynos_dp_link_start(dp);
>  			break;
>  		case CLOCK_RECOVERY:
> -			exynos_dp_process_clock_recovery(dp);
> +			ret = exynos_dp_process_clock_recovery(dp);
>  			break;
>  		case EQUALIZER_TRAINING:
> -			exynos_dp_process_equalizer_training(dp);
> +			ret = exynos_dp_process_equalizer_training(dp);
>  			break;
>  		case FINISHED:
>  			training_finished = 1;
> @@ -722,8 +743,10 @@ static int exynos_dp_sw_link_training(struct exynos_dp_device *dp)
>  			return -EREMOTEIO;
>  		}
>  	}
> +	if (ret)
> +		dev_err(dp->dev, "eDP link training failed (%d)\n", ret);
> 
> -	return retval;
> +	return ret;
>  }
> 
>  static int exynos_dp_set_link_train(struct exynos_dp_device *dp,
> --
> 1.7.7.3


^ permalink raw reply

* RE: [PATCH 04/10] video: exynos_dp: Get pll lock before pattern set
From: Jingoo Han @ 2012-08-20  9:13 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-5-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> 
> According to the exynos datasheet (Figure 49-10), we should wait for PLL
> lock before programming the training pattern when doing software eDP
> link training.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Mandeep Singh Baines <msb@chromium.org>
> ---
>  drivers/video/exynos/exynos_dp_core.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 3deded2..207bd7e 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -23,6 +23,8 @@
> 
>  #include "exynos_dp_core.h"
> 
> +#define PLL_MAX_TRIES 100
> +
>  static int exynos_dp_init_dp(struct exynos_dp_device *dp)
>  {
>  	exynos_dp_reset(dp);
> @@ -260,7 +262,7 @@ static void exynos_dp_set_lane_lane_pre_emphasis(struct exynos_dp_device *dp,
> 
>  static int exynos_dp_link_start(struct exynos_dp_device *dp)
>  {
> -	int ret, lane, lane_count;
> +	int ret, lane, lane_count, pll_tries;
>  	u8 buf[4];
> 
>  	lane_count = dp->link_train.lane_count;
> @@ -293,6 +295,16 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>  		exynos_dp_set_lane_lane_pre_emphasis(dp,
>  			PRE_EMPHASIS_LEVEL_0, lane);
> 
> +	/* Wait for PLL lock */
> +	pll_tries = 0;
> +	while (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED) {
> +		if (pll_tries = PLL_MAX_TRIES)
> +			return -ETIMEDOUT;
> +
> +		pll_tries++;
> +		udelay(100);
> +	}
> +

You don't need to add 'PLL_MAX_TRIES', please use 'DP_TIMEOUT_LOOP_COUNT' as below:

	/* Wait for PLL lock */
	while (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED) {
		timeout_loop++;
		if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
			dev_err(dp->dev, "failed to get pll lock status\n");
			return -ETIMEDOUT;
		}
		udelay(100);
	}

Also, could you use usleep_range() instead of udelay()? 


>  	/* Set training pattern 1 */
>  	exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
> 
> --
> 1.7.7.3


^ permalink raw reply

* Re: [PATCH 05/10] video: exynos_dp: Remove sink control to D0
From: Jingoo Han @ 2012-08-20  9:19 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-6-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> 
> Don't reset the sink power to D0. Removing this for three reasons:
> 
> 1) It's not required in the SW link training documentation
OK.
> 2) The comment is incorrect, D0 is normal operation, not "power down"
OK. As you mentioned, D0 is not 'power down', 'normal operation'.
However, if comment is wrong, usually we fix comment, not remove it. :)

> 3) It seems to change things in the link training that causes glitches

Um, it seems that this problem depends on LCD panel.
Other LCDs that I have tested do not have this kind of problem.
Please, modify this comment.

> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/video/exynos/exynos_dp_core.c |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 207bd7e..1c998d9 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -273,12 +273,6 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>  	for (lane = 0; lane < lane_count; lane++)
>  		dp->link_train.cr_loop[lane] = 0;
> 
> -	/* Set sink to D0 (Sink Not Ready) mode. */
> -	ret = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
> -				DPCD_SET_POWER_STATE_D0);
> -	if (ret)
> -		return ret;
> -
>  	/* Set link rate and count as you want to establish*/
>  	exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
>  	exynos_dp_set_lane_count(dp, dp->link_train.lane_count);
> --
> 1.7.7.3


^ permalink raw reply

* Re: [PATCH 06/10] video: exynos_dp: Fix get_pll_lock_status return value
From: Jingoo Han @ 2012-08-20  9:20 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-7-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> 
> Fix the return value of exynos_dp_get_pll_lock_status to
> reflect what it actually returns.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>


Acked-by: Jingoo Han <jg1.han@samsung.com>


> ---
>  drivers/video/exynos/exynos_dp_core.h |    2 +-
>  drivers/video/exynos/exynos_dp_reg.c  |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> index 8526e54..6431c65 100644
> --- a/drivers/video/exynos/exynos_dp_core.h
> +++ b/drivers/video/exynos/exynos_dp_core.h
> @@ -43,7 +43,7 @@ void exynos_dp_init_interrupt(struct exynos_dp_device *dp);
>  void exynos_dp_reset(struct exynos_dp_device *dp);
>  void exynos_dp_swreset(struct exynos_dp_device *dp);
>  void exynos_dp_config_interrupt(struct exynos_dp_device *dp);
> -u32 exynos_dp_get_pll_lock_status(struct exynos_dp_device *dp);
> +enum pll_status exynos_dp_get_pll_lock_status(struct exynos_dp_device *dp);
>  void exynos_dp_set_pll_power_down(struct exynos_dp_device *dp, bool enable);
>  void exynos_dp_set_analog_power_down(struct exynos_dp_device *dp,
>  				enum analog_power_block block,
> diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
> index a121bed..d7b1494 100644
> --- a/drivers/video/exynos/exynos_dp_reg.c
> +++ b/drivers/video/exynos/exynos_dp_reg.c
> @@ -179,7 +179,7 @@ void exynos_dp_config_interrupt(struct exynos_dp_device *dp)
>  	writel(reg, dp->reg_base + EXYNOS_DP_INT_STA_MASK);
>  }
> 
> -u32 exynos_dp_get_pll_lock_status(struct exynos_dp_device *dp)
> +enum pll_status exynos_dp_get_pll_lock_status(struct exynos_dp_device *dp)
>  {
>  	u32 reg;
> 
> --
> 1.7.7.3


^ permalink raw reply

* Re: [PATCH 07/10] video: exynos_dp: Improve EDID error handling
From: Jingoo Han @ 2012-08-20  9:22 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-8-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> 
> EDID error handling has 2 problems:
>  - It doesn't fail as early as it can
>  - The retry counts for i2c and aux transactions are huge
> 
> This patch fails if the initial i2c transaction fails, and reduces the
> aux and i2c retry counts down to 3.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> ---
>  drivers/video/exynos/exynos_dp_core.c |   18 ++++++++++--------
>  drivers/video/exynos/exynos_dp_reg.c  |    9 ++++-----
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 1c998d9..2882362 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -89,9 +89,11 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
>  	 */
> 
>  	/* Read Extension Flag, Number of 128-byte EDID extension blocks */
> -	exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> +	retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
>  				EDID_EXTENSION_FLAG,
>  				&extend_block);
> +	if (retval)
> +		return retval;
> 
>  	if (extend_block > 0) {
>  		dev_dbg(dp->dev, "EDID data includes a single extension!\n");
> @@ -177,21 +179,21 @@ static int exynos_dp_handle_edid(struct exynos_dp_device *dp)
>  {
>  	u8 buf[12];
>  	int i;
> -	int retval;
> +	int ret;
> 
>  	/* Read DPCD DPCD_ADDR_DPCD_REV~RECEIVE_PORT1_CAP_1 */
> -	exynos_dp_read_bytes_from_dpcd(dp,
> -		DPCD_ADDR_DPCD_REV,
> -		12, buf);
> +	ret = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_DPCD_REV, 12, buf);
> +	if (ret)
> +		return ret;

Could you use 'retval' instead of 'ret'?

> 
>  	/* Read EDID */
>  	for (i = 0; i < 3; i++) {
> -		retval = exynos_dp_read_edid(dp);
> -		if (retval = 0)
> +		ret = exynos_dp_read_edid(dp);
> +		if (!ret)
>  			break;
>  	}
> 
> -	return retval;
> +	return ret;
>  }
> 
>  static void exynos_dp_enable_rx_to_enhanced_mode(struct exynos_dp_device *dp,
> diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
> index d7b1494..389e0f0 100644
> --- a/drivers/video/exynos/exynos_dp_reg.c
> +++ b/drivers/video/exynos/exynos_dp_reg.c
> @@ -547,7 +547,7 @@ int exynos_dp_write_bytes_to_dpcd(struct exynos_dp_device *dp,
>  		else
>  			cur_data_count = count - start_offset;
> 
> -		for (i = 0; i < 10; i++) {
> +		for (i = 0; i < 3; i++) {
>  			/* Select DPCD device address */
>  			reg = AUX_ADDR_7_0(reg_addr + start_offset);
>  			writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
> @@ -612,7 +612,7 @@ int exynos_dp_read_bytes_from_dpcd(struct exynos_dp_device *dp,
>  			cur_data_count = count - start_offset;
> 
>  		/* AUX CH Request Transaction process */
> -		for (i = 0; i < 10; i++) {
> +		for (i = 0; i < 3; i++) {
>  			/* Select DPCD device address */
>  			reg = AUX_ADDR_7_0(reg_addr + start_offset);
>  			writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
> @@ -695,7 +695,7 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
>  	int i;
>  	int retval;
> 
> -	for (i = 0; i < 10; i++) {
> +	for (i = 0; i < 3; i++) {
>  		/* Clear AUX CH data buffer */
>  		reg = BUF_CLR;
>  		writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
> @@ -703,7 +703,6 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
>  		/* Select EDID device */
>  		retval = exynos_dp_select_i2c_device(dp, device_addr, reg_addr);
>  		if (retval != 0) {
> -			dev_err(dp->dev, "Select EDID device fail!\n");
>  			continue;
>  		}
> 
> @@ -745,7 +744,7 @@ int exynos_dp_read_bytes_from_i2c(struct exynos_dp_device *dp,
>  	int retval = 0;
> 
>  	for (i = 0; i < count; i += 16) {
> -		for (j = 0; j < 100; j++) {
> +		for (j = 0; j < 3; j++) {
>  			/* Clear AUX CH data buffer */
>  			reg = BUF_CLR;
>  			writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
> --
> 1.7.7.3


^ permalink raw reply

* Re: [PATCH 09/10] video: exynos_dp: Fix bug when checking dp->irq
From: Jingoo Han @ 2012-08-20  9:24 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-10-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:

> -----Original Message-----
> From: Sean Paul [mailto:seanpaul@chromium.org]
> Sent: Wednesday, August 08, 2012 12:54 PM
> To: jg1.han@samsung.com; linux-fbdev@vger.kernel.org
> Cc: Sean Paul
> Subject: [PATCH 09/10] video: exynos_dp: Fix bug when checking dp->irq
> 
> Fix a bug where we check !dp->irq instead of the correct check for
> -ENXIO.
> 
> BUG=None
> TEST=None
> 
> Change-Id: Ibcdd16fb71eb41e4380ac8911ad6063634ff80e2

Remove unnecessary commit message such as 'Change-Id',
'BUG', and 'TEST'.


> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-on: https://gerrit.chromium.org/gerrit/29144
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> ---
>  drivers/video/exynos/exynos_dp_core.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 68ad494..eb5816f 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -919,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  	}
> 
>  	dp->irq = platform_get_irq(pdev, 0);
> -	if (!dp->irq) {
> +	if (dp->irq = -ENXIO) {
>  		dev_err(&pdev->dev, "failed to get irq\n");
>  		ret = -ENODEV;
>  		goto err_clock;
> --
> 1.7.7.3


^ permalink raw reply

* Re: [PATCH 10/10] video: exynos_dp: Enable hotplug interrupts
From: Jingoo Han @ 2012-08-20  9:29 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-11-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> 
> Enable hotplug interrupts and move the hotplug scheduling into the
> interrupt handler. This allows us to introduce a screen at any time
> while we're running.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> ---
>  drivers/video/exynos/exynos_dp_core.c |   40 +++++++++++++++++++++++++-------
>  drivers/video/exynos/exynos_dp_core.h |    9 +++++++
>  drivers/video/exynos/exynos_dp_reg.c  |   35 ++++++++++++++++++++++++++++-
>  3 files changed, 74 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index eb5816f..5279027 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -47,10 +47,6 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
>  {
>  	int timeout_loop = 0;
> 
> -	exynos_dp_init_hpd(dp);
> -
> -	usleep_range(200, 210);
> -
>  	while (exynos_dp_get_plug_in_status(dp) != 0) {
>  		timeout_loop++;
>  		if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> @@ -829,8 +825,32 @@ static void exynos_dp_enable_scramble(struct exynos_dp_device *dp, bool enable)
>  static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
>  {
>  	struct exynos_dp_device *dp = arg;
> -
> -	dev_err(dp->dev, "exynos_dp_irq_handler\n");
> +	enum dp_irq_type irq_type;
> +
> +	irq_type = exynos_dp_get_irq_type(dp);
> +	switch (irq_type) {
> +	case DP_IRQ_TYPE_HP_CABLE_IN:
> +		dev_dbg(dp->dev, "Received irq - cable in\n");
> +		schedule_work(&dp->hotplug_work);
> +		exynos_dp_clear_hotplug_interrupts(dp);
> +		break;
> +	case DP_IRQ_TYPE_HP_CABLE_OUT:
> +		dev_dbg(dp->dev, "Received irq - cable out\n");
> +		exynos_dp_clear_hotplug_interrupts(dp);
> +		break;
> +	case DP_IRQ_TYPE_HP_CHANGE:
> +		/*
> +		 * We get these change notifications once in a while, but there
> +		 * is nothing we can do with them. Just ignore it for now and
> +		 * only handle cable changes.
> +		 */
> +		dev_dbg(dp->dev, "Received irq - hotplug change; ignoring.\n");
> +		exynos_dp_clear_hotplug_interrupts(dp);
> +		break;
> +	default:
> +		dev_err(dp->dev, "Received irq - unknown type!\n");
> +		break;
> +	}
>  	return IRQ_HANDLED;
>  }
> 
> @@ -843,7 +863,7 @@ static void exynos_dp_hotplug(struct work_struct *work)
> 
>  	ret = exynos_dp_detect_hpd(dp);
>  	if (ret) {
> -		dev_err(dp->dev, "unable to detect hpd\n");
> +		/* Cable has been disconnected, we're done */
>  		return;
>  	}
> 
> @@ -941,7 +961,6 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  	exynos_dp_init_dp(dp);
> 
>  	platform_set_drvdata(pdev, dp);
> -	schedule_work(&dp->hotplug_work);
> 
>  	return 0;
> 
> @@ -975,6 +994,8 @@ static int exynos_dp_suspend(struct device *dev)
>  	struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
>  	struct exynos_dp_device *dp = platform_get_drvdata(pdev);
> 
> +	disable_irq(dp->irq);
> +
>  	if (work_pending(&dp->hotplug_work))
>  		flush_work_sync(&dp->hotplug_work);
> 
> @@ -998,7 +1019,8 @@ static int exynos_dp_resume(struct device *dev)
>  	clk_enable(dp->clock);
> 
>  	exynos_dp_init_dp(dp);
> -	schedule_work(&dp->hotplug_work);
> +
> +	enable_irq(dp->irq);
> 
>  	return 0;
>  }
> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> index cf1010b..a5795dd 100644
> --- a/drivers/video/exynos/exynos_dp_core.h
> +++ b/drivers/video/exynos/exynos_dp_core.h
> @@ -13,6 +13,13 @@
>  #ifndef _EXYNOS_DP_CORE_H
>  #define _EXYNOS_DP_CORE_H
> 
> +enum dp_irq_type {
> +	DP_IRQ_TYPE_HP_CABLE_IN,
> +	DP_IRQ_TYPE_HP_CABLE_OUT,
> +	DP_IRQ_TYPE_HP_CHANGE,
> +	DP_IRQ_TYPE_UNKNOWN,
> +};
> +
>  struct link_train {
>  	int eq_loop;
>  	int cr_loop[4];
> @@ -51,6 +58,8 @@ void exynos_dp_set_analog_power_down(struct exynos_dp_device *dp,
>  				bool enable);
>  void exynos_dp_init_analog_func(struct exynos_dp_device *dp);
>  void exynos_dp_init_hpd(struct exynos_dp_device *dp);
> +enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp);
> +void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp);
>  void exynos_dp_reset_aux(struct exynos_dp_device *dp);
>  void exynos_dp_init_aux(struct exynos_dp_device *dp);
>  int exynos_dp_get_plug_in_status(struct exynos_dp_device *dp);
> diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
> index 389e0f0..5906fc7 100644
> --- a/drivers/video/exynos/exynos_dp_reg.c
> +++ b/drivers/video/exynos/exynos_dp_reg.c
> @@ -327,7 +327,7 @@ void exynos_dp_init_analog_func(struct exynos_dp_device *dp)
>  	writel(reg, dp->reg_base + EXYNOS_DP_FUNC_EN_2);
>  }
> 
> -void exynos_dp_init_hpd(struct exynos_dp_device *dp)
> +void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp)
>  {
>  	u32 reg;
> 
> @@ -336,10 +336,43 @@ void exynos_dp_init_hpd(struct exynos_dp_device *dp)
> 
>  	reg = INT_HPD;
>  	writel(reg, dp->reg_base + EXYNOS_DP_INT_STA);
> +}
> +
> +void exynos_dp_init_hpd(struct exynos_dp_device *dp)
> +{
> +	u32 reg;
> +
> +	exynos_dp_clear_hotplug_interrupts(dp);
> 
>  	reg = readl(dp->reg_base + EXYNOS_DP_SYS_CTL_3);
>  	reg &= ~(F_HPD | HPD_CTRL);
>  	writel(reg, dp->reg_base + EXYNOS_DP_SYS_CTL_3);
> +
> +	/* Unmask hotplug interrupts */
> +	reg = HOTPLUG_CHG | HPD_LOST | PLUG;
> +	writel(reg, dp->reg_base + EXYNOS_DP_COMMON_INT_MASK_4);


Please don't handle mask bits in init_hpd.

This masking bits are easily changed by modifying definition of masking bits
as below:

- #define COMMON_INT_MASK_4 (0)
+ #define COMMON_INT_MASK_4 (HOTPLUG_CHG | HPD_LOST | PLUG)

Then, it can be used in exynos_dp_config_interrupt().


> +
> +	reg = INT_HPD;
> +	writel(reg, dp->reg_base + EXYNOS_DP_INT_STA_MASK);
> +}
> +
> +enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp)
> +{
> +	u32 reg;
> +
> +	/* Parse hotplug interrupt status register */
> +	reg = readl(dp->reg_base + EXYNOS_DP_COMMON_INT_STA_4);
> +
> +	if (reg & PLUG)
> +		return DP_IRQ_TYPE_HP_CABLE_IN;
> +
> +	if (reg & HPD_LOST)
> +		return DP_IRQ_TYPE_HP_CABLE_OUT;
> +
> +	if (reg & HOTPLUG_CHG)
> +		return DP_IRQ_TYPE_HP_CHANGE;
> +
> +	return DP_IRQ_TYPE_UNKNOWN;
>  }
> 
>  void exynos_dp_reset_aux(struct exynos_dp_device *dp)
> --
> 1.7.7.3


^ permalink raw reply

* Re: [PATCH 03/10] video: exynos_dp: Clean up SW link training
From: Jingoo Han @ 2012-08-20  9:35 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1344398064-13563-4-git-send-email-seanpaul@chromium.org>

On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> 
> Clean up some of the SW training code to make it more clear and reduce
> duplicate code.


Clean up code of the SW training code is not necessary.
I cleaned the SW training code, at the patch that I sent as below.

    http://www.spinics.net/lists/linux-fbdev/msg06849.html


> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Mandeep Singh Baines <msb@chromium.org>
> ---
>  drivers/video/exynos/exynos_dp_core.c |  180 ++++++++++++---------------------
>  1 files changed, 67 insertions(+), 113 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 1836e33..3deded2 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -260,10 +260,8 @@ static void exynos_dp_set_lane_lane_pre_emphasis(struct exynos_dp_device *dp,
> 
>  static int exynos_dp_link_start(struct exynos_dp_device *dp)
>  {
> -	int ret;
> -	u8 buf[5];
> -	int lane;
> -	int lane_count;
> +	int ret, lane, lane_count;
> +	u8 buf[4];
> 
>  	lane_count = dp->link_train.lane_count;
> 
> @@ -286,8 +284,7 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>  	/* Setup RX configuration */
>  	buf[0] = dp->link_train.link_rate;
>  	buf[1] = dp->link_train.lane_count;
> -	ret = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_LINK_BW_SET,
> -				2, buf);
> +	ret = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_LINK_BW_SET, 2, buf);
>  	if (ret)
>  		return ret;
> 
> @@ -300,16 +297,15 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>  	exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
> 
>  	/* Set RX training pattern */
> -	buf[0] = DPCD_SCRAMBLING_DISABLED |
> -		 DPCD_TRAINING_PATTERN_1;
> -	exynos_dp_write_byte_to_dpcd(dp,
> -		DPCD_ADDR_TRAINING_PATTERN_SET, buf[0]);
> +	ret = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_TRAINING_PATTERN_SET,
> +		DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_1);
> +	if (ret)
> +		return ret;
> 
>  	for (lane = 0; lane < lane_count; lane++)
>  		buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
>  			    DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
> -	ret = exynos_dp_write_bytes_to_dpcd(dp,
> -		DPCD_ADDR_TRAINING_LANE0_SET,
> +	ret = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
>  		lane_count, buf);
>  	if (ret)
>  		return ret;
> @@ -484,67 +480,32 @@ static int exynos_dp_check_max_cr_loop(struct exynos_dp_device *dp,
> 
>  static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>  {
> -	int ret;
> -	u8 data;
> -	u8 link_status[6];
> -	int lane;
> -	int lane_count;
> -	u8 buf[5];
> -
> -	u8 adjust_request[2];
> -	u8 voltage_swing;
> -	u8 pre_emphasis;
> -	u8 training_lane;
> +	int ret, lane, lane_count;
> +	u8 voltage_swing, pre_emphasis, training_lane, link_status[6];
> +	u8 *adjust_request;
> 
>  	usleep_range(100, 101);
> 
> -	ret = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
> -				6, link_status);
> +	ret = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS, 6,
> +			link_status);
>  	if (ret)
>  		return ret;
> 
>  	lane_count = dp->link_train.lane_count;
> +	adjust_request = link_status + 4;
> 
>  	if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
>  		/* set training pattern 2 for EQ */
>  		exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
> 
> -		adjust_request[0] = link_status[4];
> -		adjust_request[1] = link_status[5];
> -
> -		exynos_dp_get_adjust_train(dp, adjust_request);
> -
> -		buf[0] = DPCD_SCRAMBLING_DISABLED |
> -			 DPCD_TRAINING_PATTERN_2;
>  		ret = exynos_dp_write_byte_to_dpcd(dp,
>  			DPCD_ADDR_TRAINING_PATTERN_SET,
> -			buf[0]);
> +			DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_2);
>  		if (ret)
>  			return ret;
> 
> -		for (lane = 0; lane < lane_count; lane++) {
> -			exynos_dp_set_lane_link_training(dp,
> -				dp->link_train.training_lane[lane],
> -				lane);
> -			buf[lane] = dp->link_train.training_lane[lane];
> -			ret = exynos_dp_write_byte_to_dpcd(dp,
> -				DPCD_ADDR_TRAINING_LANE0_SET + lane,
> -				buf[lane]);
> -			if (ret)
> -				return ret;
> -		}
>  		dp->link_train.lt_state = EQUALIZER_TRAINING;
>  	} else {
> -		exynos_dp_read_byte_from_dpcd(dp,
> -			DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
> -			&data);
> -		adjust_request[0] = data;
> -
> -		exynos_dp_read_byte_from_dpcd(dp,
> -			DPCD_ADDR_ADJUST_REQUEST_LANE2_3,
> -			&data);
> -		adjust_request[1] = data;
> -
>  		for (lane = 0; lane < lane_count; lane++) {
>  			training_lane = exynos_dp_get_lane_link_training(
>  							dp, lane);
> @@ -560,36 +521,31 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
> 
>  		if (exynos_dp_check_max_cr_loop(dp, voltage_swing) != 0) {
>  			exynos_dp_reduce_link_rate(dp);
> -		} else {
> -			exynos_dp_get_adjust_train(dp, adjust_request);
> -
> -			for (lane = 0; lane < lane_count; lane++) {
> -				exynos_dp_set_lane_link_training(dp,
> -					dp->link_train.training_lane[lane],
> -					lane);
> -				buf[lane] = dp->link_train.training_lane[lane];
> -				ret = exynos_dp_write_byte_to_dpcd(dp,
> -					DPCD_ADDR_TRAINING_LANE0_SET + lane,
> -					buf[lane]);
> -				if (ret)
> -					return ret;
> -			}
> +			return ret;
>  		}
>  	}
> 
> +	exynos_dp_get_adjust_train(dp, adjust_request);
> +
> +	for (lane = 0; lane < lane_count; lane++) {
> +		exynos_dp_set_lane_link_training(dp,
> +			dp->link_train.training_lane[lane], lane);
> +		ret = exynos_dp_write_byte_to_dpcd(dp,
> +			DPCD_ADDR_TRAINING_LANE0_SET + lane,
> +			dp->link_train.training_lane[lane]);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return ret;
>  }
> 
>  static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>  {
> -	int ret;
> +	int ret, lane, lane_count;
>  	u8 link_status[6];
> -	int lane;
> -	int lane_count;
> -	u8 buf[5];
>  	u32 reg;
> -
> -	u8 adjust_request[2];
> +	u8 *adjust_request;
> 
>  	usleep_range(400, 401);
> 
> @@ -597,55 +553,53 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>  				6, link_status);
>  	if (ret)
>  		return ret;
> +
> +	adjust_request = link_status + 4;
>  	lane_count = dp->link_train.lane_count;
> 
> -	if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
> -		adjust_request[0] = link_status[4];
> -		adjust_request[1] = link_status[5];
> +	if (exynos_dp_clock_recovery_ok(link_status, lane_count)) {
> +		exynos_dp_reduce_link_rate(dp);
> +		return ret;
> +	}
> +	if (exynos_dp_channel_eq_ok(link_status, lane_count) = 0) {
> +		/* traing pattern Set to Normal */
> +		exynos_dp_training_pattern_dis(dp);
> 
> -		if (exynos_dp_channel_eq_ok(link_status, lane_count) = 0) {
> -			/* traing pattern Set to Normal */
> -			exynos_dp_training_pattern_dis(dp);
> +		dev_info(dp->dev, "Link Training success!\n");
> 
> -			dev_info(dp->dev, "Link Training success!\n");
> +		exynos_dp_get_link_bandwidth(dp, &reg);
> +		dp->link_train.link_rate = reg;
> +		dev_dbg(dp->dev, "final bandwidth = %.2x\n",
> +			dp->link_train.link_rate);
> 
> -			exynos_dp_get_link_bandwidth(dp, &reg);
> -			dp->link_train.link_rate = reg;
> -			dev_dbg(dp->dev, "final bandwidth = %.2x\n",
> -				dp->link_train.link_rate);
> +		exynos_dp_get_lane_count(dp, &reg);
> +		dp->link_train.lane_count = reg;
> +		dev_dbg(dp->dev, "final lane count = %.2x\n",
> +			dp->link_train.lane_count);
> +		/* set enhanced mode if available */
> +		exynos_dp_set_enhanced_mode(dp);
> 
> -			exynos_dp_get_lane_count(dp, &reg);
> -			dp->link_train.lane_count = reg;
> -			dev_dbg(dp->dev, "final lane count = %.2x\n",
> -				dp->link_train.lane_count);
> -			/* set enhanced mode if available */
> -			exynos_dp_set_enhanced_mode(dp);
> +		dp->link_train.lt_state = FINISHED;
> +	} else {
> +		/* not all locked */
> +		dp->link_train.eq_loop++;
> 
> -			dp->link_train.lt_state = FINISHED;
> +		if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
> +			exynos_dp_reduce_link_rate(dp);
>  		} else {
> -			/* not all locked */
> -			dp->link_train.eq_loop++;
> -
> -			if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
> -				exynos_dp_reduce_link_rate(dp);
> -			} else {
> -				exynos_dp_get_adjust_train(dp, adjust_request);
> -
> -				for (lane = 0; lane < lane_count; lane++) {
> -					exynos_dp_set_lane_link_training(dp,
> -						dp->link_train.training_lane[lane],
> -						lane);
> -					buf[lane] = dp->link_train.training_lane[lane];
> -					ret = exynos_dp_write_byte_to_dpcd(dp,
> -						DPCD_ADDR_TRAINING_LANE0_SET + lane,
> -						buf[lane]);
> -					if (ret)
> -						return ret;
> -				}
> +			exynos_dp_get_adjust_train(dp, adjust_request);
> +
> +			for (lane = 0; lane < lane_count; lane++) {
> +				exynos_dp_set_lane_link_training(dp,
> +					dp->link_train.training_lane[lane],
> +					lane);
> +				ret = exynos_dp_write_byte_to_dpcd(dp,
> +					DPCD_ADDR_TRAINING_LANE0_SET + lane,
> +					dp->link_train.training_lane[lane]);
> +				if (ret)
> +					return ret;
>  			}
>  		}
> -	} else {
> -		exynos_dp_reduce_link_rate(dp);
>  	}
> 
>  	return ret;
> --
> 1.7.7.3


^ permalink raw reply

* Re: [PATCH V4 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Tomi Valkeinen @ 2012-08-20 10:46 UTC (permalink / raw)
  To: Mahapatra, Chandrabhanu; +Cc: linux-omap, linux-fbdev
In-Reply-To: <CAF0AtAtePhxTsoCyatfXWR+e4sc3WHmg=Wq_ZXkowoGpOg63FA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

On Mon, 2012-08-20 at 16:06 +0530, Mahapatra, Chandrabhanu wrote:

> By releasing resources did you mean the dss.feat memory? I think
> dss_init_features call is wrongly placed and should be placed at the
> very beginning of the probe function.

I meant e.g. pm_runtime_disable() etc. But if the call is moved t the
beginning, there's nothing to release.

 Tomi



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH V4 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Mahapatra, Chandrabhanu @ 2012-08-20 10:48 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345452124.2684.7.camel@deskari>

On Mon, Aug 20, 2012 at 2:12 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Fri, 2012-08-17 at 16:54 +0300, Tomi Valkeinen wrote:
>> On Thu, 2012-08-16 at 16:48 +0530, Chandrabhanu Mahapatra wrote:
>> > All the cpu_is checks have been moved to dss_init_features function providing a
>> > much more generic and cleaner interface. The OMAP version and revision specific
>> > initializations in various functions are cleaned and the necessary data are
>> > moved to dss_features structure which is local to dss.c.
>> >
>> > Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>>
>> > +static int __init dss_init_features(struct device *dev)
>> > +{
>> > +   dss.feat = devm_kzalloc(dev, sizeof(*dss.feat), GFP_KERNEL);
>> > +   if (!dss.feat) {
>> > +           dev_err(dev, "Failed to allocate local DSS Features\n");
>> > +           return -ENOMEM;
>> > +   }
>> > +
>> > +   if (cpu_is_omap24xx())
>> > +           dss.feat = &omap24xx_dss_features;
>> > +   else if (cpu_is_omap34xx())
>> > +           dss.feat = &omap34xx_dss_features;
>> > +   else if (cpu_is_omap3630())
>> > +           dss.feat = &omap3630_dss_features;
>> > +   else if (cpu_is_omap44xx())
>> > +           dss.feat = &omap44xx_dss_features;
>> > +   else
>> > +           return -ENODEV;
>> > +
>> > +   return 0;
>> > +}
>>
>> This is not correct (and same problem in dispc). You allocate the feat
>> struct and assign the pointer to dss.feat, but then overwrite dss.feat
>> pointer with the pointer to omap24xx_dss_features (which is freed
>> later). You need to memcpy it.
>>

Ok.

>> I also get a crash on omap3 overo board when loading omapdss:
>

During recent tests I never saw the crash happen. The kernel used to
freeze during booting after printing
"Uncompressing Linux... done, booting the kernel."

> The crash happens because dss_get_clocks uses the feat stuff, but the
> dss_init_features is only called later. Did you test this? I can't see
> how that can work on any board.
>

Thanks for pointing that out. Correcting above fixed the problem.

> Also, in the current place you have dss_init_features call, in case
> there's an error you can't just return, you need to release the
> resources. The same problem is in the dispc.c.
>
>  Tomi
>

By releasing resources did you mean the dss.feat memory? I think
dss_init_features call is wrongly placed and should be placed at the
very beginning of the probe function.

-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Re: [RFC 0/5] Generic panel framework
From: Tomi Valkeinen @ 2012-08-20 11:39 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-fbdev, dri-devel, linux-leds, linux-media, Bryan Wu,
	Richard Purdie, Marcus Lorentzon, Sumit Semwal, Archit Taneja,
	Sebastien Guiriec, Inki Dae, Kyungmin Park
In-Reply-To: <4948190.AFNtaaFKXQ@avalon>

[-- Attachment #1: Type: text/plain, Size: 2284 bytes --]

On Sat, 2012-08-18 at 03:16 +0200, Laurent Pinchart wrote:
> Hi Tomi,

> mipi-dbi-bus might not belong to include/video/panel/ though, as it can be 
> used for non-panel devices (at least in theory). The future mipi-dsi-bus 
> certainly will.

They are both display busses. So while they could be used for anything,
I find it quite unlikely as there are much better alternatives for
generic bus needs.

> Would you be able to send incremental patches on top of v2 to implement the 
> solution you have in mind ? It would be neat if you could also implement mipi-
> dsi-bus for the OMAP DSS and test the code with a real device :-)

Yes, I'd like to try this out on OMAP, both DBI and DSI. However, I fear
it'll be quite complex due to the dependencies all around we have in the
current driver. We're working on simplifying things so that it'll be
easier to try thing like the panel framework, though, so we're going in
the right direction.

> > Generally about locks, if we define that panel ops may only be called
> > exclusively, does it simplify things? I think we can make such
> > requirements, as there should be only one display framework that handles
> > the panel. Then we don't need locking for things like enable/disable.
> 
> Pushing locking to callers would indeed simplify panel drivers, but we need to 
> make sure we won't need to expose a panel to several callers in the future.

I have a feeling that would be a bad idea.

Display related stuff are quite sensitive to any delays, so any extra
transactions over, say, DSI bus could cause a noticeable glitch on the
screen. I'm not sure what are all the possible ops that a panel can
offer, but I think all that affect the display or could cause delays
should be handled by one controlling entity (drm or such). The
controlling entity needs to handle locking anyway, so in that sense I
don't think it's an extra burden for it.

The things that come to my mind that could possibly cause calls to the
panel outside drm: debugfs, sysfs, audio, backlight. Of those, I think
backlight should go through drm. Audio, no idea. debugfs and sysfs
locking needs to be handled by the panel driver, and they are a bit
problematic as I guess having them requires full locking.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH V5 0/6] OMAPDSS: Cleanup cpu_is checks
From: Chandrabhanu Mahapatra @ 2012-08-20 13:33 UTC (permalink / raw)
  To: tomi.valkeinen, tony, paul
  Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1343912532.git.cmahapatra@ti.com>

Hi everyone,
this patch series aims at cleaning up of DSS of cpu_is checks thereby making it
more generic.

The 1st patch cleans up cpu_is checks from DISPC code.
The 2nd patch removes unused functions from DSS code.
The 3rd patch cleans up cpu_is checks from DSS code.
The 4th patch removes cpu_is checks from VENC code.
The 5th patch disables VENC support on OMAP4.
The 6th patch removes cpu_is checks from DPI code and replaces it with a
dss feature.

Changes with respect to V1 series:
 1st patch :
	* moved dispc_ops structure definitions from dss_features.c to dispc.c
	  and renamed it to dispc_features
	* moved dispc_features definitions close to dispc_init_features() thereby
	  eliminating various functions declarations from top of dispc.c
	* used __initconst before dispc_features definitions and __init before
	  dispc_init_features()
	* removed definitions of _dispc_lcd_timings_ok_24xx,
	  _dispc_lcd_timings_ok_44xx, _dispc_mgr_set_lcd_timings_hv_24xx and
	  _dispc_mgr_set_lcd_timings_hv_44xx replacing them with appropiate variables
	* renamed various dispc_features structures to give consistent naming

 3rd patch :
	* moved dss_ops structure definitions from dss_features.c to dss.c and
	  renamed it to dss_features
	* removed all functions from dss_features structure and replaced them with
	  appropiate variables
	* used __initconst before dss_features definitions and __init before 
	  dss_init_features()
	* renamed various dss_features structures to give consistent naming
	* renamed factor variable of dss_features structure to dss_fck_multiplier

All your comments and suggestions are welcome.

Refenence Tree:
git@gitorious.org:~chandrabhanu/linux-omap-dss2/chandrabhanus-linux.git dss_cleanup

Chandrabhanu Mahapatra (6):
  OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
  OMAPDSS: DSS: Remove redundant functions
  OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
  OMAPDSS: VENC: Remove cpu_is_xxxx checks
  ARM: OMAP: Disable venc for OMAP4
  OMAPDSS: DPI: Remove cpu_is_xxxx checks

 arch/arm/mach-omap2/display.c          |    1 -
 drivers/video/omap2/dss/dispc.c        |  433 ++++++++++++++++++++------------
 drivers/video/omap2/dss/dpi.c          |   12 +-
 drivers/video/omap2/dss/dss.c          |  162 ++++++------
 drivers/video/omap2/dss/dss.h          |    2 -
 drivers/video/omap2/dss/dss_features.c |    1 +
 drivers/video/omap2/dss/dss_features.h |    1 +
 drivers/video/omap2/dss/venc.c         |   11 -
 8 files changed, 363 insertions(+), 260 deletions(-)

-- 
1.7.10


^ permalink raw reply

* [PATCH V5 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-20 13:34 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1345468541.git.cmahapatra@ti.com>

All the cpu_is checks have been moved to dispc_init_features function providing
a much more generic and cleaner interface. The OMAP version and revision
specific functions and data are initialized by dispc_features structure which is
local to dispc.c.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |  433 +++++++++++++++++++++++++--------------
 1 file changed, 278 insertions(+), 155 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ff52702..3fad33a 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -81,6 +81,23 @@ struct dispc_irq_stats {
 	unsigned irqs[32];
 };
 
+struct dispc_features {
+	int hp_max;
+	int vp_max;
+	int sw_max;
+	int sw_start;
+	int fp_start;
+	int bp_start;
+	int (*calc_scaling) (enum omap_channel channel,
+		const struct omap_video_timings *mgr_timings,
+		u16 width, u16 height, u16 out_width, u16 out_height,
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+		u16 pos_x, unsigned long *core_clk);
+	unsigned long (*calc_core_clk) (enum omap_channel channel,
+		u16 width, u16 height, u16 out_width, u16 out_height);
+};
+
 static struct {
 	struct platform_device *pdev;
 	void __iomem    *base;
@@ -101,6 +118,8 @@ static struct {
 	bool		ctx_valid;
 	u32		ctx[DISPC_SZ_REGS / sizeof(u32)];
 
+	const struct dispc_features *feat;
+
 #ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
 	spinlock_t irq_stats_lock;
 	struct dispc_irq_stats irq_stats;
@@ -1939,7 +1958,18 @@ static unsigned long calc_core_clk_five_taps(enum omap_channel channel,
 	return core_clk;
 }
 
-static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
+static unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+		u16 height, u16 out_width, u16 out_height)
+{
+	unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+	if (height > out_height && width > out_width)
+		return pclk * 4;
+	else
+		return pclk * 2;
+}
+
+static unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
 		u16 height, u16 out_width, u16 out_height)
 {
 	unsigned int hf, vf;
@@ -1958,25 +1988,163 @@ static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
 		hf = 2;
 	else
 		hf = 1;
-
 	if (height > out_height)
 		vf = 2;
 	else
 		vf = 1;
 
-	if (cpu_is_omap24xx()) {
-		if (vf > 1 && hf > 1)
-			return pclk * 4;
-		else
-			return pclk * 2;
-	} else if (cpu_is_omap34xx()) {
-		return pclk * vf * hf;
-	} else {
-		if (hf > 1)
-			return DIV_ROUND_UP(pclk, out_width) * width;
-		else
-			return pclk;
+	return pclk * vf * hf;
+}
+
+static unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+		u16 height, u16 out_width, u16 out_height)
+{
+	unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+	if (width > out_width)
+		return DIV_ROUND_UP(pclk, out_width) * width;
+	else
+		return pclk;
+}
+
+static int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+		const struct omap_video_timings *mgr_timings,
+		u16 width, u16 height, u16 out_width, u16 out_height,
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+		u16 pos_x, unsigned long *core_clk)
+{
+	int error;
+	u16 in_width, in_height;
+	int min_factor = min(*decim_x, *decim_y);
+	const int maxsinglelinewidth +			dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+	*five_taps = false;
+
+	do {
+		in_height = DIV_ROUND_UP(height, *decim_y);
+		in_width = DIV_ROUND_UP(width, *decim_x);
+		*core_clk = dispc.feat->calc_core_clk(channel, in_width,
+				in_height, out_width, out_height);
+		error = (in_width > maxsinglelinewidth || !*core_clk ||
+			*core_clk > dispc_core_clk_rate());
+		if (error) {
+			if (*decim_x = *decim_y) {
+				*decim_x = min_factor;
+				++*decim_y;
+			} else {
+				swap(*decim_x, *decim_y);
+				if (*decim_x < *decim_y)
+					++*decim_x;
+			}
+		}
+	} while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+	if (in_width > maxsinglelinewidth) {
+		DSSERR("Cannot scale max input width exceeded");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+		const struct omap_video_timings *mgr_timings,
+		u16 width, u16 height, u16 out_width, u16 out_height,
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+		u16 pos_x, unsigned long *core_clk)
+{
+	int error;
+	u16 in_width, in_height;
+	int min_factor = min(*decim_x, *decim_y);
+	const int maxsinglelinewidth +			dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+	do {
+		in_height = DIV_ROUND_UP(height, *decim_y);
+		in_width = DIV_ROUND_UP(width, *decim_x);
+		*core_clk = calc_core_clk_five_taps(channel, mgr_timings,
+			in_width, in_height, out_width, out_height, color_mode);
+
+		error = check_horiz_timing_omap3(channel, mgr_timings, pos_x,
+			in_width, in_height, out_width, out_height);
+
+		if (in_width > maxsinglelinewidth)
+			if (in_height > out_height &&
+						in_height < out_height * 2)
+				*five_taps = false;
+		if (!*five_taps)
+			*core_clk = dispc.feat->calc_core_clk(channel, in_width,
+					in_height, out_width, out_height);
+
+		error = (error || in_width > maxsinglelinewidth * 2 ||
+			(in_width > maxsinglelinewidth && *five_taps) ||
+			!*core_clk || *core_clk > dispc_core_clk_rate());
+		if (error) {
+			if (*decim_x = *decim_y) {
+				*decim_x = min_factor;
+				++*decim_y;
+			} else {
+				swap(*decim_x, *decim_y);
+				if (*decim_x < *decim_y)
+					++*decim_x;
+			}
+		}
+	} while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+	if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width, height,
+		out_width, out_height)){
+			DSSERR("horizontal timing too tight\n");
+			return -EINVAL;
+	}
+
+	if (in_width > (maxsinglelinewidth * 2)) {
+		DSSERR("Cannot setup scaling");
+		DSSERR("width exceeds maximum width possible");
+		return -EINVAL;
+	}
+
+	if (in_width > maxsinglelinewidth && *five_taps) {
+		DSSERR("cannot setup scaling with five taps");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+		const struct omap_video_timings *mgr_timings,
+		u16 width, u16 height, u16 out_width, u16 out_height,
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+		u16 pos_x, unsigned long *core_clk)
+{
+	u16 in_width, in_width_max;
+	int decim_x_min = *decim_x;
+	u16 in_height = DIV_ROUND_UP(height, *decim_y);
+	const int maxsinglelinewidth +				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+	in_width_max = dispc_core_clk_rate() /
+			DIV_ROUND_UP(dispc_mgr_pclk_rate(channel), out_width);
+	*decim_x = DIV_ROUND_UP(width, in_width_max);
+
+	*decim_x = *decim_x > decim_x_min ? *decim_x : decim_x_min;
+	if (*decim_x > *x_predecim)
+		return -EINVAL;
+
+	do {
+		in_width = DIV_ROUND_UP(width, *decim_x);
+	} while (*decim_x <= *x_predecim &&
+			in_width > maxsinglelinewidth && ++*decim_x);
+
+	if (in_width > maxsinglelinewidth) {
+		DSSERR("Cannot scale width exceeds max line width");
+		return -EINVAL;
 	}
+
+	*core_clk = dispc.feat->calc_core_clk(channel, in_width, in_height,
+				out_width, out_height);
+	return 0;
 }
 
 static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1988,12 +2156,9 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 {
 	struct omap_overlay *ovl = omap_dss_get_overlay(plane);
 	const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
-	const int maxsinglelinewidth -				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
 	const int max_decim_limit = 16;
 	unsigned long core_clk = 0;
-	int decim_x, decim_y, error, min_factor;
-	u16 in_width, in_height, in_width_max = 0;
+	int decim_x, decim_y, ret;
 
 	if (width = out_width && height = out_height)
 		return 0;
@@ -2017,118 +2182,17 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	decim_x = DIV_ROUND_UP(DIV_ROUND_UP(width, out_width), maxdownscale);
 	decim_y = DIV_ROUND_UP(DIV_ROUND_UP(height, out_height), maxdownscale);
 
-	min_factor = min(decim_x, decim_y);
-
 	if (decim_x > *x_predecim || out_width > width * 8)
 		return -EINVAL;
 
 	if (decim_y > *y_predecim || out_height > height * 8)
 		return -EINVAL;
 
-	if (cpu_is_omap24xx()) {
-		*five_taps = false;
-
-		do {
-			in_height = DIV_ROUND_UP(height, decim_y);
-			in_width = DIV_ROUND_UP(width, decim_x);
-			core_clk = calc_core_clk(channel, in_width, in_height,
-					out_width, out_height);
-			error = (in_width > maxsinglelinewidth || !core_clk ||
-				core_clk > dispc_core_clk_rate());
-			if (error) {
-				if (decim_x = decim_y) {
-					decim_x = min_factor;
-					decim_y++;
-				} else {
-					swap(decim_x, decim_y);
-					if (decim_x < decim_y)
-						decim_x++;
-				}
-			}
-		} while (decim_x <= *x_predecim && decim_y <= *y_predecim &&
-				error);
-
-		if (in_width > maxsinglelinewidth) {
-			DSSERR("Cannot scale max input width exceeded");
-			return -EINVAL;
-		}
-	} else if (cpu_is_omap34xx()) {
-
-		do {
-			in_height = DIV_ROUND_UP(height, decim_y);
-			in_width = DIV_ROUND_UP(width, decim_x);
-			core_clk = calc_core_clk_five_taps(channel, mgr_timings,
-				in_width, in_height, out_width, out_height,
-				color_mode);
-
-			error = check_horiz_timing_omap3(channel, mgr_timings,
-				pos_x, in_width, in_height, out_width,
-				out_height);
-
-			if (in_width > maxsinglelinewidth)
-				if (in_height > out_height &&
-					in_height < out_height * 2)
-					*five_taps = false;
-			if (!*five_taps)
-				core_clk = calc_core_clk(channel, in_width,
-					in_height, out_width, out_height);
-			error = (error || in_width > maxsinglelinewidth * 2 ||
-				(in_width > maxsinglelinewidth && *five_taps) ||
-				!core_clk || core_clk > dispc_core_clk_rate());
-			if (error) {
-				if (decim_x = decim_y) {
-					decim_x = min_factor;
-					decim_y++;
-				} else {
-					swap(decim_x, decim_y);
-					if (decim_x < decim_y)
-						decim_x++;
-				}
-			}
-		} while (decim_x <= *x_predecim && decim_y <= *y_predecim
-			&& error);
-
-		if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width,
-			height, out_width, out_height)){
-				DSSERR("horizontal timing too tight\n");
-				return -EINVAL;
-		}
-
-		if (in_width > (maxsinglelinewidth * 2)) {
-			DSSERR("Cannot setup scaling");
-			DSSERR("width exceeds maximum width possible");
-			return -EINVAL;
-		}
-
-		if (in_width > maxsinglelinewidth && *five_taps) {
-			DSSERR("cannot setup scaling with five taps");
-			return -EINVAL;
-		}
-	} else {
-		int decim_x_min = decim_x;
-		in_height = DIV_ROUND_UP(height, decim_y);
-		in_width_max = dispc_core_clk_rate() /
-				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
-						out_width);
-		decim_x = DIV_ROUND_UP(width, in_width_max);
-
-		decim_x = decim_x > decim_x_min ? decim_x : decim_x_min;
-		if (decim_x > *x_predecim)
-			return -EINVAL;
-
-		do {
-			in_width = DIV_ROUND_UP(width, decim_x);
-		} while (decim_x <= *x_predecim &&
-				in_width > maxsinglelinewidth && decim_x++);
-
-		if (in_width > maxsinglelinewidth) {
-			DSSERR("Cannot scale width exceeds max line width");
-			return -EINVAL;
-		}
-
-		core_clk = calc_core_clk(channel, in_width, in_height,
-				out_width, out_height);
-	}
+	ret = dispc.feat->calc_scaling(channel, mgr_timings, width, height,
+		out_width, out_height, color_mode, five_taps, x_predecim,
+		y_predecim, &decim_x, &decim_y, pos_x, &core_clk);
+	if (ret)
+		return ret;
 
 	DSSDBG("required core clk rate = %lu Hz\n", core_clk);
 	DSSDBG("current core clk rate = %lu Hz\n", dispc_core_clk_rate());
@@ -2604,24 +2668,13 @@ static bool _dispc_mgr_size_ok(u16 width, u16 height)
 static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
 		int vsw, int vfp, int vbp)
 {
-	if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
-		if (hsw < 1 || hsw > 64 ||
-				hfp < 1 || hfp > 256 ||
-				hbp < 1 || hbp > 256 ||
-				vsw < 1 || vsw > 64 ||
-				vfp < 0 || vfp > 255 ||
-				vbp < 0 || vbp > 255)
-			return false;
-	} else {
-		if (hsw < 1 || hsw > 256 ||
-				hfp < 1 || hfp > 4096 ||
-				hbp < 1 || hbp > 4096 ||
-				vsw < 1 || vsw > 256 ||
-				vfp < 0 || vfp > 4095 ||
-				vbp < 0 || vbp > 4095)
-			return false;
-	}
-
+	if (hsw < 1 || hsw > dispc.feat->sw_max ||
+			hfp < 1 || hfp > dispc.feat->hp_max ||
+			hbp < 1 || hbp > dispc.feat->hp_max ||
+			vsw < 1 || vsw > dispc.feat->sw_max ||
+			vfp < 0 || vfp > dispc.feat->vp_max ||
+			vbp < 0 || vbp > dispc.feat->vp_max)
+		return false;
 	return true;
 }
 
@@ -2653,19 +2706,12 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
 	u32 timing_h, timing_v, l;
 	bool onoff, rf, ipc;
 
-	if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
-		timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
-			FLD_VAL(hbp-1, 27, 20);
-
-		timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
-			FLD_VAL(vbp, 27, 20);
-	} else {
-		timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
-			FLD_VAL(hbp-1, 31, 20);
-
-		timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
-			FLD_VAL(vbp, 31, 20);
-	}
+	timing_h = FLD_VAL(hsw-1, dispc.feat->sw_start, 0) |
+			FLD_VAL(hfp-1, dispc.feat->fp_start, 8) |
+			FLD_VAL(hbp-1, dispc.feat->bp_start, 20);
+	timing_v = FLD_VAL(vsw-1, dispc.feat->sw_start, 0) |
+			FLD_VAL(vfp, dispc.feat->fp_start, 8) |
+			FLD_VAL(vbp, dispc.feat->bp_start, 20);
 
 	dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
 	dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
@@ -3671,6 +3717,79 @@ static void _omap_dispc_initial_config(void)
 	dispc_ovl_enable_zorder_planes();
 }
 
+static const struct dispc_features omap24xx_dispc_feats __initconst = {
+	.hp_max			=	256,
+	.vp_max			=	255,
+	.sw_max			=	64,
+	.sw_start		=	5,
+	.fp_start		=	15,
+	.bp_start		=	27,
+	.calc_scaling		=	dispc_ovl_calc_scaling_24xx,
+	.calc_core_clk		=	calc_core_clk_24xx,
+};
+
+static const struct dispc_features omap34xx_rev1_0_dispc_feats __initconst = {
+	.hp_max			=	256,
+	.vp_max			=	255,
+	.sw_max			=	64,
+	.sw_start		=	5,
+	.fp_start		=	15,
+	.bp_start		=	27,
+	.calc_scaling		=	dispc_ovl_calc_scaling_34xx,
+	.calc_core_clk		=	calc_core_clk_34xx,
+};
+
+static const struct dispc_features omap34xx_rev3_0_dispc_feats __initconst = {
+	.hp_max			=	4096,
+	.vp_max			=	4095,
+	.sw_max			=	256,
+	.sw_start		=	7,
+	.fp_start		=	19,
+	.bp_start		=	31,
+	.calc_scaling		=	dispc_ovl_calc_scaling_34xx,
+	.calc_core_clk		=	calc_core_clk_34xx,
+};
+
+static const struct dispc_features omap44xx_dispc_feats __initconst = {
+	.hp_max			=	4096,
+	.vp_max			=	4095,
+	.sw_max			=	256,
+	.sw_start		=	7,
+	.fp_start		=	19,
+	.bp_start		=	31,
+	.calc_scaling		=	dispc_ovl_calc_scaling_44xx,
+	.calc_core_clk		=	calc_core_clk_44xx,
+};
+
+static int __init dispc_init_features(struct device *dev)
+{
+	struct dispc_features *feat = devm_kzalloc(dev, sizeof(*feat),
+								GFP_KERNEL);
+	if (!feat) {
+		dev_err(dev, "Failed to allocate DISPC Features\n");
+		return -ENOMEM;
+	}
+
+	if (cpu_is_omap24xx()) {
+		memcpy(feat, &omap24xx_dispc_feats, sizeof(*feat));
+	} else if (cpu_is_omap34xx()) {
+		if (omap_rev() < OMAP3430_REV_ES3_0)
+			memcpy(feat, &omap34xx_rev1_0_dispc_feats,
+							sizeof(*feat));
+		else
+			memcpy(feat, &omap34xx_rev3_0_dispc_feats,
+							sizeof(*feat));
+	} else if (cpu_is_omap44xx()) {
+		memcpy(feat, &omap44xx_dispc_feats, sizeof(*feat));
+	} else {
+		return -ENODEV;
+	}
+
+	dispc.feat = feat;
+
+	return 0;
+}
+
 /* DISPC HW IP initialisation */
 static int __init omap_dispchw_probe(struct platform_device *pdev)
 {
@@ -3681,6 +3800,10 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
 
 	dispc.pdev = pdev;
 
+	r = dispc_init_features(&dispc.pdev->dev);
+	if (r)
+		return r;
+
 	spin_lock_init(&dispc.irq_lock);
 
 #ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
-- 
1.7.10


^ permalink raw reply related

* [PATCH V5 2/6] OMAPDSS: DSS: Remove redundant functions
From: Chandrabhanu Mahapatra @ 2012-08-20 13:35 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1345468541.git.cmahapatra@ti.com>

Functions dss_calc_clock_rates() and dss_get_clock_div() are removed as these
functions have become redundant and no longer used.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dss.c |   45 -----------------------------------------
 drivers/video/omap2/dss/dss.h |    2 --
 2 files changed, 47 deletions(-)

diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 4491ab7..10566ae 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -431,31 +431,6 @@ enum omap_dss_clk_source dss_get_lcd_clk_source(enum omap_channel channel)
 	}
 }
 
-/* calculate clock rates using dividers in cinfo */
-int dss_calc_clock_rates(struct dss_clock_info *cinfo)
-{
-	if (dss.dpll4_m4_ck) {
-		unsigned long prate;
-		u16 fck_div_max = 16;
-
-		if (cpu_is_omap3630() || cpu_is_omap44xx())
-			fck_div_max = 32;
-
-		if (cinfo->fck_div > fck_div_max || cinfo->fck_div = 0)
-			return -EINVAL;
-
-		prate = clk_get_rate(clk_get_parent(dss.dpll4_m4_ck));
-
-		cinfo->fck = prate / cinfo->fck_div;
-	} else {
-		if (cinfo->fck_div != 0)
-			return -EINVAL;
-		cinfo->fck = clk_get_rate(dss.dss_clk);
-	}
-
-	return 0;
-}
-
 int dss_set_clock_div(struct dss_clock_info *cinfo)
 {
 	if (dss.dpll4_m4_ck) {
@@ -478,26 +453,6 @@ int dss_set_clock_div(struct dss_clock_info *cinfo)
 	return 0;
 }
 
-int dss_get_clock_div(struct dss_clock_info *cinfo)
-{
-	cinfo->fck = clk_get_rate(dss.dss_clk);
-
-	if (dss.dpll4_m4_ck) {
-		unsigned long prate;
-
-		prate = clk_get_rate(clk_get_parent(dss.dpll4_m4_ck));
-
-		if (cpu_is_omap3630() || cpu_is_omap44xx())
-			cinfo->fck_div = prate / (cinfo->fck);
-		else
-			cinfo->fck_div = prate / (cinfo->fck / 2);
-	} else {
-		cinfo->fck_div = 0;
-	}
-
-	return 0;
-}
-
 unsigned long dss_get_dpll4_rate(void)
 {
 	if (dss.dpll4_m4_ck)
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index f67afe7..33c0952 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -296,9 +296,7 @@ void dss_set_venc_output(enum omap_dss_venc_type type);
 void dss_set_dac_pwrdn_bgz(bool enable);
 
 unsigned long dss_get_dpll4_rate(void);
-int dss_calc_clock_rates(struct dss_clock_info *cinfo);
 int dss_set_clock_div(struct dss_clock_info *cinfo);
-int dss_get_clock_div(struct dss_clock_info *cinfo);
 int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
 		struct dispc_clock_info *dispc_cinfo);
 
-- 
1.7.10


^ permalink raw reply related

* [PATCH V5 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-20 13:35 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1345468541.git.cmahapatra@ti.com>

All the cpu_is checks have been moved to dss_init_features function providing a
much more generic and cleaner interface. The OMAP version and revision specific
initializations in various functions are cleaned and the necessary data are
moved to dss_features structure which is local to dss.c.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dss.c |  117 ++++++++++++++++++++++++++---------------
 1 file changed, 76 insertions(+), 41 deletions(-)

diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 10566ae..f6d3d0a 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -31,6 +31,7 @@
 #include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/gfp.h>
 
 #include <video/omapdss.h>
 
@@ -65,6 +66,12 @@ struct dss_reg {
 static int dss_runtime_get(void);
 static void dss_runtime_put(void);
 
+struct dss_features {
+	u16 fck_div_max;
+	int dss_fck_multiplier;
+	char *clk_name;
+};
+
 static struct {
 	struct platform_device *pdev;
 	void __iomem    *base;
@@ -83,6 +90,8 @@ static struct {
 
 	bool		ctx_valid;
 	u32		ctx[DSS_SZ_REGS / sizeof(u32)];
+
+	const struct dss_features *feat;
 } dss;
 
 static const char * const dss_generic_clk_source_names[] = {
@@ -91,6 +100,30 @@ static const char * const dss_generic_clk_source_names[] = {
 	[OMAP_DSS_CLK_SRC_FCK]			= "DSS_FCK",
 };
 
+static const struct dss_features omap24xx_dss_feats __initconst = {
+	.fck_div_max		=	16,
+	.dss_fck_multiplier	=	2,
+	.clk_name		=	NULL,
+};
+
+static const struct dss_features omap34xx_dss_feats __initconst = {
+	.fck_div_max		=	16,
+	.dss_fck_multiplier	=	2,
+	.clk_name		=	"dpll4_m4_ck",
+};
+
+static const struct dss_features omap3630_dss_feats __initconst = {
+	.fck_div_max		=	32,
+	.dss_fck_multiplier	=	1,
+	.clk_name		=	"dpll4_m4_ck",
+};
+
+static const struct dss_features omap44xx_dss_feats __initconst = {
+	.fck_div_max		=	32,
+	.dss_fck_multiplier	=	1,
+	.clk_name		=	"dpll_per_m5x2_ck",
+};
+
 static inline void dss_write_reg(const struct dss_reg idx, u32 val)
 {
 	__raw_writel(val, dss.base + idx.idx);
@@ -236,7 +269,6 @@ const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src)
 	return dss_generic_clk_source_names[clk_src];
 }
 
-
 void dss_dump_clocks(struct seq_file *s)
 {
 	unsigned long dpll4_ck_rate;
@@ -259,18 +291,10 @@ void dss_dump_clocks(struct seq_file *s)
 
 		seq_printf(s, "dpll4_ck %lu\n", dpll4_ck_rate);
 
-		if (cpu_is_omap3630() || cpu_is_omap44xx())
-			seq_printf(s, "%s (%s) = %lu / %lu  = %lu\n",
-					fclk_name, fclk_real_name,
-					dpll4_ck_rate,
-					dpll4_ck_rate / dpll4_m4_ck_rate,
-					fclk_rate);
-		else
-			seq_printf(s, "%s (%s) = %lu / %lu * 2 = %lu\n",
-					fclk_name, fclk_real_name,
-					dpll4_ck_rate,
-					dpll4_ck_rate / dpll4_m4_ck_rate,
-					fclk_rate);
+		seq_printf(s, "%s (%s) = %lu / %lu * %d  = %lu\n",
+				fclk_name, fclk_real_name, dpll4_ck_rate,
+				dpll4_ck_rate / dpll4_m4_ck_rate,
+				dss.feat->dss_fck_multiplier, fclk_rate);
 	} else {
 		seq_printf(s, "%s (%s) = %lu\n",
 				fclk_name, fclk_real_name,
@@ -470,7 +494,7 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
 
 	unsigned long fck, max_dss_fck;
 
-	u16 fck_div, fck_div_max = 16;
+	u16 fck_div;
 
 	int match = 0;
 	int min_fck_per_pck;
@@ -480,9 +504,8 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
 	max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
 
 	fck = clk_get_rate(dss.dss_clk);
-	if (req_pck = dss.cache_req_pck &&
-			((cpu_is_omap34xx() && prate = dss.cache_prate) ||
-			 dss.cache_dss_cinfo.fck = fck)) {
+	if (req_pck = dss.cache_req_pck && prate = dss.cache_prate &&
+		dss.cache_dss_cinfo.fck = fck) {
 		DSSDBG("dispc clock info found from cache.\n");
 		*dss_cinfo = dss.cache_dss_cinfo;
 		*dispc_cinfo = dss.cache_dispc_cinfo;
@@ -519,16 +542,10 @@ retry:
 
 		goto found;
 	} else {
-		if (cpu_is_omap3630() || cpu_is_omap44xx())
-			fck_div_max = 32;
-
-		for (fck_div = fck_div_max; fck_div > 0; --fck_div) {
+		for (fck_div = dss.feat->fck_div_max; fck_div > 0; --fck_div) {
 			struct dispc_clock_info cur_dispc;
 
-			if (fck_div_max = 32)
-				fck = prate / fck_div;
-			else
-				fck = prate / fck_div * 2;
+			fck = prate / fck_div * dss.feat->dss_fck_multiplier;
 
 			if (fck > max_dss_fck)
 				continue;
@@ -645,22 +662,11 @@ static int dss_get_clocks(void)
 
 	dss.dss_clk = clk;
 
-	if (cpu_is_omap34xx()) {
-		clk = clk_get(NULL, "dpll4_m4_ck");
-		if (IS_ERR(clk)) {
-			DSSERR("Failed to get dpll4_m4_ck\n");
-			r = PTR_ERR(clk);
-			goto err;
-		}
-	} else if (cpu_is_omap44xx()) {
-		clk = clk_get(NULL, "dpll_per_m5x2_ck");
-		if (IS_ERR(clk)) {
-			DSSERR("Failed to get dpll_per_m5x2_ck\n");
-			r = PTR_ERR(clk);
-			goto err;
-		}
-	} else { /* omap24xx */
-		clk = NULL;
+	clk = clk_get(NULL, dss.feat->clk_name);
+	if (IS_ERR(clk)) {
+		DSSERR("Failed to get %s\n", dss.feat->clk_name);
+		r = PTR_ERR(clk);
+		goto err;
 	}
 
 	dss.dpll4_m4_ck = clk;
@@ -716,6 +722,31 @@ void dss_debug_dump_clocks(struct seq_file *s)
 }
 #endif
 
+static int __init dss_init_features(struct device *dev)
+{
+	struct dss_features *feat = devm_kzalloc(dev, sizeof(*feat),
+								GFP_KERNEL);
+	if (!feat) {
+		dev_err(dev, "Failed to allocate local DSS Features\n");
+		return -ENOMEM;
+	}
+
+	if (cpu_is_omap24xx())
+		memcpy(feat, &omap24xx_dss_feats, sizeof(*feat));
+	else if (cpu_is_omap34xx())
+		memcpy(feat, &omap34xx_dss_feats, sizeof(*feat));
+	else if (cpu_is_omap3630())
+		memcpy(feat, &omap3630_dss_feats, sizeof(*feat));
+	else if (cpu_is_omap44xx())
+		memcpy(feat, &omap44xx_dss_feats, sizeof(*feat));
+	else
+		return -ENODEV;
+
+	dss.feat = feat;
+
+	return 0;
+}
+
 /* DSS HW IP initialisation */
 static int __init omap_dsshw_probe(struct platform_device *pdev)
 {
@@ -725,6 +756,10 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
 
 	dss.pdev = pdev;
 
+	r = dss_init_features(&dss.pdev->dev);
+	if (r)
+		return r;
+
 	dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0);
 	if (!dss_mem) {
 		DSSERR("can't get IORESOURCE_MEM DSS\n");
-- 
1.7.10


^ permalink raw reply related

* [PATCH V5 4/6] OMAPDSS: VENC: Remove cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-20 13:36 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1345468541.git.cmahapatra@ti.com>

OMAP4 checks are removed from VENC to provide it a cleaner interface. These
checks were introduced by patches "HACK: OMAP: DSS2: VENC: disable VENC on OMAP4
to prevent crash" (ba02fa37de) by Tomi Valkeinen <tomi.valkeinen@ti.com> and
"OMAPDSS: VENC: fix NULL pointer dereference in DSS2 VENC sysfs debug attr on
OMAP4" (cc1d3e032d)  by Danny Kukawka <danny.kukawka@bisect.de> to prevent VENC
from crashing OMAP4 kernel. An alternative approach is used to do the same in
later patches.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/venc.c |   11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 3a22087..8fc165a 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -752,11 +752,6 @@ static void venc_dump_regs(struct seq_file *s)
 {
 #define DUMPREG(r) seq_printf(s, "%-35s %08x\n", #r, venc_read_reg(r))
 
-	if (cpu_is_omap44xx()) {
-		seq_printf(s, "VENC currently disabled on OMAP44xx\n");
-		return;
-	}
-
 	if (venc_runtime_get())
 		return;
 
@@ -971,16 +966,10 @@ static struct platform_driver omap_venchw_driver = {
 
 int __init venc_init_platform_driver(void)
 {
-	if (cpu_is_omap44xx())
-		return 0;
-
 	return platform_driver_probe(&omap_venchw_driver, omap_venchw_probe);
 }
 
 void __exit venc_uninit_platform_driver(void)
 {
-	if (cpu_is_omap44xx())
-		return;
-
 	platform_driver_unregister(&omap_venchw_driver);
 }
-- 
1.7.10


^ permalink raw reply related

* [PATCH V5 5/6] ARM: OMAP: Disable venc for OMAP4
From: Chandrabhanu Mahapatra @ 2012-08-20 13:36 UTC (permalink / raw)
  To: tomi.valkeinen, tony, paul
  Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1345468541.git.cmahapatra@ti.com>

This is a alternative to "HACK: OMAP: DSS2: VENC: disable VENC on OMAP4 to
prevent crash" (ba02fa37de) by Tomi Valkeinen <tomi.valkeinen@ti.com> to prevent
VENC from crashing OMAP4 kernel. This prevents OMAPDSS from initial registration
of a device for VENC on OMAP4.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 arch/arm/mach-omap2/display.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index af1ed7d..ee40739 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -95,7 +95,6 @@ static const struct omap_dss_hwmod_data omap4_dss_hwmod_data[] __initdata = {
 	{ "dss_core", "omapdss_dss", -1 },
 	{ "dss_dispc", "omapdss_dispc", -1 },
 	{ "dss_rfbi", "omapdss_rfbi", -1 },
-	{ "dss_venc", "omapdss_venc", -1 },
 	{ "dss_dsi1", "omapdss_dsi", 0 },
 	{ "dss_dsi2", "omapdss_dsi", 1 },
 	{ "dss_hdmi", "omapdss_hdmi", -1 },
-- 
1.7.10


^ permalink raw reply related

* [PATCH V5 6/6] OMAPDSS: DPI: Remove cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-20 13:36 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1345468541.git.cmahapatra@ti.com>

The OMAP3 checks have been removed and replaced by a dss feature
FEAT_DPI_USES_VDDS_DSI for cleaner implementation. The patches
"OMAP: DSS2: enable VDDS_DSI when using DPI" (8a2cfea8cc) by Tomi Valkeinen
<tomi.valkeinen@nokia.com> and "ARM: omap: fix oops in
drivers/video/omap2/dss/dpi.c" (4041071571) by Russell King
<rmk+kernel@arm.linux.org.uk> had introduced these checks. As it is evident
from these patches a dependency exists for some DSS pins on VDDS_DSI which is
better shown by dss feature FEAT_DPI_USES_VDDS_DSI.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dpi.c          |   12 +++++++-----
 drivers/video/omap2/dss/dss_features.c |    1 +
 drivers/video/omap2/dss/dss_features.h |    1 +
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 3266be2..b97f7b8 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -34,6 +34,7 @@
 #include <plat/cpu.h>
 
 #include "dss.h"
+#include "dss_features.h"
 
 static struct {
 	struct regulator *vdds_dsi_reg;
@@ -169,7 +170,7 @@ int omapdss_dpi_display_enable(struct omap_dss_device *dssdev)
 {
 	int r;
 
-	if (cpu_is_omap34xx() && !dpi.vdds_dsi_reg) {
+	if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI) && !dpi.vdds_dsi_reg) {
 		DSSERR("no VDSS_DSI regulator\n");
 		return -ENODEV;
 	}
@@ -185,7 +186,7 @@ int omapdss_dpi_display_enable(struct omap_dss_device *dssdev)
 		goto err_start_dev;
 	}
 
-	if (cpu_is_omap34xx()) {
+	if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI)) {
 		r = regulator_enable(dpi.vdds_dsi_reg);
 		if (r)
 			goto err_reg_enable;
@@ -229,7 +230,7 @@ err_dsi_pll_init:
 err_get_dsi:
 	dispc_runtime_put();
 err_get_dispc:
-	if (cpu_is_omap34xx())
+	if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI))
 		regulator_disable(dpi.vdds_dsi_reg);
 err_reg_enable:
 	omap_dss_stop_device(dssdev);
@@ -250,7 +251,7 @@ void omapdss_dpi_display_disable(struct omap_dss_device *dssdev)
 
 	dispc_runtime_put();
 
-	if (cpu_is_omap34xx())
+	if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI))
 		regulator_disable(dpi.vdds_dsi_reg);
 
 	omap_dss_stop_device(dssdev);
@@ -329,7 +330,8 @@ static int __init dpi_init_display(struct omap_dss_device *dssdev)
 {
 	DSSDBG("init_display\n");
 
-	if (cpu_is_omap34xx() && dpi.vdds_dsi_reg = NULL) {
+	if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI) &&
+					dpi.vdds_dsi_reg = NULL) {
 		struct regulator *vdds_dsi;
 
 		vdds_dsi = dss_get_vdds_dsi();
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index 9387097..2fe39d6 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -373,6 +373,7 @@ static const enum dss_feat_id omap3430_dss_feat_list[] = {
 	FEAT_ALPHA_FIXED_ZORDER,
 	FEAT_FIFO_MERGE,
 	FEAT_OMAP3_DSI_FIFO_BUG,
+	FEAT_DPI_USES_VDDS_DSI,
 };
 
 static const enum dss_feat_id omap3630_dss_feat_list[] = {
diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
index 996ffcb..26d43a4 100644
--- a/drivers/video/omap2/dss/dss_features.h
+++ b/drivers/video/omap2/dss/dss_features.h
@@ -50,6 +50,7 @@ enum dss_feat_id {
 	FEAT_DSI_VC_OCP_WIDTH,
 	FEAT_DSI_REVERSE_TXCLKESC,
 	FEAT_DSI_GNQ,
+	FEAT_DPI_USES_VDDS_DSI,
 	FEAT_HDMI_CTS_SWMODE,
 	FEAT_HDMI_AUDIO_USE_MCLK,
 	FEAT_HANDLE_UV_SEPARATE,
-- 
1.7.10


^ permalink raw reply related

* Re: i.MX27 SLCDC driver
From: Matt Sealey @ 2012-08-20 15:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <502DEDCD.9050303@gmail.com>

On Fri, Aug 17, 2012 at 2:07 AM, Gaëtan Carlier <gcembed@gmail.com> wrote:
> Hello,
> I would like to write the driver for SmartLCD controller of i.MX27. This
> kind of interface needs a LCD with an embedded graphic controller partially
> controlled by GPIO. I want to link it with an OLED DD12832.
>
> How have I to write this driver : One driver for SLCDC and one driver for
> DD12832 ? How to link them together ?
>
> Maybe use similar philosophy than soc-camera ?

Please use DRM if you can. That way your SLCDC controller is a "crtc"
and your OLED panel is a combination "encoder" and "connector". The
DRM subsystem isn't by far the best solution since it is designed
around PCI-class graphics cards and the split is intended for those
and not for more tightly-knit LCDC+display combinations, but it does
split things the way you're thinking and with a little extra code it
will do exactly what you want if a little less efficient than your
attached driver.

I was sure Sascha had a driver for this already;
http://git.pengutronix.de/?p=imx/linux-2.6.git;a=commit;hÃ33f47fa185edfd1c9d889fc8af040dd557b02c
- it should not be too hard to add your panel and the specific gpio
control as an encoder/connector. Maybe with a user this can actually
go upstream.

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply

* Re: i.MX27 SLCDC driver
From: Gaëtan Carlier @ 2012-08-20 16:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKGA1bmuUxiHbkF0n+bOx81EBQujuhu+piDsfHSVo2LejoPmcw@mail.gmail.com>

Hello
On 08/20/2012 05:21 PM, Matt Sealey wrote:
> On Fri, Aug 17, 2012 at 2:07 AM, Gaëtan Carlier <gcembed@gmail.com> wrote:
>> Hello,
>> I would like to write the driver for SmartLCD controller of i.MX27. This
>> kind of interface needs a LCD with an embedded graphic controller partially
>> controlled by GPIO. I want to link it with an OLED DD12832.
>>
>> How have I to write this driver : One driver for SLCDC and one driver for
>> DD12832 ? How to link them together ?
>>
>> Maybe use similar philosophy than soc-camera ?
>
> Please use DRM if you can. That way your SLCDC controller is a "crtc"
> and your OLED panel is a combination "encoder" and "connector". The
> DRM subsystem isn't by far the best solution since it is designed
> around PCI-class graphics cards and the split is intended for those
> and not for more tightly-knit LCDC+display combinations, but it does
> split things the way you're thinking and with a little extra code it
> will do exactly what you want if a little less efficient than your
> attached driver.
>
> I was sure Sascha had a driver for this already;
> http://git.pengutronix.de/?p=imx/linux-2.6.git;a=commit;hÃ33f47fa185edfd1c9d889fc8af040dd557b02c
> - it should not be too hard to add your panel and the specific gpio
> control as an encoder/connector. Maybe with a user this can actually
> go upstream.
>
Thanks a lot for your advises. I will study this code.
Regards,
Gaëtan Carlier.

^ permalink raw reply

* [PATCH 0/7] framebuffer fixes
From: Mikulas Patocka @ 2012-08-20 18:04 UTC (permalink / raw)
  To: linux-fbdev

Hi

Here I'm sending 7 patches for Linux framebuffer.

The first patch fixes a bug in cfb_copyarea if the position is not aligned 
on 4-byte or 8-byte boundary.

The next three patches fix bugs on mach64 and remove videomode 
restriction.

The next three patches fix bugs on tga and remove videomode restriction.

Mikulas

^ permalink raw reply

* [PATCH 1/7] framebuffer: fix cfb_copyarea
From: Mikulas Patocka @ 2012-08-20 18:05 UTC (permalink / raw)
  To: linux-fbdev

framebuffer: fix cfb_copyarea

The function cfb_copyarea is buggy when the copy operation is not aligned on
long boundary (4 bytes on 32-bit machines, 8 bytes on 64-bit machines).

How to reproduce:
- use x86-64 machine
- use a framebuffer driver without acceleration (for example uvesafb)
- set the framebuffer to 8-bit depth
	(for example fbset -a 1024x768-60 -depth 8)
- load a font with character width that is not a multiple of 8 pixels
	note: the console-tools package cannot load a font that has
	width different from 8 pixels. You need to install the packages
	"kbd" and "console-terminus" and use the program "setfont" to
	set font width (for example: setfont Uni2-Terminus20x10)
- move some text left and right on the bash command line and you get a
	screen corruption

To expose more bugs,
put this line to the end of uvesafb_init_info:
info->flags |= FBINFO_HWACCEL_COPYAREA | FBINFO_READS_FAST;
- Now framebuffer console will use cfb_copyarea for console scrolling.
You get a screen corruption when console is scrolled.

This patch is a rewrite of cfb_copyarea. It fixes the bugs, with this
patch, console scrolling in 8-bit depth with a font width that is not a
multiple of 8 pixels works fine.

The cfb_copyarea code was very buggy and it looks like it was written
and never tried with non-8-pixel font.

Signed-off-by: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
Cc: stable@kernel.org

---
 drivers/video/cfbcopyarea.c |  153 ++++++++++++++++++++++----------------------
 1 file changed, 78 insertions(+), 75 deletions(-)

Index: linux-3.4.3-fast/drivers/video/cfbcopyarea.c
=================================--- linux-3.4.3-fast.orig/drivers/video/cfbcopyarea.c	2012-07-02 02:02:08.000000000 +0200
+++ linux-3.4.3-fast/drivers/video/cfbcopyarea.c	2012-07-02 02:03:33.000000000 +0200
@@ -43,13 +43,22 @@
      */
 
 static void
-bitcpy(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
-		const unsigned long __iomem *src, int src_idx, int bits,
+bitcpy(struct fb_info *p, unsigned long __iomem *dst, unsigned dst_idx,
+		const unsigned long __iomem *src, unsigned src_idx, int bits,
 		unsigned n, u32 bswapmask)
 {
 	unsigned long first, last;
 	int const shift = dst_idx-src_idx;
-	int left, right;
+
+#if 0
+	/*
+	 * If you suspect bug in this function, compare it with this simple
+	 * memmove implementation.
+	 */
+	fb_memmove((char *)dst + ((dst_idx & (bits - 1))) / 8,
+		   (char *)src + ((src_idx & (bits - 1))) / 8, n / 8);
+	return;
+#endif
 
 	first = fb_shifted_pixels_mask_long(p, dst_idx, bswapmask);
 	last = ~fb_shifted_pixels_mask_long(p, (dst_idx+n) % bits, bswapmask);
@@ -98,9 +107,8 @@ bitcpy(struct fb_info *p, unsigned long
 		unsigned long d0, d1;
 		int m;
 
-		right = shift & (bits - 1);
-		left = -shift & (bits - 1);
-		bswapmask &= shift;
+		int const left = shift & (bits - 1);
+		int const right = -shift & (bits - 1);
 
 		if (dst_idx+n <= bits) {
 			// Single destination word
@@ -110,15 +118,15 @@ bitcpy(struct fb_info *p, unsigned long
 			d0 = fb_rev_pixels_in_long(d0, bswapmask);
 			if (shift > 0) {
 				// Single source word
-				d0 >>= right;
+				d0 <<= left;
 			} else if (src_idx+n <= bits) {
 				// Single source word
-				d0 <<= left;
+				d0 >>= right;
 			} else {
 				// 2 source words
 				d1 = FB_READL(src + 1);
 				d1 = fb_rev_pixels_in_long(d1, bswapmask);
-				d0 = d0<<left | d1>>right;
+				d0 = d0 >> right | d1 << left;
 			}
 			d0 = fb_rev_pixels_in_long(d0, bswapmask);
 			FB_WRITEL(comp(d0, FB_READL(dst), first), dst);
@@ -135,60 +143,59 @@ bitcpy(struct fb_info *p, unsigned long
 			if (shift > 0) {
 				// Single source word
 				d1 = d0;
-				d0 >>= right;
-				dst++;
+				d0 <<= left;
 				n -= bits - dst_idx;
 			} else {
 				// 2 source words
 				d1 = FB_READL(src++);
 				d1 = fb_rev_pixels_in_long(d1, bswapmask);
 
-				d0 = d0<<left | d1>>right;
-				dst++;
+				d0 = d0 >> right | d1 << left;
 				n -= bits - dst_idx;
 			}
 			d0 = fb_rev_pixels_in_long(d0, bswapmask);
 			FB_WRITEL(comp(d0, FB_READL(dst), first), dst);
 			d0 = d1;
+			dst++;
 
 			// Main chunk
 			m = n % bits;
 			n /= bits;
 			while ((n >= 4) && !bswapmask) {
 				d1 = FB_READL(src++);
-				FB_WRITEL(d0 << left | d1 >> right, dst++);
+				FB_WRITEL(d0 >> right | d1 << left, dst++);
 				d0 = d1;
 				d1 = FB_READL(src++);
-				FB_WRITEL(d0 << left | d1 >> right, dst++);
+				FB_WRITEL(d0 >> right | d1 << left, dst++);
 				d0 = d1;
 				d1 = FB_READL(src++);
-				FB_WRITEL(d0 << left | d1 >> right, dst++);
+				FB_WRITEL(d0 >> right | d1 << left, dst++);
 				d0 = d1;
 				d1 = FB_READL(src++);
-				FB_WRITEL(d0 << left | d1 >> right, dst++);
+				FB_WRITEL(d0 >> right | d1 << left, dst++);
 				d0 = d1;
 				n -= 4;
 			}
 			while (n--) {
 				d1 = FB_READL(src++);
 				d1 = fb_rev_pixels_in_long(d1, bswapmask);
-				d0 = d0 << left | d1 >> right;
+				d0 = d0 >> right | d1 << left;
 				d0 = fb_rev_pixels_in_long(d0, bswapmask);
 				FB_WRITEL(d0, dst++);
 				d0 = d1;
 			}
 
 			// Trailing bits
-			if (last) {
-				if (m <= right) {
+			if (m) {
+				if (m <= bits - right) {
 					// Single source word
-					d0 <<= left;
+					d0 >>= right;
 				} else {
 					// 2 source words
 					d1 = FB_READL(src);
 					d1 = fb_rev_pixels_in_long(d1,
 								bswapmask);
-					d0 = d0<<left | d1>>right;
+					d0 = d0 >> right | d1 << left;
 				}
 				d0 = fb_rev_pixels_in_long(d0, bswapmask);
 				FB_WRITEL(comp(d0, FB_READL(dst), last), dst);
@@ -202,43 +209,46 @@ bitcpy(struct fb_info *p, unsigned long
      */
 
 static void
-bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
-		const unsigned long __iomem *src, int src_idx, int bits,
+bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, unsigned dst_idx,
+		const unsigned long __iomem *src, unsigned src_idx, int bits,
 		unsigned n, u32 bswapmask)
 {
 	unsigned long first, last;
 	int shift;
 
-	dst += (n-1)/bits;
-	src += (n-1)/bits;
-	if ((n-1) % bits) {
-		dst_idx += (n-1) % bits;
-		dst += dst_idx >> (ffs(bits) - 1);
-		dst_idx &= bits - 1;
-		src_idx += (n-1) % bits;
-		src += src_idx >> (ffs(bits) - 1);
-		src_idx &= bits - 1;
-	}
+#if 0
+	/*
+	 * If you suspect bug in this function, compare it with this simple
+	 * memmove implementation.
+	 */
+	fb_memmove((char *)dst + ((dst_idx & (bits - 1))) / 8,
+		   (char *)src + ((src_idx & (bits - 1))) / 8, n / 8);
+	return;
+#endif
+
+	dst += (dst_idx + n - 1) / bits;
+	src += (src_idx + n - 1) / bits;
+	dst_idx = (dst_idx + n - 1) % bits;
+	src_idx = (src_idx + n - 1) % bits;
 
 	shift = dst_idx-src_idx;
 
-	first = fb_shifted_pixels_mask_long(p, bits - 1 - dst_idx, bswapmask);
-	last = ~fb_shifted_pixels_mask_long(p, bits - 1 - ((dst_idx-n) % bits),
-					    bswapmask);
+	first = ~fb_shifted_pixels_mask_long(p, (dst_idx + 1) % bits, bswapmask);
+	last = fb_shifted_pixels_mask_long(p, (bits + dst_idx + 1 - n) % bits, bswapmask);
 
 	if (!shift) {
 		// Same alignment for source and dest
 
 		if ((unsigned long)dst_idx+1 >= n) {
 			// Single word
-			if (last)
-				first &= last;
-			FB_WRITEL( comp( FB_READL(src), FB_READL(dst), first), dst);
+			if (first)
+				last &= first;
+			FB_WRITEL( comp( FB_READL(src), FB_READL(dst), last), dst);
 		} else {
 			// Multiple destination words
 
 			// Leading bits
-			if (first != ~0UL) {
+			if (first) {
 				FB_WRITEL( comp( FB_READL(src), FB_READL(dst), first), dst);
 				dst--;
 				src--;
@@ -262,7 +272,7 @@ bitcpy_rev(struct fb_info *p, unsigned l
 				FB_WRITEL(FB_READL(src--), dst--);
 
 			// Trailing bits
-			if (last)
+			if (last != -1UL)
 				FB_WRITEL( comp( FB_READL(src), FB_READL(dst), last), dst);
 		}
 	} else {
@@ -270,29 +280,28 @@ bitcpy_rev(struct fb_info *p, unsigned l
 		unsigned long d0, d1;
 		int m;
 
-		int const left = -shift & (bits-1);
-		int const right = shift & (bits-1);
-		bswapmask &= shift;
+		int const left = shift & (bits-1);
+		int const right = -shift & (bits-1);
 
 		if ((unsigned long)dst_idx+1 >= n) {
 			// Single destination word
-			if (last)
-				first &= last;
+			if (first)
+				last &= first;
 			d0 = FB_READL(src);
 			if (shift < 0) {
 				// Single source word
-				d0 <<= left;
+				d0 >>= right;
 			} else if (1+(unsigned long)src_idx >= n) {
 				// Single source word
-				d0 >>= right;
+				d0 <<= left;
 			} else {
 				// 2 source words
 				d1 = FB_READL(src - 1);
 				d1 = fb_rev_pixels_in_long(d1, bswapmask);
-				d0 = d0>>right | d1<<left;
+				d0 = d0 << left | d1 >> right;
 			}
 			d0 = fb_rev_pixels_in_long(d0, bswapmask);
-			FB_WRITEL(comp(d0, FB_READL(dst), first), dst);
+			FB_WRITEL(comp(d0, FB_READL(dst), last), dst);
 		} else {
 			// Multiple destination words
 			/** We must always remember the last value read, because in case
@@ -307,12 +316,12 @@ bitcpy_rev(struct fb_info *p, unsigned l
 			if (shift < 0) {
 				// Single source word
 				d1 = d0;
-				d0 <<= left;
+				d0 >>= right;
 			} else {
 				// 2 source words
 				d1 = FB_READL(src--);
 				d1 = fb_rev_pixels_in_long(d1, bswapmask);
-				d0 = d0>>right | d1<<left;
+				d0 = d0 << left | d1 >> right;
 			}
 			d0 = fb_rev_pixels_in_long(d0, bswapmask);
 			FB_WRITEL(comp(d0, FB_READL(dst), first), dst);
@@ -325,39 +334,39 @@ bitcpy_rev(struct fb_info *p, unsigned l
 			n /= bits;
 			while ((n >= 4) && !bswapmask) {
 				d1 = FB_READL(src--);
-				FB_WRITEL(d0 >> right | d1 << left, dst--);
+				FB_WRITEL(d0 << left | d1 >> right, dst--);
 				d0 = d1;
 				d1 = FB_READL(src--);
-				FB_WRITEL(d0 >> right | d1 << left, dst--);
+				FB_WRITEL(d0 << left | d1 >> right, dst--);
 				d0 = d1;
 				d1 = FB_READL(src--);
-				FB_WRITEL(d0 >> right | d1 << left, dst--);
+				FB_WRITEL(d0 << left | d1 >> right, dst--);
 				d0 = d1;
 				d1 = FB_READL(src--);
-				FB_WRITEL(d0 >> right | d1 << left, dst--);
+				FB_WRITEL(d0 << left | d1 >> right, dst--);
 				d0 = d1;
 				n -= 4;
 			}
 			while (n--) {
 				d1 = FB_READL(src--);
 				d1 = fb_rev_pixels_in_long(d1, bswapmask);
-				d0 = d0 >> right | d1 << left;
+				d0 = d0 << left | d1 >> right;
 				d0 = fb_rev_pixels_in_long(d0, bswapmask);
 				FB_WRITEL(d0, dst--);
 				d0 = d1;
 			}
 
 			// Trailing bits
-			if (last) {
-				if (m <= left) {
+			if (m) {
+				if (m <= bits - left) {
 					// Single source word
-					d0 >>= right;
+					d0 <<= left;
 				} else {
 					// 2 source words
 					d1 = FB_READL(src);
 					d1 = fb_rev_pixels_in_long(d1,
 								bswapmask);
-					d0 = d0>>right | d1<<left;
+					d0 = d0 << left | d1 >> right;
 				}
 				d0 = fb_rev_pixels_in_long(d0, bswapmask);
 				FB_WRITEL(comp(d0, FB_READL(dst), last), dst);
@@ -371,9 +380,9 @@ void cfb_copyarea(struct fb_info *p, con
 	u32 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy;
 	u32 height = area->height, width = area->width;
 	unsigned long const bits_per_line = p->fix.line_length*8u;
-	unsigned long __iomem *dst = NULL, *src = NULL;
+	unsigned long __iomem *base = NULL;
 	int bits = BITS_PER_LONG, bytes = bits >> 3;
-	int dst_idx = 0, src_idx = 0, rev_copy = 0;
+	unsigned dst_idx = 0, src_idx = 0, rev_copy = 0;
 	u32 bswapmask = fb_compute_bswapmask(p);
 
 	if (p->state != FBINFO_STATE_RUNNING)
@@ -389,7 +398,7 @@ void cfb_copyarea(struct fb_info *p, con
 
 	// split the base of the framebuffer into a long-aligned address and the
 	// index of the first bit
-	dst = src = (unsigned long __iomem *)((unsigned long)p->screen_base & ~(bytes-1));
+	base = (unsigned long __iomem *)((unsigned long)p->screen_base & ~(bytes-1));
 	dst_idx = src_idx = 8*((unsigned long)p->screen_base & (bytes-1));
 	// add offset of source and target area
 	dst_idx += dy*bits_per_line + dx*p->var.bits_per_pixel;
@@ -402,20 +411,14 @@ void cfb_copyarea(struct fb_info *p, con
 		while (height--) {
 			dst_idx -= bits_per_line;
 			src_idx -= bits_per_line;
-			dst += dst_idx >> (ffs(bits) - 1);
-			dst_idx &= (bytes - 1);
-			src += src_idx >> (ffs(bits) - 1);
-			src_idx &= (bytes - 1);
-			bitcpy_rev(p, dst, dst_idx, src, src_idx, bits,
+			bitcpy_rev(p, base + (dst_idx / bits), dst_idx % bits,
+				base + (src_idx / bits), src_idx % bits, bits,
 				width*p->var.bits_per_pixel, bswapmask);
 		}
 	} else {
 		while (height--) {
-			dst += dst_idx >> (ffs(bits) - 1);
-			dst_idx &= (bytes - 1);
-			src += src_idx >> (ffs(bits) - 1);
-			src_idx &= (bytes - 1);
-			bitcpy(p, dst, dst_idx, src, src_idx, bits,
+			bitcpy(p, base + (dst_idx / bits), dst_idx % bits,
+				base + (src_idx / bits), src_idx % bits, bits,
 				width*p->var.bits_per_pixel, bswapmask);
 			dst_idx += bits_per_line;
 			src_idx += bits_per_line;


^ permalink raw reply

* [PATCH 2/7] aty: remove mode restriction
From: Mikulas Patocka @ 2012-08-20 18:06 UTC (permalink / raw)
  To: linux-fbdev

aty: remove mode restriction

The mach64 video card works fine in 1920x1080 resolution,
there's no need to limit it.

Signed-off-by: Mikulas Patocka <mpatocka@artax.karlin.mff.cuni.cz>

---
 drivers/video/aty/atyfb_base.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-3.6-rc2-fast/drivers/video/aty/atyfb_base.c
=================================--- linux-3.6-rc2-fast.orig/drivers/video/aty/atyfb_base.c	2012-08-18 04:07:55.000000000 +0200
+++ linux-3.6-rc2-fast/drivers/video/aty/atyfb_base.c	2012-08-18 04:09:09.000000000 +0200
@@ -861,8 +861,8 @@ static int aty_var_to_crtc(const struct 
 	h_sync_pol = sync & FB_SYNC_HOR_HIGH_ACT ? 0 : 1;
 	v_sync_pol = sync & FB_SYNC_VERT_HIGH_ACT ? 0 : 1;
 
-	if ((xres > 1600) || (yres > 1200)) {
-		FAIL("MACH64 chips are designed for max 1600x1200\n"
+	if ((xres > 1920) || (yres > 1200)) {
+		FAIL("MACH64 chips are designed for max 1920x1200\n"
 		     "select another resolution.");
 	}
 	h_sync_strt = h_disp + var->right_margin;


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox