Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 10/20] OMAPDSS: Resolve channels for outputs
From: Archit Taneja @ 2013-03-11 12:31 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513DC870.3030908@ti.com>

On Monday 11 March 2013 05:35 PM, Tomi Valkeinen wrote:
> On 2013-03-11 07:35, Archit Taneja wrote:
>> Hi,
>>
>> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>>> The DISPC channel used for each output is currently passed in panel
>>> platform data from the board files.
>>>
>>> To simplify this, and to make the panel drivers less dependent on OMAP,
>>> this patch changes omapdss to resolve the channel independently. The
>>> channel is resolved based on the OMAP version and, in case of DSI, the
>>> DSI module id.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> ---
>>>    drivers/video/omap2/dss/dpi.c  |   37 ++++++++++++++++++++++++++-----
>>>    drivers/video/omap2/dss/dsi.c  |   48
>>> ++++++++++++++++++++++++++++++++++++++++
>>>    drivers/video/omap2/dss/rfbi.c |    2 ++
>>>    drivers/video/omap2/dss/sdi.c  |    2 ++
>>>    4 files changed, 84 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/dpi.c
>>> b/drivers/video/omap2/dss/dpi.c
>>> index e282456..3261644 100644
>>> --- a/drivers/video/omap2/dss/dpi.c
>>> +++ b/drivers/video/omap2/dss/dpi.c
>>> @@ -396,12 +396,44 @@ static int __init dpi_verify_dsi_pll(struct
>>> platform_device *dsidev)
>>>        return 0;
>>>    }
>>>
>>> +/*
>>> + * Return a hardcoded channel for the DPI output. This should work for
>>> + * current use cases, but this can be later expanded to either resolve
>>> + * the channel in some more dynamic manner, or get the channel as a user
>>> + * parameter.
>>> + */
>>> +static enum omap_channel dpi_get_channel(void)
>>> +{
>>> +    switch (omapdss_get_version()) {
>>> +    case OMAPDSS_VER_OMAP24xx:
>>> +    case OMAPDSS_VER_OMAP34xx_ES1:
>>> +    case OMAPDSS_VER_OMAP34xx_ES3:
>>> +    case OMAPDSS_VER_OMAP3630:
>>> +    case OMAPDSS_VER_AM35xx:
>>> +        return OMAP_DSS_CHANNEL_LCD;
>>> +
>>> +    case OMAPDSS_VER_OMAP4430_ES1:
>>> +    case OMAPDSS_VER_OMAP4430_ES2:
>>> +    case OMAPDSS_VER_OMAP4:
>>> +        return OMAP_DSS_CHANNEL_LCD2;
>>> +
>>> +    case OMAPDSS_VER_OMAP5:
>>> +        return OMAP_DSS_CHANNEL_LCD2;
>>> +
>>> +    default:
>>> +        DSSWARN("unsupported DSS version\n");
>>> +        return OMAP_DSS_CHANNEL_LCD;
>>> +    }
>>> +}
>>> +
>>>    static int __init dpi_init_display(struct omap_dss_device *dssdev)
>>>    {
>>>        struct platform_device *dsidev;
>>>
>>>        DSSDBG("init_display\n");
>>>
>>> +    dssdev->channel = dpi_get_channel();
>>
>> In patch 14 of the series, we remove these dssdev->channel assignments.
>> I don't see the point of adding them in this patch in the first place.
>> The dssdev->channel assignments will not be modified in this series, so
>> we don't need to worry about a kernel crash or something after this patch.
>>
>> I feel this patch should only add the dpi_get_channel and
>> dsi_get_channel funcs.
>
> Here's the combined patch. I changed the recommended_channel to dispc_channel.
> "recommended" was not that good name, as currently the channel cannot be changed
> later, because, for example in DPI case, we store the DSI PLL which is based on
> the channel.

The patch looks fine now.

Reviewed-by: Archit Taneja <archit@ti.com>

Archit

>
>
> Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Date:   Wed Feb 13 11:23:54 2013 +0200
>
>      OMAPDSS: add output->dispc_channel
>
>      The DISPC channel used for each output is currently passed in panel
>      platform data from the board files.
>
>      To simplify this, and to make the panel drivers less dependent on OMAP,
>      this patch changes omapdss to resolve the channel independently. The
>      channel is resolved based on the OMAP version and, in case of DSI, the
>      DSI module id. This resolved channel is stored into a new field in
>      output, dispc_channel.
>
>      The few places where dssdev->channel was used are changed to use
>      output->recommended_channel. After this patch, dssdev->channel is
>      obsolete.
>
>      Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
> index e282456..409e53b 100644
> --- a/drivers/video/omap2/dss/dpi.c
> +++ b/drivers/video/omap2/dss/dpi.c
> @@ -396,6 +396,36 @@ static int __init dpi_verify_dsi_pll(struct platform_device *dsidev)
>   	return 0;
>   }
>
> +/*
> + * Return a hardcoded channel for the DPI output. This should work for
> + * current use cases, but this can be later expanded to either resolve
> + * the channel in some more dynamic manner, or get the channel as a user
> + * parameter.
> + */
> +static enum omap_channel dpi_get_channel(void)
> +{
> +	switch (omapdss_get_version()) {
> +	case OMAPDSS_VER_OMAP24xx:
> +	case OMAPDSS_VER_OMAP34xx_ES1:
> +	case OMAPDSS_VER_OMAP34xx_ES3:
> +	case OMAPDSS_VER_OMAP3630:
> +	case OMAPDSS_VER_AM35xx:
> +		return OMAP_DSS_CHANNEL_LCD;
> +
> +	case OMAPDSS_VER_OMAP4430_ES1:
> +	case OMAPDSS_VER_OMAP4430_ES2:
> +	case OMAPDSS_VER_OMAP4:
> +		return OMAP_DSS_CHANNEL_LCD2;
> +
> +	case OMAPDSS_VER_OMAP5:
> +		return OMAP_DSS_CHANNEL_LCD3;
> +
> +	default:
> +		DSSWARN("unsupported DSS version\n");
> +		return OMAP_DSS_CHANNEL_LCD;
> +	}
> +}
> +
>   static int __init dpi_init_display(struct omap_dss_device *dssdev)
>   {
>   	struct platform_device *dsidev;
> @@ -416,12 +446,7 @@ static int __init dpi_init_display(struct omap_dss_device *dssdev)
>   		dpi.vdds_dsi_reg = vdds_dsi;
>   	}
>
> -	/*
> -	 * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
> -	 * source for DPI is SoC integration detail, not something that should
> -	 * be configured in the dssdev
> -	 */
> -	dsidev = dpi_get_dsidev(dssdev->channel);
> +	dsidev = dpi_get_dsidev(dpi.output.dispc_channel);
>
>   	if (dsidev && dpi_verify_dsi_pll(dsidev)) {
>   		dsidev = NULL;
> @@ -513,6 +538,7 @@ static void __init dpi_init_output(struct platform_device *pdev)
>   	out->id = OMAP_DSS_OUTPUT_DPI;
>   	out->type = OMAP_DISPLAY_TYPE_DPI;
>   	out->name = "dpi";
> +	out->dispc_channel = dpi_get_channel();
>
>   	dss_register_output(out);
>   }
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 1a6ad6f..28e0b99 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -4946,6 +4946,55 @@ void omapdss_dsi_set_videomode_timings(struct omap_dss_device *dssdev,
>   }
>   EXPORT_SYMBOL(omapdss_dsi_set_videomode_timings);
>
> +/*
> + * Return a hardcoded channel for the DSI output. This should work for
> + * current use cases, but this can be later expanded to either resolve
> + * the channel in some more dynamic manner, or get the channel as a user
> + * parameter.
> + */
> +static enum omap_channel dsi_get_channel(int module_id)
> +{
> +	switch (omapdss_get_version()) {
> +	case OMAPDSS_VER_OMAP24xx:
> +		DSSWARN("DSI not supported\n");
> +		return OMAP_DSS_CHANNEL_LCD;
> +
> +	case OMAPDSS_VER_OMAP34xx_ES1:
> +	case OMAPDSS_VER_OMAP34xx_ES3:
> +	case OMAPDSS_VER_OMAP3630:
> +	case OMAPDSS_VER_AM35xx:
> +		return OMAP_DSS_CHANNEL_LCD;
> +
> +	case OMAPDSS_VER_OMAP4430_ES1:
> +	case OMAPDSS_VER_OMAP4430_ES2:
> +	case OMAPDSS_VER_OMAP4:
> +		switch (module_id) {
> +		case 0:
> +			return OMAP_DSS_CHANNEL_LCD;
> +		case 1:
> +			return OMAP_DSS_CHANNEL_LCD2;
> +		default:
> +			DSSWARN("unsupported module id\n");
> +			return OMAP_DSS_CHANNEL_LCD;
> +		}
> +
> +	case OMAPDSS_VER_OMAP5:
> +		switch (module_id) {
> +		case 0:
> +			return OMAP_DSS_CHANNEL_LCD;
> +		case 1:
> +			return OMAP_DSS_CHANNEL_LCD3;
> +		default:
> +			DSSWARN("unsupported module id\n");
> +			return OMAP_DSS_CHANNEL_LCD;
> +		}
> +
> +	default:
> +		DSSWARN("unsupported DSS version\n");
> +		return OMAP_DSS_CHANNEL_LCD;
> +	}
> +}
> +
>   static int __init dsi_init_display(struct omap_dss_device *dssdev)
>   {
>   	struct platform_device *dsidev > @@ -5184,6 +5233,7 @@ static void __init dsi_init_output(struct platform_device *dsidev)
>
>   	out->type = OMAP_DISPLAY_TYPE_DSI;
>   	out->name = dsi->module_id = 0 ? "dsi0" : "dsi1";
> +	out->dispc_channel = dsi_get_channel(dsi->module_id);
>
>   	dss_register_output(out);
>   }
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 888cfe3..e03619a 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -1012,8 +1012,6 @@ static void __init hdmi_probe_pdata(struct platform_device *pdev)
>   	hdmi.ls_oe_gpio = priv->ls_oe_gpio;
>   	hdmi.hpd_gpio = priv->hpd_gpio;
>
> -	dssdev->channel = OMAP_DSS_CHANNEL_DIGIT;
> -
>   	r = hdmi_init_display(dssdev);
>   	if (r) {
>   		DSSERR("device %s init failed: %d\n", dssdev->name, r);
> @@ -1047,6 +1045,7 @@ static void __init hdmi_init_output(struct platform_device *pdev)
>   	out->id = OMAP_DSS_OUTPUT_HDMI;
>   	out->type = OMAP_DISPLAY_TYPE_HDMI;
>   	out->name = "hdmi";
> +	out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT;
>
>   	dss_register_output(out);
>   }
> diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
> index a47a9e5..05c0646 100644
> --- a/drivers/video/omap2/dss/rfbi.c
> +++ b/drivers/video/omap2/dss/rfbi.c
> @@ -1026,6 +1026,7 @@ static void __init rfbi_init_output(struct platform_device *pdev)
>   	out->id = OMAP_DSS_OUTPUT_DBI;
>   	out->type = OMAP_DISPLAY_TYPE_DBI;
>   	out->name = "rfbi";
> +	out->dispc_channel = OMAP_DSS_CHANNEL_LCD;
>
>   	dss_register_output(out);
>   }
> diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
> index 0802927..5d37db5 100644
> --- a/drivers/video/omap2/dss/sdi.c
> +++ b/drivers/video/omap2/dss/sdi.c
> @@ -279,6 +279,7 @@ static void __init sdi_init_output(struct platform_device *pdev)
>   	out->id = OMAP_DSS_OUTPUT_SDI;
>   	out->type = OMAP_DISPLAY_TYPE_SDI;
>   	out->name = "sdi";
> +	out->dispc_channel = OMAP_DSS_CHANNEL_LCD;
>
>   	dss_register_output(out);
>   }
> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
> index c8130f8..866e015 100644
> --- a/drivers/video/omap2/dss/venc.c
> +++ b/drivers/video/omap2/dss/venc.c
> @@ -786,8 +786,6 @@ static void __init venc_probe_pdata(struct platform_device *vencdev)
>
>   	dss_copy_device_pdata(dssdev, plat_dssdev);
>
> -	dssdev->channel = OMAP_DSS_CHANNEL_DIGIT;
> -
>   	r = venc_init_display(dssdev);
>   	if (r) {
>   		DSSERR("device %s init failed: %d\n", dssdev->name, r);
> @@ -820,6 +818,7 @@ static void __init venc_init_output(struct platform_device *pdev)
>   	out->id = OMAP_DSS_OUTPUT_VENC;
>   	out->type = OMAP_DISPLAY_TYPE_VENC;
>   	out->name = "venc";
> +	out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT;
>
>   	dss_register_output(out);
>   }
> diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c
> index ca585ef..f38348e 100644
> --- a/drivers/video/omap2/omapfb/omapfb-main.c
> +++ b/drivers/video/omap2/omapfb/omapfb-main.c
> @@ -2388,7 +2388,7 @@ static int omapfb_init_connections(struct omapfb2_device *fbdev,
>   		struct omap_dss_device *dssdev = fbdev->displays[i].dssdev;
>   		struct omap_dss_output *out = dssdev->output;
>
> -		mgr = omap_dss_get_overlay_manager(dssdev->channel);
> +		mgr = omap_dss_get_overlay_manager(out->dispc_channel);
>
>   		if (!mgr || !out)
>   			continue;
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index ba9cea7..ec322a7 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -547,6 +547,9 @@ struct omap_dss_output {
>   	/* display type supported by the output */
>   	enum omap_display_type type;
>
> +	/* DISPC channel for this output */
> +	enum omap_channel dispc_channel;
> +
>   	/* output instance */
>   	enum omap_dss_output_id id;
>
>
>


^ permalink raw reply

* Re: [PATCH 2/2] omapfb: fix broken build on Amstrad E3/Delta
From: Tomi Valkeinen @ 2013-03-11 12:22 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Aaro Koskinen, linux-fbdev, linux-omap
In-Reply-To: <20130304223911.GZ11806@atomide.com>

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

On 2013-03-05 00:39, Tony Lindgren wrote:
> Hi,
> 
> * Aaro Koskinen <aaro.koskinen@iki.fi> [130304 13:08]:
>> Fix the following build regression in 3.9-rc1 by including
>> <machine/hardware.h>:
>>
>> drivers/video/omap/lcd_ams_delta.c: In function 'ams_delta_lcd_set_power':
>> drivers/video/omap/lcd_ams_delta.c:48:4: error: implicit declaration of function 'omap_writeb' [-Werror=implicit-function-declaration]
>> drivers/video/omap/lcd_ams_delta.c:49:6: error: 'OMAP_PWL_ENABLE' undeclared (first use in this function)
>> drivers/video/omap/lcd_ams_delta.c:49:6: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/video/omap/lcd_ams_delta.c:50:19: error: 'OMAP_PWL_CLK_ENABLE' undeclared (first use in this function)
>> drivers/video/omap/lcd_ams_delta.c: In function 'ams_delta_lcd_set_contrast':
>> drivers/video/omap/lcd_ams_delta.c:66:22: error: 'OMAP_PWL_ENABLE' undeclared (first use in this function)
> 
> I have this already queued up in omap-for-v3.9-rc1/fixes as 0adcbaf78
> (ARM: OMAP1: Fix build related to kgdb.h no longer including serial_8250.h)
> and for lcd_osk.c as omap1_defconfig was broken.
> 
> I don't anything for the first patch you posted, so I suggest
> Tomi applies that one.

Ok, I'll take the first patch.

 Tomi




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

^ permalink raw reply

* Re: [PATCH 10/20] OMAPDSS: Resolve channels for outputs
From: Tomi Valkeinen @ 2013-03-11 12:05 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513D6D06.7030902@ti.com>

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

On 2013-03-11 07:35, Archit Taneja wrote:
> Hi,
> 
> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>> The DISPC channel used for each output is currently passed in panel
>> platform data from the board files.
>>
>> To simplify this, and to make the panel drivers less dependent on OMAP,
>> this patch changes omapdss to resolve the channel independently. The
>> channel is resolved based on the OMAP version and, in case of DSI, the
>> DSI module id.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/video/omap2/dss/dpi.c  |   37 ++++++++++++++++++++++++++-----
>>   drivers/video/omap2/dss/dsi.c  |   48
>> ++++++++++++++++++++++++++++++++++++++++
>>   drivers/video/omap2/dss/rfbi.c |    2 ++
>>   drivers/video/omap2/dss/sdi.c  |    2 ++
>>   4 files changed, 84 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dpi.c
>> b/drivers/video/omap2/dss/dpi.c
>> index e282456..3261644 100644
>> --- a/drivers/video/omap2/dss/dpi.c
>> +++ b/drivers/video/omap2/dss/dpi.c
>> @@ -396,12 +396,44 @@ static int __init dpi_verify_dsi_pll(struct
>> platform_device *dsidev)
>>       return 0;
>>   }
>>
>> +/*
>> + * Return a hardcoded channel for the DPI output. This should work for
>> + * current use cases, but this can be later expanded to either resolve
>> + * the channel in some more dynamic manner, or get the channel as a user
>> + * parameter.
>> + */
>> +static enum omap_channel dpi_get_channel(void)
>> +{
>> +    switch (omapdss_get_version()) {
>> +    case OMAPDSS_VER_OMAP24xx:
>> +    case OMAPDSS_VER_OMAP34xx_ES1:
>> +    case OMAPDSS_VER_OMAP34xx_ES3:
>> +    case OMAPDSS_VER_OMAP3630:
>> +    case OMAPDSS_VER_AM35xx:
>> +        return OMAP_DSS_CHANNEL_LCD;
>> +
>> +    case OMAPDSS_VER_OMAP4430_ES1:
>> +    case OMAPDSS_VER_OMAP4430_ES2:
>> +    case OMAPDSS_VER_OMAP4:
>> +        return OMAP_DSS_CHANNEL_LCD2;
>> +
>> +    case OMAPDSS_VER_OMAP5:
>> +        return OMAP_DSS_CHANNEL_LCD2;
>> +
>> +    default:
>> +        DSSWARN("unsupported DSS version\n");
>> +        return OMAP_DSS_CHANNEL_LCD;
>> +    }
>> +}
>> +
>>   static int __init dpi_init_display(struct omap_dss_device *dssdev)
>>   {
>>       struct platform_device *dsidev;
>>
>>       DSSDBG("init_display\n");
>>
>> +    dssdev->channel = dpi_get_channel();
> 
> In patch 14 of the series, we remove these dssdev->channel assignments.
> I don't see the point of adding them in this patch in the first place.
> The dssdev->channel assignments will not be modified in this series, so
> we don't need to worry about a kernel crash or something after this patch.
> 
> I feel this patch should only add the dpi_get_channel and
> dsi_get_channel funcs.

Here's the combined patch. I changed the recommended_channel to dispc_channel.
"recommended" was not that good name, as currently the channel cannot be changed
later, because, for example in DPI case, we store the DSI PLL which is based on
the channel.


Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date:   Wed Feb 13 11:23:54 2013 +0200

    OMAPDSS: add output->dispc_channel
    
    The DISPC channel used for each output is currently passed in panel
    platform data from the board files.
    
    To simplify this, and to make the panel drivers less dependent on OMAP,
    this patch changes omapdss to resolve the channel independently. The
    channel is resolved based on the OMAP version and, in case of DSI, the
    DSI module id. This resolved channel is stored into a new field in
    output, dispc_channel.
    
    The few places where dssdev->channel was used are changed to use
    output->recommended_channel. After this patch, dssdev->channel is
    obsolete.
    
    Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index e282456..409e53b 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -396,6 +396,36 @@ static int __init dpi_verify_dsi_pll(struct platform_device *dsidev)
 	return 0;
 }
 
+/*
+ * Return a hardcoded channel for the DPI output. This should work for
+ * current use cases, but this can be later expanded to either resolve
+ * the channel in some more dynamic manner, or get the channel as a user
+ * parameter.
+ */
+static enum omap_channel dpi_get_channel(void)
+{
+	switch (omapdss_get_version()) {
+	case OMAPDSS_VER_OMAP24xx:
+	case OMAPDSS_VER_OMAP34xx_ES1:
+	case OMAPDSS_VER_OMAP34xx_ES3:
+	case OMAPDSS_VER_OMAP3630:
+	case OMAPDSS_VER_AM35xx:
+		return OMAP_DSS_CHANNEL_LCD;
+
+	case OMAPDSS_VER_OMAP4430_ES1:
+	case OMAPDSS_VER_OMAP4430_ES2:
+	case OMAPDSS_VER_OMAP4:
+		return OMAP_DSS_CHANNEL_LCD2;
+
+	case OMAPDSS_VER_OMAP5:
+		return OMAP_DSS_CHANNEL_LCD3;
+
+	default:
+		DSSWARN("unsupported DSS version\n");
+		return OMAP_DSS_CHANNEL_LCD;
+	}
+}
+
 static int __init dpi_init_display(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev;
@@ -416,12 +446,7 @@ static int __init dpi_init_display(struct omap_dss_device *dssdev)
 		dpi.vdds_dsi_reg = vdds_dsi;
 	}
 
-	/*
-	 * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
-	 * source for DPI is SoC integration detail, not something that should
-	 * be configured in the dssdev
-	 */
-	dsidev = dpi_get_dsidev(dssdev->channel);
+	dsidev = dpi_get_dsidev(dpi.output.dispc_channel);
 
 	if (dsidev && dpi_verify_dsi_pll(dsidev)) {
 		dsidev = NULL;
@@ -513,6 +538,7 @@ static void __init dpi_init_output(struct platform_device *pdev)
 	out->id = OMAP_DSS_OUTPUT_DPI;
 	out->type = OMAP_DISPLAY_TYPE_DPI;
 	out->name = "dpi";
+	out->dispc_channel = dpi_get_channel();
 
 	dss_register_output(out);
 }
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 1a6ad6f..28e0b99 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -4946,6 +4946,55 @@ void omapdss_dsi_set_videomode_timings(struct omap_dss_device *dssdev,
 }
 EXPORT_SYMBOL(omapdss_dsi_set_videomode_timings);
 
+/*
+ * Return a hardcoded channel for the DSI output. This should work for
+ * current use cases, but this can be later expanded to either resolve
+ * the channel in some more dynamic manner, or get the channel as a user
+ * parameter.
+ */
+static enum omap_channel dsi_get_channel(int module_id)
+{
+	switch (omapdss_get_version()) {
+	case OMAPDSS_VER_OMAP24xx:
+		DSSWARN("DSI not supported\n");
+		return OMAP_DSS_CHANNEL_LCD;
+
+	case OMAPDSS_VER_OMAP34xx_ES1:
+	case OMAPDSS_VER_OMAP34xx_ES3:
+	case OMAPDSS_VER_OMAP3630:
+	case OMAPDSS_VER_AM35xx:
+		return OMAP_DSS_CHANNEL_LCD;
+
+	case OMAPDSS_VER_OMAP4430_ES1:
+	case OMAPDSS_VER_OMAP4430_ES2:
+	case OMAPDSS_VER_OMAP4:
+		switch (module_id) {
+		case 0:
+			return OMAP_DSS_CHANNEL_LCD;
+		case 1:
+			return OMAP_DSS_CHANNEL_LCD2;
+		default:
+			DSSWARN("unsupported module id\n");
+			return OMAP_DSS_CHANNEL_LCD;
+		}
+
+	case OMAPDSS_VER_OMAP5:
+		switch (module_id) {
+		case 0:
+			return OMAP_DSS_CHANNEL_LCD;
+		case 1:
+			return OMAP_DSS_CHANNEL_LCD3;
+		default:
+			DSSWARN("unsupported module id\n");
+			return OMAP_DSS_CHANNEL_LCD;
+		}
+
+	default:
+		DSSWARN("unsupported DSS version\n");
+		return OMAP_DSS_CHANNEL_LCD;
+	}
+}
+
 static int __init dsi_init_display(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev =
@@ -5184,6 +5233,7 @@ static void __init dsi_init_output(struct platform_device *dsidev)
 
 	out->type = OMAP_DISPLAY_TYPE_DSI;
 	out->name = dsi->module_id == 0 ? "dsi0" : "dsi1";
+	out->dispc_channel = dsi_get_channel(dsi->module_id);
 
 	dss_register_output(out);
 }
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 888cfe3..e03619a 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -1012,8 +1012,6 @@ static void __init hdmi_probe_pdata(struct platform_device *pdev)
 	hdmi.ls_oe_gpio = priv->ls_oe_gpio;
 	hdmi.hpd_gpio = priv->hpd_gpio;
 
-	dssdev->channel = OMAP_DSS_CHANNEL_DIGIT;
-
 	r = hdmi_init_display(dssdev);
 	if (r) {
 		DSSERR("device %s init failed: %d\n", dssdev->name, r);
@@ -1047,6 +1045,7 @@ static void __init hdmi_init_output(struct platform_device *pdev)
 	out->id = OMAP_DSS_OUTPUT_HDMI;
 	out->type = OMAP_DISPLAY_TYPE_HDMI;
 	out->name = "hdmi";
+	out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT;
 
 	dss_register_output(out);
 }
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index a47a9e5..05c0646 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -1026,6 +1026,7 @@ static void __init rfbi_init_output(struct platform_device *pdev)
 	out->id = OMAP_DSS_OUTPUT_DBI;
 	out->type = OMAP_DISPLAY_TYPE_DBI;
 	out->name = "rfbi";
+	out->dispc_channel = OMAP_DSS_CHANNEL_LCD;
 
 	dss_register_output(out);
 }
diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
index 0802927..5d37db5 100644
--- a/drivers/video/omap2/dss/sdi.c
+++ b/drivers/video/omap2/dss/sdi.c
@@ -279,6 +279,7 @@ static void __init sdi_init_output(struct platform_device *pdev)
 	out->id = OMAP_DSS_OUTPUT_SDI;
 	out->type = OMAP_DISPLAY_TYPE_SDI;
 	out->name = "sdi";
+	out->dispc_channel = OMAP_DSS_CHANNEL_LCD;
 
 	dss_register_output(out);
 }
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index c8130f8..866e015 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -786,8 +786,6 @@ static void __init venc_probe_pdata(struct platform_device *vencdev)
 
 	dss_copy_device_pdata(dssdev, plat_dssdev);
 
-	dssdev->channel = OMAP_DSS_CHANNEL_DIGIT;
-
 	r = venc_init_display(dssdev);
 	if (r) {
 		DSSERR("device %s init failed: %d\n", dssdev->name, r);
@@ -820,6 +818,7 @@ static void __init venc_init_output(struct platform_device *pdev)
 	out->id = OMAP_DSS_OUTPUT_VENC;
 	out->type = OMAP_DISPLAY_TYPE_VENC;
 	out->name = "venc";
+	out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT;
 
 	dss_register_output(out);
 }
diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c
index ca585ef..f38348e 100644
--- a/drivers/video/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/omap2/omapfb/omapfb-main.c
@@ -2388,7 +2388,7 @@ static int omapfb_init_connections(struct omapfb2_device *fbdev,
 		struct omap_dss_device *dssdev = fbdev->displays[i].dssdev;
 		struct omap_dss_output *out = dssdev->output;
 
-		mgr = omap_dss_get_overlay_manager(dssdev->channel);
+		mgr = omap_dss_get_overlay_manager(out->dispc_channel);
 
 		if (!mgr || !out)
 			continue;
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index ba9cea7..ec322a7 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -547,6 +547,9 @@ struct omap_dss_output {
 	/* display type supported by the output */
 	enum omap_display_type type;
 
+	/* DISPC channel for this output */
+	enum omap_channel dispc_channel;
+
 	/* output instance */
 	enum omap_dss_output_id id;
 



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

^ permalink raw reply related

* Re: [PATCH 10/20] OMAPDSS: Resolve channels for outputs
From: Tomi Valkeinen @ 2013-03-11 12:01 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513D716C.2050807@ti.com>

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

On 2013-03-11 07:53, Archit Taneja wrote:
> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>> The DISPC channel used for each output is currently passed in panel
>> platform data from the board files.
>>
>> To simplify this, and to make the panel drivers less dependent on OMAP,
>> this patch changes omapdss to resolve the channel independently. The
>> channel is resolved based on the OMAP version and, in case of DSI, the
>> DSI module id.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/video/omap2/dss/dpi.c  |   37 ++++++++++++++++++++++++++-----
>>   drivers/video/omap2/dss/dsi.c  |   48
>> ++++++++++++++++++++++++++++++++++++++++
>>   drivers/video/omap2/dss/rfbi.c |    2 ++
>>   drivers/video/omap2/dss/sdi.c  |    2 ++
>>   4 files changed, 84 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dpi.c
>> b/drivers/video/omap2/dss/dpi.c
>> index e282456..3261644 100644
>> --- a/drivers/video/omap2/dss/dpi.c
>> +++ b/drivers/video/omap2/dss/dpi.c
>> @@ -396,12 +396,44 @@ static int __init dpi_verify_dsi_pll(struct
>> platform_device *dsidev)
>>       return 0;
>>   }
>>
>> +/*
>> + * Return a hardcoded channel for the DPI output. This should work for
>> + * current use cases, but this can be later expanded to either resolve
>> + * the channel in some more dynamic manner, or get the channel as a user
>> + * parameter.
>> + */
>> +static enum omap_channel dpi_get_channel(void)
>> +{
>> +    switch (omapdss_get_version()) {
>> +    case OMAPDSS_VER_OMAP24xx:
>> +    case OMAPDSS_VER_OMAP34xx_ES1:
>> +    case OMAPDSS_VER_OMAP34xx_ES3:
>> +    case OMAPDSS_VER_OMAP3630:
>> +    case OMAPDSS_VER_AM35xx:
>> +        return OMAP_DSS_CHANNEL_LCD;
>> +
>> +    case OMAPDSS_VER_OMAP4430_ES1:
>> +    case OMAPDSS_VER_OMAP4430_ES2:
>> +    case OMAPDSS_VER_OMAP4:
>> +        return OMAP_DSS_CHANNEL_LCD2;
>> +
>> +    case OMAPDSS_VER_OMAP5:
>> +        return OMAP_DSS_CHANNEL_LCD2;
>> +
>> +    default:
>> +        DSSWARN("unsupported DSS version\n");
>> +        return OMAP_DSS_CHANNEL_LCD;
>> +    }
>> +}
> 
> I had another comment for this patch. On OMAP5, it makes sense for us to
> not use LCD2 as the recommended channel. LCD2_CLK's only source is
> DSS_CLK from PRCM. So it's not a very flexible channel to use. We could
> use LCD3 (at the cost of not using DSI2).
> 
> We also need to fix dpi_get_dsidev() for OMAP5. Currently, it assumes
> that LCD2_CLK can be sourced from DSI2 PLL, we need to ensure DPI has a
> dsidev only if it's LCD1 or LCD3.

I fixed it this way:


Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date:   Mon Mar 11 13:57:38 2013 +0200

    OMAPDSS: DPI: fix dpi_get_dsidev() for omap5
    
    On OMAP5 the DISPC channels and DSI PLLs are not connected the same way.
    LCD2 on OMAP5 cannot use any DSI PLL as a source clock, but LCD3 can use
    DSI2's PLL.
    
    This patch fixes dpi_get_dsidev() by adding separate case for OMAP5 to
    handle the difference.
    
    Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 409e53b..433eff7 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -63,15 +63,29 @@ static struct platform_device *dpi_get_dsidev(enum omap_channel channel)
 	case OMAPDSS_VER_OMAP3630:
 	case OMAPDSS_VER_AM35xx:
 		return NULL;
-	default:
-		break;
-	}
 
-	switch (channel) {
-	case OMAP_DSS_CHANNEL_LCD:
-		return dsi_get_dsidev_from_id(0);
-	case OMAP_DSS_CHANNEL_LCD2:
-		return dsi_get_dsidev_from_id(1);
+	case OMAPDSS_VER_OMAP4430_ES1:
+	case OMAPDSS_VER_OMAP4430_ES2:
+	case OMAPDSS_VER_OMAP4:
+		switch (channel) {
+		case OMAP_DSS_CHANNEL_LCD:
+			return dsi_get_dsidev_from_id(0);
+		case OMAP_DSS_CHANNEL_LCD2:
+			return dsi_get_dsidev_from_id(1);
+		default:
+			return NULL;
+		}
+
+	case OMAPDSS_VER_OMAP5:
+		switch (channel) {
+		case OMAP_DSS_CHANNEL_LCD:
+			return dsi_get_dsidev_from_id(0);
+		case OMAP_DSS_CHANNEL_LCD3:
+			return dsi_get_dsidev_from_id(1);
+		default:
+			return NULL;
+		}
+
 	default:
 		return NULL;
 	}



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

^ permalink raw reply related

* Re: [PATCH 10/20] OMAPDSS: Resolve channels for outputs
From: Tomi Valkeinen @ 2013-03-11 11:54 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513D716C.2050807@ti.com>

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

On 2013-03-11 07:53, Archit Taneja wrote:
> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>> The DISPC channel used for each output is currently passed in panel
>> platform data from the board files.
>>
>> To simplify this, and to make the panel drivers less dependent on OMAP,
>> this patch changes omapdss to resolve the channel independently. The
>> channel is resolved based on the OMAP version and, in case of DSI, the
>> DSI module id.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/video/omap2/dss/dpi.c  |   37 ++++++++++++++++++++++++++-----
>>   drivers/video/omap2/dss/dsi.c  |   48
>> ++++++++++++++++++++++++++++++++++++++++
>>   drivers/video/omap2/dss/rfbi.c |    2 ++
>>   drivers/video/omap2/dss/sdi.c  |    2 ++
>>   4 files changed, 84 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dpi.c
>> b/drivers/video/omap2/dss/dpi.c
>> index e282456..3261644 100644
>> --- a/drivers/video/omap2/dss/dpi.c
>> +++ b/drivers/video/omap2/dss/dpi.c
>> @@ -396,12 +396,44 @@ static int __init dpi_verify_dsi_pll(struct
>> platform_device *dsidev)
>>       return 0;
>>   }
>>
>> +/*
>> + * Return a hardcoded channel for the DPI output. This should work for
>> + * current use cases, but this can be later expanded to either resolve
>> + * the channel in some more dynamic manner, or get the channel as a user
>> + * parameter.
>> + */
>> +static enum omap_channel dpi_get_channel(void)
>> +{
>> +    switch (omapdss_get_version()) {
>> +    case OMAPDSS_VER_OMAP24xx:
>> +    case OMAPDSS_VER_OMAP34xx_ES1:
>> +    case OMAPDSS_VER_OMAP34xx_ES3:
>> +    case OMAPDSS_VER_OMAP3630:
>> +    case OMAPDSS_VER_AM35xx:
>> +        return OMAP_DSS_CHANNEL_LCD;
>> +
>> +    case OMAPDSS_VER_OMAP4430_ES1:
>> +    case OMAPDSS_VER_OMAP4430_ES2:
>> +    case OMAPDSS_VER_OMAP4:
>> +        return OMAP_DSS_CHANNEL_LCD2;
>> +
>> +    case OMAPDSS_VER_OMAP5:
>> +        return OMAP_DSS_CHANNEL_LCD2;
>> +
>> +    default:
>> +        DSSWARN("unsupported DSS version\n");
>> +        return OMAP_DSS_CHANNEL_LCD;
>> +    }
>> +}
> 
> I had another comment for this patch. On OMAP5, it makes sense for us to
> not use LCD2 as the recommended channel. LCD2_CLK's only source is
> DSS_CLK from PRCM. So it's not a very flexible channel to use. We could
> use LCD3 (at the cost of not using DSI2).

Ok. Yes, this looks to be the case. I'll use LCD3 for DPI for now. In
the worst case we may need to pass some channel setup parameters via
omapdss DT data or platform data, but I'd really much like the driver to
be able to resolve the dispc channels by itself...

> We also need to fix dpi_get_dsidev() for OMAP5. Currently, it assumes
> that LCD2_CLK can be sourced from DSI2 PLL, we need to ensure DPI has a
> dsidev only if it's LCD1 or LCD3.

Right. I'll add if (OMAP5) there, and return DSI2 PLL for LCD3, and NULL
for LCD2.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 10/20] OMAPDSS: Resolve channels for outputs
From: Tomi Valkeinen @ 2013-03-11 11:02 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513D6D06.7030902@ti.com>

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

On 2013-03-11 07:35, Archit Taneja wrote:
> Hi,
> 
> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>> The DISPC channel used for each output is currently passed in panel
>> platform data from the board files.
>>
>> To simplify this, and to make the panel drivers less dependent on OMAP,
>> this patch changes omapdss to resolve the channel independently. The
>> channel is resolved based on the OMAP version and, in case of DSI, the
>> DSI module id.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/video/omap2/dss/dpi.c  |   37 ++++++++++++++++++++++++++-----
>>   drivers/video/omap2/dss/dsi.c  |   48
>> ++++++++++++++++++++++++++++++++++++++++
>>   drivers/video/omap2/dss/rfbi.c |    2 ++
>>   drivers/video/omap2/dss/sdi.c  |    2 ++
>>   4 files changed, 84 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dpi.c
>> b/drivers/video/omap2/dss/dpi.c
>> index e282456..3261644 100644
>> --- a/drivers/video/omap2/dss/dpi.c
>> +++ b/drivers/video/omap2/dss/dpi.c
>> @@ -396,12 +396,44 @@ static int __init dpi_verify_dsi_pll(struct
>> platform_device *dsidev)
>>       return 0;
>>   }
>>
>> +/*
>> + * Return a hardcoded channel for the DPI output. This should work for
>> + * current use cases, but this can be later expanded to either resolve
>> + * the channel in some more dynamic manner, or get the channel as a user
>> + * parameter.
>> + */
>> +static enum omap_channel dpi_get_channel(void)
>> +{
>> +    switch (omapdss_get_version()) {
>> +    case OMAPDSS_VER_OMAP24xx:
>> +    case OMAPDSS_VER_OMAP34xx_ES1:
>> +    case OMAPDSS_VER_OMAP34xx_ES3:
>> +    case OMAPDSS_VER_OMAP3630:
>> +    case OMAPDSS_VER_AM35xx:
>> +        return OMAP_DSS_CHANNEL_LCD;
>> +
>> +    case OMAPDSS_VER_OMAP4430_ES1:
>> +    case OMAPDSS_VER_OMAP4430_ES2:
>> +    case OMAPDSS_VER_OMAP4:
>> +        return OMAP_DSS_CHANNEL_LCD2;
>> +
>> +    case OMAPDSS_VER_OMAP5:
>> +        return OMAP_DSS_CHANNEL_LCD2;
>> +
>> +    default:
>> +        DSSWARN("unsupported DSS version\n");
>> +        return OMAP_DSS_CHANNEL_LCD;
>> +    }
>> +}
>> +
>>   static int __init dpi_init_display(struct omap_dss_device *dssdev)
>>   {
>>       struct platform_device *dsidev;
>>
>>       DSSDBG("init_display\n");
>>
>> +    dssdev->channel = dpi_get_channel();
> 
> In patch 14 of the series, we remove these dssdev->channel assignments.
> I don't see the point of adding them in this patch in the first place.
> The dssdev->channel assignments will not be modified in this series, so
> we don't need to worry about a kernel crash or something after this patch.
> 
> I feel this patch should only add the dpi_get_channel and
> dsi_get_channel funcs.

Yes, you're right. It's just extra going back and forth. I think I'll
merge this with the patch adding recommended channel.

>> +
>>       if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI) &&
>>                       dpi.vdds_dsi_reg == NULL) {
>>           struct regulator *vdds_dsi;
>> @@ -416,11 +448,6 @@ static int __init dpi_init_display(struct
>> omap_dss_device *dssdev)
>>           dpi.vdds_dsi_reg = vdds_dsi;
>>       }
>>
>> -    /*
>> -     * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
>> -     * source for DPI is SoC integration detail, not something that
>> should
>> -     * be configured in the dssdev
>> -     */
>>       dsidev = dpi_get_dsidev(dssdev->channel);
>>
>>       if (dsidev && dpi_verify_dsi_pll(dsidev)) {
>> diff --git a/drivers/video/omap2/dss/dsi.c
>> b/drivers/video/omap2/dss/dsi.c
>> index 1a6ad6f..c39ca86 100644
>> --- a/drivers/video/omap2/dss/dsi.c
>> +++ b/drivers/video/omap2/dss/dsi.c
>> @@ -4946,6 +4946,52 @@ void omapdss_dsi_set_videomode_timings(struct
>> omap_dss_device *dssdev,
>>   }
>>   EXPORT_SYMBOL(omapdss_dsi_set_videomode_timings);
>>
>> +/*
>> + * Return a hardcoded channel for the DSI output. This should work for
>> + * current use cases, but this can be later expanded to either resolve
>> + * the channel in some more dynamic manner, or get the channel as a user
>> + * parameter.
>> + */
>> +static enum omap_channel dsi_get_channel(int module_id)
>> +{
>> +    switch (omapdss_get_version()) {
>> +    case OMAPDSS_VER_OMAP24xx:
> 
> We should remove the above case so that we hit the default case and get
> a warning about omap2 not having DSI.

Yep, I'll fix that.

 Tomi




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

^ permalink raw reply

* Re: [PATCH v2 2/2] video: imxfb: Add DT support
From: Mark Rutland @ 2013-03-11 10:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1362504608-15839-3-git-send-email-mpa@pengutronix.de>

Hi,

Any reason for not CCing devicetree-discuss?

I have a couple of comments on the binding and the way it's parsed.

On Tue, Mar 05, 2013 at 05:30:08PM +0000, Markus Pargmann wrote:
> Add devicetree support for imx framebuffer driver. It uses the generic
> display bindings and helper functions.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> ---
>
> Notes:
>     Changes in v2:
>     - Removed pwmr register property
>     - Cleanup of devicetree binding documentation
>     - Use default values for pwmr and lscr1
>
>  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
>  drivers/video/imxfb.c                              | 182 +++++++++++++++++----
>  2 files changed, 197 insertions(+), 34 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
>
> diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> new file mode 100644
> index 0000000..e1a53a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> @@ -0,0 +1,49 @@
> +Freescale imx21 Framebuffer
> +
> +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> +
> +Required properties:
> +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> +- reg : Should contain 1 register ranges(address and length)
> +- interrupts : One interrupt of the fb dev
> +
> +Required nodes:
> +- display: Phandle to a display node as described in
> +       Documentation/devicetree/bindings/video/display-timing.txt
> +       Additional, the display node has to define properties:
> +       - bpp: Bits per pixel
> +       - pcr: LCDC PCR value

As these are non-standard, it would be good to prefix them (e.g. "fsl,pcr").

If you need them, why are they not a good fit for the generic binding?

I'm not familiar with the hardware, what is the PCR exactly?

[...]

> -static int __init imxfb_probe(struct platform_device *pdev)
> +static int imxfb_of_read_mode(struct device_node *np,
> +               struct imx_fb_videomode *imxfb_mode)
> +{
> +       int ret;
> +       struct fb_videomode *of_mode = &imxfb_mode->mode;
> +       u32 bpp;
> +       u32 pcr;
> +
> +       ret = of_property_read_string(np, "model", &of_mode->name);
> +       if (ret)
> +               of_mode->name = NULL;
> +
> +       ret = of_get_fb_videomode(np, of_mode, OF_USE_NATIVE_MODE);
> +       if (ret)
> +               return ret;
> +
> +       ret = of_property_read_u32(np, "bpp", &bpp);
> +       ret |= of_property_read_u32(np, "pcr", &pcr);
> +
> +       if (ret)
> +               return ret;

Is this return value used anywhere in anything more than an "if (!err)"
capacity?  If so it may be worth having individual return value checks:

If one call returns -EINVAL (-22) and the other -ENODATA (-61), out the other
end we'd get -EISDIR (-21). If we don't care particularly about which error
code we actually pass on, we could always return a sensible code when ret is
nonzero:

if (ret)
	return -EINVAL;

> +
> +       if (bpp > 255)
> +               return -EINVAL;

Might it also be worth checking for 0 here?

[...]

> @@ -837,15 +914,51 @@ static int __init imxfb_probe(struct platform_device *pdev)
>
>         fbi = info->par;
>
> -       if (!fb_mode)
> -               fb_mode = pdata->mode[0].mode.name;
> -
>         platform_set_drvdata(pdev, info);
>
>         ret = imxfb_init_fbinfo(pdev);
>         if (ret < 0)
>                 goto failed_init;
>
> +       if (pdata) {
> +               if (!fb_mode)
> +                       fb_mode = pdata->mode[0].mode.name;
> +
> +               fbi->mode = pdata->mode;
> +               fbi->num_modes = pdata->num_modes;
> +       } else {
> +               struct device_node *display_np;
> +               fb_mode = NULL;
> +
> +               display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> +               if (!display_np) {
> +                       dev_err(&pdev->dev, "No display defined in devicetree\n");
> +                       ret = -EINVAL;
> +                       goto failed_of_parse;
> +               }
> +
> +               /*
> +                * imxfb does not support more modes, we choose only the native
> +                * mode.
> +                */
> +               fbi->num_modes = 1;
> +
> +               fbi->mode = devm_kzalloc(&pdev->dev,
> +                               sizeof(struct imx_fb_videomode), GFP_KERNEL);
> +               if (!fbi->mode) {
> +                       ret = -ENOMEM;
> +                       goto failed_of_parse;
> +               }
> +
> +               ret = imxfb_of_read_mode(display_np, fbi->mode);
> +               if (ret)
> +                       goto failed_of_parse;
> +       }
> +
> +       for (i = 0, m = &fbi->mode[0]; i < fbi->num_modes; i++, m++)
> +               info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> +                               m->mode.xres * m->mode.yres * m->bpp / 8);

Surely this is broken if bpp is not as multiple of 8?

If we can only handle multiples of 8, could we not sanity check this earlier?

If there's no strong preference for describing it in bits, could we not
describe it in bytes and side-step the issue?

[...]

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH 19/20] OMAPDSS: DSI: fix DSI channel source initialization
From: Archit Taneja @ 2013-03-11  8:27 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513D8198.7070702@ti.com>

On Monday 11 March 2013 12:32 PM, Tomi Valkeinen wrote:
> On 2013-03-11 08:10, Archit Taneja wrote:
>> Hi,
>>
>> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>>> During the initialization of the DSI protocol registers, we always set
>>> the sources of all DSI channels to L4. However, we don't update the
>>> value in the dsi_data, so we may end up with a different value in the
>>> register and in the dsi_data, leading to DSI problems.
>>>
>>> This patch fixes the issue by initializing also the channel source in
>>> the dsi_data.
>>
>> We set in omap_dsihw_probe:
>>
>> static int __init omap_dsihw_probe(struct platform_device *dsidev)
>> {
>>      ...
>>      ...
>>          /* DSI VCs initialization */
>>          for (i = 0; i < ARRAY_SIZE(dsi->vc); i++) {
>>                  dsi->vc[i].source = DSI_VC_SOURCE_L4;
>>                  dsi->vc[i].dssdev = NULL;
>>                  dsi->vc[i].vc_id = 0;
>>          }
>>      ...
>>      ...
>> }
>
> Hmm... I did have a bug related to this when prototyping CDF. Ah.
> Consider this:
>
> Panel powers up and uses DSI normally. A DSI VC is set to video mode.
> Then the panel power down. Then it powers up again, and enables DSI. At
> this time, dsi_vc_initial_config() is called again, setting the source
> in the registers to L4. But the source in dsi_data is still VP.

Oh ok.

>
> So perhaps the whole piece of code from omap_dsihw_probe should be moved
> to somewhere else (dsi_vc_initial_config() sounds like a good place), so
> that they are initialized each time the registers are initialized.

VC source is something which seems fine to do in 
dsi_vc_initial_config(). I'm not sure about the dssdev and vc_id fields 
though, we would need to call omap_dsi_request_vc() and 
omap_dsi_set_vc_id() again after a power off. Currently, we do it only 
once in taal_probe().

Archit


^ permalink raw reply

* Re: [PATCH 19/20] OMAPDSS: DSI: fix DSI channel source initialization
From: Tomi Valkeinen @ 2013-03-11  8:25 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513D92A5.5050509@ti.com>

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

On 2013-03-11 10:15, Archit Taneja wrote:
> On Monday 11 March 2013 12:32 PM, Tomi Valkeinen wrote:
>> On 2013-03-11 08:10, Archit Taneja wrote:
>>> Hi,
>>>
>>> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>>>> During the initialization of the DSI protocol registers, we always set
>>>> the sources of all DSI channels to L4. However, we don't update the
>>>> value in the dsi_data, so we may end up with a different value in the
>>>> register and in the dsi_data, leading to DSI problems.
>>>>
>>>> This patch fixes the issue by initializing also the channel source in
>>>> the dsi_data.
>>>
>>> We set in omap_dsihw_probe:
>>>
>>> static int __init omap_dsihw_probe(struct platform_device *dsidev)
>>> {
>>>      ...
>>>      ...
>>>          /* DSI VCs initialization */
>>>          for (i = 0; i < ARRAY_SIZE(dsi->vc); i++) {
>>>                  dsi->vc[i].source = DSI_VC_SOURCE_L4;
>>>                  dsi->vc[i].dssdev = NULL;
>>>                  dsi->vc[i].vc_id = 0;
>>>          }
>>>      ...
>>>      ...
>>> }
>>
>> Hmm... I did have a bug related to this when prototyping CDF. Ah.
>> Consider this:
>>
>> Panel powers up and uses DSI normally. A DSI VC is set to video mode.
>> Then the panel power down. Then it powers up again, and enables DSI. At
>> this time, dsi_vc_initial_config() is called again, setting the source
>> in the registers to L4. But the source in dsi_data is still VP.
> 
> Oh ok.
> 
>>
>> So perhaps the whole piece of code from omap_dsihw_probe should be moved
>> to somewhere else (dsi_vc_initial_config() sounds like a good place), so
>> that they are initialized each time the registers are initialized.
> 
> VC source is something which seems fine to do in
> dsi_vc_initial_config(). I'm not sure about the dssdev and vc_id fields
> though, we would need to call omap_dsi_request_vc() and
> omap_dsi_set_vc_id() again after a power off. Currently, we do it only
> once in taal_probe().

Oh, right. Well, neither dssdev nor vc_id is written to registers, so
they won't have similar issues in any case. So I guess this patch is
fine as it is, and we do not need to touch dssdev nor vc_id.

 Tomi



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

^ permalink raw reply

* [PATCH] matrox: fix coccicheck warning in matroxfb_base.c
From: Silviu-Mihai Popescu @ 2013-03-11  7:32 UTC (permalink / raw)
  To: linux-fbdev; +Cc: FlorianSchandinat, linux-kernel, Silviu-Mihai Popescu

This replaces a call to kmalloc() followed by memset() with just one
call to kzalloc().

Signed-off-by: Silviu-Mihai Popescu <silviupopescu1990@gmail.com>
---
 drivers/video/matrox/matroxfb_base.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/video/matrox/matroxfb_base.c b/drivers/video/matrox/matroxfb_base.c
index 401a56e..2456529 100644
--- a/drivers/video/matrox/matroxfb_base.c
+++ b/drivers/video/matrox/matroxfb_base.c
@@ -2029,10 +2029,9 @@ static int matroxfb_probe(struct pci_dev* pdev, const struct pci_device_id* dumm
 		return -1;
 	}
 
-	minfo = kmalloc(sizeof(*minfo), GFP_KERNEL);
+	minfo = kzalloc(sizeof(*minfo), GFP_KERNEL);
 	if (!minfo)
 		return -1;
-	memset(minfo, 0, sizeof(*minfo));
 
 	minfo->pcidev = pdev;
 	minfo->dead = 0;
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 19/20] OMAPDSS: DSI: fix DSI channel source initialization
From: Tomi Valkeinen @ 2013-03-11  7:02 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513D754A.4000607@ti.com>

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

On 2013-03-11 08:10, Archit Taneja wrote:
> Hi,
> 
> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>> During the initialization of the DSI protocol registers, we always set
>> the sources of all DSI channels to L4. However, we don't update the
>> value in the dsi_data, so we may end up with a different value in the
>> register and in the dsi_data, leading to DSI problems.
>>
>> This patch fixes the issue by initializing also the channel source in
>> the dsi_data.
> 
> We set in omap_dsihw_probe:
> 
> static int __init omap_dsihw_probe(struct platform_device *dsidev)
> {
>     ...
>     ...
>         /* DSI VCs initialization */
>         for (i = 0; i < ARRAY_SIZE(dsi->vc); i++) {
>                 dsi->vc[i].source = DSI_VC_SOURCE_L4;
>                 dsi->vc[i].dssdev = NULL;
>                 dsi->vc[i].vc_id = 0;
>         }
>     ...
>     ...
> }

Hmm... I did have a bug related to this when prototyping CDF. Ah.
Consider this:

Panel powers up and uses DSI normally. A DSI VC is set to video mode.
Then the panel power down. Then it powers up again, and enables DSI. At
this time, dsi_vc_initial_config() is called again, setting the source
in the registers to L4. But the source in dsi_data is still VP.

So perhaps the whole piece of code from omap_dsihw_probe should be moved
to somewhere else (dsi_vc_initial_config() sounds like a good place), so
that they are initialized each time the registers are initialized.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 20/20] OMAPDSS: Taal: remove rotate & mirror support
From: Tomi Valkeinen @ 2013-03-11  6:51 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <513D7807.2010509@ti.com>

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

On 2013-03-11 08:21, Archit Taneja wrote:
> Hi,
> 
> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
>> Taal panel driver has support to set rotation and mirroring. However,
>> these features cannot be used without causing tearing, and are never
>> used. The code is just extra bloat, so let's remove it.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> <snip>
>>   static ssize_t taal_num_errors_show(struct device *dev,
>> @@ -1025,10 +973,6 @@ static int taal_power_on(struct omap_dss_device
>> *dssdev)
>>       if (r)
>>           goto err;
>>
>> -    r = taal_set_addr_mode(td, td->rotate, td->mirror);
>> -    if (r)
>> -        goto err;
>> -
> 
> I'm curious if we need to set the address mode(to the default value) at
> least once. It may not be a requirement for Taal, but if that's the
> case, why did we have a set_addr_mode() call in taal_power_on() in the
> first place? Is it because we can prepare rotation and mirroring before
> we enable the panel?

The panel resets its registers at HW reset, so in case we have changed
the rotation or mirroring, we need to set the addr more at power_on to
keep the user's rotation and mirroring after resuming from blanking. But
now that the rotation or mirroring is never changed, the default value
is always fine.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 14/20] OMAPDSS: remove dssdev->channel assignments
From: Archit Taneja @ 2013-03-11  6:36 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743515-10152-15-git-send-email-tomi.valkeinen@ti.com>

On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
> Now that the driver uses ooutput->recommended_channel, we can remove the

Typo above for 'output'. We could discard this patch if we don't add 
dssdev->channel assignments in patch # 10.

Archit

^ permalink raw reply

* Re: [PATCH 20/20] OMAPDSS: Taal: remove rotate & mirror support
From: Archit Taneja @ 2013-03-11  6:33 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743515-10152-21-git-send-email-tomi.valkeinen@ti.com>

Hi,

On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
> Taal panel driver has support to set rotation and mirroring. However,
> these features cannot be used without causing tearing, and are never
> used. The code is just extra bloat, so let's remove it.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

<snip>
>   static ssize_t taal_num_errors_show(struct device *dev,
> @@ -1025,10 +973,6 @@ static int taal_power_on(struct omap_dss_device *dssdev)
>   	if (r)
>   		goto err;
>
> -	r = taal_set_addr_mode(td, td->rotate, td->mirror);
> -	if (r)
> -		goto err;
> -

I'm curious if we need to set the address mode(to the default value) at 
least once. It may not be a requirement for Taal, but if that's the 
case, why did we have a set_addr_mode() call in taal_power_on() in the 
first place? Is it because we can prepare rotation and mirroring before 
we enable the panel?

Archit


^ permalink raw reply

* Re: [PATCH 19/20] OMAPDSS: DSI: fix DSI channel source initialization
From: Archit Taneja @ 2013-03-11  6:22 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743515-10152-20-git-send-email-tomi.valkeinen@ti.com>

Hi,

On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
> During the initialization of the DSI protocol registers, we always set
> the sources of all DSI channels to L4. However, we don't update the
> value in the dsi_data, so we may end up with a different value in the
> register and in the dsi_data, leading to DSI problems.
>
> This patch fixes the issue by initializing also the channel source in
> the dsi_data.

We set in omap_dsihw_probe:

static int __init omap_dsihw_probe(struct platform_device *dsidev)
{
	...
	...
         /* DSI VCs initialization */
         for (i = 0; i < ARRAY_SIZE(dsi->vc); i++) {
                 dsi->vc[i].source = DSI_VC_SOURCE_L4;
                 dsi->vc[i].dssdev = NULL;
                 dsi->vc[i].vc_id = 0;
         }
	...
	...
}

<snip>

Archit


^ permalink raw reply

* Re: [PATCH 1/3] video: backlight: adp5520: fix compiler warning in adp5520_show
From: devendra.aaru @ 2013-03-11  6:18 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1362771069-16345-1-git-send-email-devendra.aaru@gmail.com>

On Sun, Mar 10, 2013 at 8:56 PM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Saturday, March 09, 2013 4:31 AM, Devendra Naga wrote:
>>
>> while compiling with make W=1 (gcc gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8))
>>
>> found the following warning
>>
>> drivers/video/backlight/adp5520_bl.c: In function ‘adp5520_show’:
>> drivers/video/backlight/adp5520_bl.c:146:6: warning: variable ‘error’ set but not used [-Wunused-but-
>> set-variable]
>>
>> fixed by removing the variable
>>
>> Cc: Jingoo Han <jg1.han@samsung.com>
>> Cc: Michael Hennerich <michael.hennerich@analog.com>
>> Cc: Richard Purdie <rpurdie@rpsys.net>
>>
>> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
>> ---
>>  drivers/video/backlight/adp5520_bl.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/backlight/adp5520_bl.c b/drivers/video/backlight/adp5520_bl.c
>> index a1e41d4..9f8b20b 100644
>> --- a/drivers/video/backlight/adp5520_bl.c
>> +++ b/drivers/video/backlight/adp5520_bl.c
>> @@ -143,11 +143,10 @@ static int adp5520_bl_setup(struct backlight_device *bl)
>>  static ssize_t adp5520_show(struct device *dev, char *buf, int reg)
>>  {
>>       struct adp5520_bl *data = dev_get_drvdata(dev);
>> -     int error;
>>       uint8_t reg_val;
>>
>>       mutex_lock(&data->lock);
>> -     error = adp5520_read(data->master, reg, &reg_val);
>> +     adp5520_read(data->master, reg, &reg_val);
>>       mutex_unlock(&data->lock);
>
> Hi Devendra Naga,
>
> I also agree with Andrew Morton's opinion.
> It would be better to check return value from I2C read/write functions.
>

thanks, i will do and send a patch sooner or Andrew can merge his
patch with my Acked By.


> Best regards,
> Jingoo Han
>
>>
>>       return sprintf(buf, "%u\n", reg_val);
>> --
>> 1.8.1.2
>

^ permalink raw reply

* Re: [PATCH 18/20] OMAPDSS: DSI: delay dispc initialization
From: Archit Taneja @ 2013-03-11  6:17 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743515-10152-19-git-send-email-tomi.valkeinen@ti.com>

On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
> We currently setup both DSI and DISPC related things when the DSI bus is
> enabled. There's no need for DISPC related thing at that point, though,
> but only later when the video output is enabled.
>
> To make it possible to use the DSI bus before DISPC overlay manager is
> selected, this patch moves DSI's DISPC initialization to
> dsi_enable_video_output(), from omapdss_dsi_display_enable(). We also
> move the selection of DISPC's LCD clock to dsi_enable_video_output.
>
> This way there are no DISPC dependencies until the video output is
> enabled.

This is a good patch. I hope CDF also makes sure the Display controller 
and DSI bus are made more independent in this manner.

One thing which we should eventually add is to ensure that 
omap_dsi_update() for command mode panels is called only after 
dsi_enable_video_output() is called. I think we manage this in our panel 
driver, but maybe some sort of check could help there.

<snip>

Archit


^ permalink raw reply

* Re: [PATCH 1/3] video: backlight: adp5520: fix compiler warning in adp5520_show
From: devendra.aaru @ 2013-03-11  6:16 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1362771069-16345-1-git-send-email-devendra.aaru@gmail.com>

On Fri, Mar 8, 2013 at 4:01 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Fri,  8 Mar 2013 14:31:07 -0500 Devendra Naga <devendra.aaru@gmail.com> wrote:
>
>> while compiling with make W=1 (gcc gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8))
>>
>> found the following warning
>>
>> drivers/video/backlight/adp5520_bl.c: In function ___adp5520_show___:
>> drivers/video/backlight/adp5520_bl.c:146:6: warning: variable ___error___ set but not used [-Wunused-but-set-variable]
>>
>> fixed by removing the variable
>>
>> ...
>>
>> --- a/drivers/video/backlight/adp5520_bl.c
>> +++ b/drivers/video/backlight/adp5520_bl.c
>> @@ -143,11 +143,10 @@ static int adp5520_bl_setup(struct backlight_device *bl)
>>  static ssize_t adp5520_show(struct device *dev, char *buf, int reg)
>>  {
>>       struct adp5520_bl *data = dev_get_drvdata(dev);
>> -     int error;
>>       uint8_t reg_val;
>>
>>       mutex_lock(&data->lock);
>> -     error = adp5520_read(data->master, reg, &reg_val);
>> +     adp5520_read(data->master, reg, &reg_val);
>>       mutex_unlock(&data->lock);
>>
>>       return sprintf(buf, "%u\n", reg_val);
>
> We shouldn't just ignore the error; with the code as it stands, a
> adp5520_read() failure will result in the kernel displaying
> uninitialised garbage.
>
> So it would be better to propagate the adp5520_read() return value back
> to the caller if it's negative.
>
>
> (This assumes that the i2c layer returns a sane return value - if it
> does, that would make i2c pretty unique :( We could get paranoid and
> return a hard-wired -EIO, but it would be bad of us to overwrite things
> like -ENOMEM).
>
> So I'd suggest this:
>
> --- a/drivers/video/backlight/adp5520_bl.c~video-backlight-adp5520-fix-compiler-warning-in-adp5520_show
> +++ a/drivers/video/backlight/adp5520_bl.c
> @@ -143,13 +143,15 @@ static int adp5520_bl_setup(struct backl
>  static ssize_t adp5520_show(struct device *dev, char *buf, int reg)
>  {
>         struct adp5520_bl *data = dev_get_drvdata(dev);
> -       int error;
> +       int ret;
>         uint8_t reg_val;
>
>         mutex_lock(&data->lock);
> -       error = adp5520_read(data->master, reg, &reg_val);
> +       ret = adp5520_read(data->master, reg, &reg_val);
>         mutex_unlock(&data->lock);
>
> +       if (ret < 0)
> +               return ret;
>         return sprintf(buf, "%u\n", reg_val);
>  }
>

Thanks for the suggestion, i will do the same and i will send you a
patch sooner.

or you can merge your change with my Acked By too :).

> _
>

^ permalink raw reply

* Re: [PATCH 10/20] OMAPDSS: Resolve channels for outputs
From: Archit Taneja @ 2013-03-11  5:54 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743515-10152-11-git-send-email-tomi.valkeinen@ti.com>

On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
> The DISPC channel used for each output is currently passed in panel
> platform data from the board files.
>
> To simplify this, and to make the panel drivers less dependent on OMAP,
> this patch changes omapdss to resolve the channel independently. The
> channel is resolved based on the OMAP version and, in case of DSI, the
> DSI module id.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/dpi.c  |   37 ++++++++++++++++++++++++++-----
>   drivers/video/omap2/dss/dsi.c  |   48 ++++++++++++++++++++++++++++++++++++++++
>   drivers/video/omap2/dss/rfbi.c |    2 ++
>   drivers/video/omap2/dss/sdi.c  |    2 ++
>   4 files changed, 84 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
> index e282456..3261644 100644
> --- a/drivers/video/omap2/dss/dpi.c
> +++ b/drivers/video/omap2/dss/dpi.c
> @@ -396,12 +396,44 @@ static int __init dpi_verify_dsi_pll(struct platform_device *dsidev)
>   	return 0;
>   }
>
> +/*
> + * Return a hardcoded channel for the DPI output. This should work for
> + * current use cases, but this can be later expanded to either resolve
> + * the channel in some more dynamic manner, or get the channel as a user
> + * parameter.
> + */
> +static enum omap_channel dpi_get_channel(void)
> +{
> +	switch (omapdss_get_version()) {
> +	case OMAPDSS_VER_OMAP24xx:
> +	case OMAPDSS_VER_OMAP34xx_ES1:
> +	case OMAPDSS_VER_OMAP34xx_ES3:
> +	case OMAPDSS_VER_OMAP3630:
> +	case OMAPDSS_VER_AM35xx:
> +		return OMAP_DSS_CHANNEL_LCD;
> +
> +	case OMAPDSS_VER_OMAP4430_ES1:
> +	case OMAPDSS_VER_OMAP4430_ES2:
> +	case OMAPDSS_VER_OMAP4:
> +		return OMAP_DSS_CHANNEL_LCD2;
> +
> +	case OMAPDSS_VER_OMAP5:
> +		return OMAP_DSS_CHANNEL_LCD2;
> +
> +	default:
> +		DSSWARN("unsupported DSS version\n");
> +		return OMAP_DSS_CHANNEL_LCD;
> +	}
> +}

I had another comment for this patch. On OMAP5, it makes sense for us to 
not use LCD2 as the recommended channel. LCD2_CLK's only source is 
DSS_CLK from PRCM. So it's not a very flexible channel to use. We could 
use LCD3 (at the cost of not using DSI2).

We also need to fix dpi_get_dsidev() for OMAP5. Currently, it assumes 
that LCD2_CLK can be sourced from DSI2 PLL, we need to ensure DPI has a 
dsidev only if it's LCD1 or LCD3.

Archit

> +
>   static int __init dpi_init_display(struct omap_dss_device *dssdev)
>   {
>   	struct platform_device *dsidev;
>
>   	DSSDBG("init_display\n");
>
> +	dssdev->channel = dpi_get_channel();
> +
>   	if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI) &&
>   					dpi.vdds_dsi_reg = NULL) {
>   		struct regulator *vdds_dsi;
> @@ -416,11 +448,6 @@ static int __init dpi_init_display(struct omap_dss_device *dssdev)
>   		dpi.vdds_dsi_reg = vdds_dsi;
>   	}
>
> -	/*
> -	 * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
> -	 * source for DPI is SoC integration detail, not something that should
> -	 * be configured in the dssdev
> -	 */
>   	dsidev = dpi_get_dsidev(dssdev->channel);
>
>   	if (dsidev && dpi_verify_dsi_pll(dsidev)) {
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 1a6ad6f..c39ca86 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -4946,6 +4946,52 @@ void omapdss_dsi_set_videomode_timings(struct omap_dss_device *dssdev,
>   }
>   EXPORT_SYMBOL(omapdss_dsi_set_videomode_timings);
>
> +/*
> + * Return a hardcoded channel for the DSI output. This should work for
> + * current use cases, but this can be later expanded to either resolve
> + * the channel in some more dynamic manner, or get the channel as a user
> + * parameter.
> + */
> +static enum omap_channel dsi_get_channel(int module_id)
> +{
> +	switch (omapdss_get_version()) {
> +	case OMAPDSS_VER_OMAP24xx:
> +	case OMAPDSS_VER_OMAP34xx_ES1:
> +	case OMAPDSS_VER_OMAP34xx_ES3:
> +	case OMAPDSS_VER_OMAP3630:
> +	case OMAPDSS_VER_AM35xx:
> +		return OMAP_DSS_CHANNEL_LCD;
> +
> +	case OMAPDSS_VER_OMAP4430_ES1:
> +	case OMAPDSS_VER_OMAP4430_ES2:
> +	case OMAPDSS_VER_OMAP4:
> +		switch (module_id) {
> +		case 0:
> +			return OMAP_DSS_CHANNEL_LCD;
> +		case 1:
> +			return OMAP_DSS_CHANNEL_LCD2;
> +		default:
> +			DSSWARN("unsupported module id\n");
> +			return OMAP_DSS_CHANNEL_LCD;
> +		}
> +
> +	case OMAPDSS_VER_OMAP5:
> +		switch (module_id) {
> +		case 0:
> +			return OMAP_DSS_CHANNEL_LCD;
> +		case 1:
> +			return OMAP_DSS_CHANNEL_LCD3;
> +		default:
> +			DSSWARN("unsupported module id\n");
> +			return OMAP_DSS_CHANNEL_LCD;
> +		}
> +
> +	default:
> +		DSSWARN("unsupported DSS version\n");
> +		return OMAP_DSS_CHANNEL_LCD;
> +	}
> +}
> +
>   static int __init dsi_init_display(struct omap_dss_device *dssdev)
>   {
>   	struct platform_device *dsidev > @@ -4954,6 +5000,8 @@ static int __init dsi_init_display(struct omap_dss_device *dssdev)
>
>   	DSSDBG("DSI init\n");
>
> +	dssdev->channel = dsi_get_channel(dsi->module_id);
> +
>   	if (dsi->vdds_dsi_reg = NULL) {
>   		struct regulator *vdds_dsi;
>
> diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
> index a47a9e5..04c4ab6 100644
> --- a/drivers/video/omap2/dss/rfbi.c
> +++ b/drivers/video/omap2/dss/rfbi.c
> @@ -945,6 +945,8 @@ EXPORT_SYMBOL(omapdss_rfbi_display_disable);
>
>   static int __init rfbi_init_display(struct omap_dss_device *dssdev)
>   {
> +	dssdev->channel = OMAP_DSS_CHANNEL_LCD;
> +
>   	rfbi.dssdev[dssdev->phy.rfbi.channel] = dssdev;
>   	return 0;
>   }
> diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
> index 0802927..d24e971 100644
> --- a/drivers/video/omap2/dss/sdi.c
> +++ b/drivers/video/omap2/dss/sdi.c
> @@ -186,6 +186,8 @@ static int __init sdi_init_display(struct omap_dss_device *dssdev)
>   {
>   	DSSDBG("SDI init\n");
>
> +	dssdev->channel = OMAP_DSS_CHANNEL_LCD;
> +
>   	if (sdi.vdds_sdi_reg = NULL) {
>   		struct regulator *vdds_sdi;
>
>


^ permalink raw reply

* Re: [PATCH 10/20] OMAPDSS: Resolve channels for outputs
From: Archit Taneja @ 2013-03-11  5:47 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743515-10152-11-git-send-email-tomi.valkeinen@ti.com>

Hi,

On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote:
> The DISPC channel used for each output is currently passed in panel
> platform data from the board files.
>
> To simplify this, and to make the panel drivers less dependent on OMAP,
> this patch changes omapdss to resolve the channel independently. The
> channel is resolved based on the OMAP version and, in case of DSI, the
> DSI module id.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/dpi.c  |   37 ++++++++++++++++++++++++++-----
>   drivers/video/omap2/dss/dsi.c  |   48 ++++++++++++++++++++++++++++++++++++++++
>   drivers/video/omap2/dss/rfbi.c |    2 ++
>   drivers/video/omap2/dss/sdi.c  |    2 ++
>   4 files changed, 84 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
> index e282456..3261644 100644
> --- a/drivers/video/omap2/dss/dpi.c
> +++ b/drivers/video/omap2/dss/dpi.c
> @@ -396,12 +396,44 @@ static int __init dpi_verify_dsi_pll(struct platform_device *dsidev)
>   	return 0;
>   }
>
> +/*
> + * Return a hardcoded channel for the DPI output. This should work for
> + * current use cases, but this can be later expanded to either resolve
> + * the channel in some more dynamic manner, or get the channel as a user
> + * parameter.
> + */
> +static enum omap_channel dpi_get_channel(void)
> +{
> +	switch (omapdss_get_version()) {
> +	case OMAPDSS_VER_OMAP24xx:
> +	case OMAPDSS_VER_OMAP34xx_ES1:
> +	case OMAPDSS_VER_OMAP34xx_ES3:
> +	case OMAPDSS_VER_OMAP3630:
> +	case OMAPDSS_VER_AM35xx:
> +		return OMAP_DSS_CHANNEL_LCD;
> +
> +	case OMAPDSS_VER_OMAP4430_ES1:
> +	case OMAPDSS_VER_OMAP4430_ES2:
> +	case OMAPDSS_VER_OMAP4:
> +		return OMAP_DSS_CHANNEL_LCD2;
> +
> +	case OMAPDSS_VER_OMAP5:
> +		return OMAP_DSS_CHANNEL_LCD2;
> +
> +	default:
> +		DSSWARN("unsupported DSS version\n");
> +		return OMAP_DSS_CHANNEL_LCD;
> +	}
> +}
> +
>   static int __init dpi_init_display(struct omap_dss_device *dssdev)
>   {
>   	struct platform_device *dsidev;
>
>   	DSSDBG("init_display\n");
>
> +	dssdev->channel = dpi_get_channel();

In patch 14 of the series, we remove these dssdev->channel assignments. 
I don't see the point of adding them in this patch in the first place. 
The dssdev->channel assignments will not be modified in this series, so 
we don't need to worry about a kernel crash or something after this patch.

I feel this patch should only add the dpi_get_channel and 
dsi_get_channel funcs.

> +
>   	if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI) &&
>   					dpi.vdds_dsi_reg = NULL) {
>   		struct regulator *vdds_dsi;
> @@ -416,11 +448,6 @@ static int __init dpi_init_display(struct omap_dss_device *dssdev)
>   		dpi.vdds_dsi_reg = vdds_dsi;
>   	}
>
> -	/*
> -	 * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
> -	 * source for DPI is SoC integration detail, not something that should
> -	 * be configured in the dssdev
> -	 */
>   	dsidev = dpi_get_dsidev(dssdev->channel);
>
>   	if (dsidev && dpi_verify_dsi_pll(dsidev)) {
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 1a6ad6f..c39ca86 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -4946,6 +4946,52 @@ void omapdss_dsi_set_videomode_timings(struct omap_dss_device *dssdev,
>   }
>   EXPORT_SYMBOL(omapdss_dsi_set_videomode_timings);
>
> +/*
> + * Return a hardcoded channel for the DSI output. This should work for
> + * current use cases, but this can be later expanded to either resolve
> + * the channel in some more dynamic manner, or get the channel as a user
> + * parameter.
> + */
> +static enum omap_channel dsi_get_channel(int module_id)
> +{
> +	switch (omapdss_get_version()) {
> +	case OMAPDSS_VER_OMAP24xx:

We should remove the above case so that we hit the default case and get 
a warning about omap2 not having DSI.

> +	case OMAPDSS_VER_OMAP34xx_ES1:
> +	case OMAPDSS_VER_OMAP34xx_ES3:
> +	case OMAPDSS_VER_OMAP3630:
> +	case OMAPDSS_VER_AM35xx:

<snip>

Archit


^ permalink raw reply

* Re: [PATCH 1/3] video: backlight: adp5520: fix compiler warning in adp5520_show
From: Jingoo Han @ 2013-03-11  0:56 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1362771069-16345-1-git-send-email-devendra.aaru@gmail.com>

On Saturday, March 09, 2013 4:31 AM, Devendra Naga wrote:
> 
> while compiling with make W=1 (gcc gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8))
> 
> found the following warning
> 
> drivers/video/backlight/adp5520_bl.c: In function ‘adp5520_show’:
> drivers/video/backlight/adp5520_bl.c:146:6: warning: variable ‘error’ set but not used [-Wunused-but-
> set-variable]
> 
> fixed by removing the variable
> 
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> 
> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
> ---
>  drivers/video/backlight/adp5520_bl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/video/backlight/adp5520_bl.c b/drivers/video/backlight/adp5520_bl.c
> index a1e41d4..9f8b20b 100644
> --- a/drivers/video/backlight/adp5520_bl.c
> +++ b/drivers/video/backlight/adp5520_bl.c
> @@ -143,11 +143,10 @@ static int adp5520_bl_setup(struct backlight_device *bl)
>  static ssize_t adp5520_show(struct device *dev, char *buf, int reg)
>  {
>  	struct adp5520_bl *data = dev_get_drvdata(dev);
> -	int error;
>  	uint8_t reg_val;
> 
>  	mutex_lock(&data->lock);
> -	error = adp5520_read(data->master, reg, &reg_val);
> +	adp5520_read(data->master, reg, &reg_val);
>  	mutex_unlock(&data->lock);

Hi Devendra Naga,

I also agree with Andrew Morton's opinion.
It would be better to check return value from I2C read/write functions.

Best regards,
Jingoo Han

> 
>  	return sprintf(buf, "%u\n", reg_val);
> --
> 1.8.1.2


^ permalink raw reply

* RE: [PATCH 2/3] video: backlight: lp855x_bl: fix compiler warning in lp855x_probe
From: Kim, Milo @ 2013-03-10 23:04 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1362771069-16345-2-git-send-email-devendra.aaru@gmail.com>

PiB3aGlsZSBkb2luZyB3aXRoIG1ha2UgVz0xIGdjYyAoZ2NjIChHQ0MpIDQuNy4yIDIwMTIxMTA5
IChSZWQgSGF0IDQuNy4yLQ0KPiA4KSkNCj4gDQo+IGZvdW5kDQo+IA0KPiBkcml2ZXJzL3ZpZGVv
L2JhY2tsaWdodC9scDg1NXhfYmwuYzogSW4gZnVuY3Rpb24g4oCYbHA4NTV4X3Byb2Jl4oCZOg0K
PiBkcml2ZXJzL3ZpZGVvL2JhY2tsaWdodC9scDg1NXhfYmwuYzozNDI6MzU6IHdhcm5pbmc6IHZh
cmlhYmxlIOKAmG1vZGXigJkNCj4gc2V0IGJ1dCBub3QgdXNlZCBbLVd1bnVzZWQtYnV0LXNldC12
YXJpYWJsZV0NCj4gDQo+IGZpeGVkIGJ5IHJlbW92aW5nIGl0IGFzIHNpbmNlIGl0cyBub3QgdXNl
ZCBhbnl3aGVyZQ0KDQpBY2tlZC1ieTogTWlsbyBLaW0gPG1pbG8ua2ltQHRpLmNvbT4NCg0K

^ permalink raw reply

* [PATCH linux-next] mfd: max8925: max8925_backlight_probe: Silence 'statement with no effect' warning
From: Tim Gardner @ 2013-03-10 18:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tim Gardner, Richard Purdie, Florian Tobias Schandinat,
	linux-fbdev

Commit 47ec340cb8e232671e7c4a4689ff32c3bdf329da 'mfd: max8925: Support dt for backlight'
caused a gcc warning if CONFIG_OF is not defined:

drivers/video/backlight/max8925_bl.c: In function 'max8925_backlight_probe':
drivers/video/backlight/max8925_bl.c:177:3: warning: statement with no effect [-Wunused-value]

gcc version 4.6.3

Convert max8925_backlight_dt_init() to an 'inline void' since it is only
called from one place where the return code is ignored. Protect the
guts of the function with '#ifdef CONFIG_OF'.

Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/video/backlight/max8925_bl.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/video/backlight/max8925_bl.c b/drivers/video/backlight/max8925_bl.c
index 5ca11b0..199f887 100644
--- a/drivers/video/backlight/max8925_bl.c
+++ b/drivers/video/backlight/max8925_bl.c
@@ -101,10 +101,10 @@ static const struct backlight_ops max8925_backlight_ops = {
 	.get_brightness	= max8925_backlight_get_brightness,
 };
 
-#ifdef CONFIG_OF
-static int max8925_backlight_dt_init(struct platform_device *pdev,
+static inline void max8925_backlight_dt_init(struct platform_device *pdev,
 			      struct max8925_backlight_pdata *pdata)
 {
+#ifdef CONFIG_OF
 	struct device_node *nproot = pdev->dev.parent->of_node, *np;
 	int dual_string;
 
@@ -118,11 +118,8 @@ static int max8925_backlight_dt_init(struct platform_device *pdev,
 
 	of_property_read_u32(np, "maxim,max8925-dual-string", &dual_string);
 	pdata->dual_string = dual_string;
-	return 0;
-}
-#else
-#define max8925_backlight_dt_init(x, y)	(-1)
 #endif
+}
 
 static int max8925_backlight_probe(struct platform_device *pdev)
 {
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH linux-next] intelfb: intelfbhw_mode_to_hw: Silence m1/m2 'may be used uninitialized' warnings
From: Tim Gardner @ 2013-03-10 17:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tim Gardner, Maik Broemme, Florian Tobias Schandinat, linux-fbdev

drivers/video/intelfb/intelfbhw.c: In function 'intelfbhw_mode_to_hw':
drivers/video/intelfb/intelfbhw.c:1144:35: warning: 'm2' may be used uninitialized in this function [-Wuninitialized]
drivers/video/intelfb/intelfbhw.c:1145:13: warning: 'm1' may be used uninitialized in this function [-Wuninitialized]

gcc version 4.6.3

Cc: Maik Broemme <mbroemme@plusserver.de>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/video/intelfb/intelfbhw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/intelfb/intelfbhw.c b/drivers/video/intelfb/intelfbhw.c
index fbad61d..cb05307 100644
--- a/drivers/video/intelfb/intelfbhw.c
+++ b/drivers/video/intelfb/intelfbhw.c
@@ -935,7 +935,7 @@ static int splitp(int index, unsigned int p, unsigned int *retp1,
 static int calc_pll_params(int index, int clock, u32 *retm1, u32 *retm2,
 			   u32 *retn, u32 *retp1, u32 *retp2, u32 *retclock)
 {
-	u32 m1, m2, n, p1, p2, n1, testm;
+	u32 m1 = 0, m2 = 0, n, p1, p2, n1, testm;
 	u32 f_vco, p, p_best = 0, m, f_out = 0;
 	u32 err_max, err_target, err_best = 10000000;
 	u32 n_best = 0, m_best = 0, f_best, f_err;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v3] video: Add Hyper-V Synthetic Video Frame Buffer Driver
From: Haiyang Zhang @ 2013-03-08 21:45 UTC (permalink / raw)
  To: FlorianSchandinat, linux-fbdev
  Cc: haiyangz, kys, olaf, jasowang, linux-kernel, devel

This is the driver for the Hyper-V Synthetic Video, which supports screen
resolution up to Full HD 1920x1080 on Windows Server 2012 host, and 1600x1200
on  Windows Server 2008 R2 or earlier.
It also solves the double mouse cursor issue of the emulated video mode.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

---
v3:
According to the comment from Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
moved pci ids back to the hyperv_fb.c, because it is the only driver using
them.

v2:
Made changes based on reviews from Olaf Hering <olaf@aepfle.de>, Geert
Uytterhoeven <geert@linux-m68k.org>.

Renamed the Kconfig string to FB_HYPERV. And, use KBUILD_MODNAME in two
additional places to keep consistency in naming.

Switched fb from system RAM to PCI mmio space, so the workaround for large
memory allocation is not necessary. The fb ops and screen refresh mechanism is
updated accordingly.

Added aperture setting so that VESA fb can be disabled automatically without
code change on the vesafb.c.
---
 drivers/video/Kconfig     |    9 +
 drivers/video/Makefile    |    1 +
 drivers/video/hyperv_fb.c |  829 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/hyperv.h    |   11 +
 4 files changed, 850 insertions(+), 0 deletions(-)
 create mode 100644 drivers/video/hyperv_fb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4c1546f..5d1a35e 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2451,6 +2451,15 @@ config FB_PUV3_UNIGFX
 	  Choose this option if you want to use the Unigfx device as a
 	  framebuffer device. Without the support of PCI & AGP.
 
+config FB_HYPERV
+	tristate "Microsoft Hyper-V Synthetic Video support"
+	depends on FB && HYPERV
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	help
+	  This framebuffer driver supports Microsoft Hyper-V Synthetic Video.
+
 source "drivers/video/omap/Kconfig"
 source "drivers/video/omap2/Kconfig"
 source "drivers/video/exynos/Kconfig"
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9df3873..97f7b6d 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -149,6 +149,7 @@ obj-$(CONFIG_FB_MSM)              += msm/
 obj-$(CONFIG_FB_NUC900)           += nuc900fb.o
 obj-$(CONFIG_FB_JZ4740)		  += jz4740_fb.o
 obj-$(CONFIG_FB_PUV3_UNIGFX)      += fb-puv3.o
+obj-$(CONFIG_FB_HYPERV)		  += hyperv_fb.o
 
 # Platform or fallback drivers go here
 obj-$(CONFIG_FB_UVESA)            += uvesafb.o
diff --git a/drivers/video/hyperv_fb.c b/drivers/video/hyperv_fb.c
new file mode 100644
index 0000000..ceb33b0
--- /dev/null
+++ b/drivers/video/hyperv_fb.c
@@ -0,0 +1,829 @@
+/*
+ * Copyright (c) 2012, Microsoft Corporation.
+ *
+ * Author:
+ *   Haiyang Zhang <haiyangz@microsoft.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ */
+
+/*
+ * Hyper-V Synthetic Video Frame Buffer Driver
+ *
+ * This is the driver for the Hyper-V Synthetic Video, which supports
+ * screen resolution up to Full HD 1920x1080 with 32 bit color on Windows
+ * Server 2012, and 1600x1200 with 16 bit color on Windows Server 2008 R2
+ * or earlier.
+ *
+ * It also solves the double mouse cursor issue of the emulated video mode.
+ *
+ * The default screen resolution is 1152x864, which may be changed by a
+ * kernel parameter:
+ *     video=hyperv_fb:<width>x<height>
+ *     For example: video=hyperv_fb:1280x1024
+ *
+ * Portrait orientation is also supported:
+ *     For example: video=hyperv_fb:864x1152
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/completion.h>
+#include <linux/fb.h>
+#include <linux/pci.h>
+
+#include <linux/hyperv.h>
+
+
+/* Hyper-V Synthetic Video Protocol definitions and structures */
+#define MAX_VMBUS_PKT_SIZE 0x4000
+
+#define SYNTHVID_VERSION(major, minor) ((minor) << 16 | (major))
+#define SYNTHVID_VERSION_WIN7 SYNTHVID_VERSION(3, 0)
+#define SYNTHVID_VERSION_WIN8 SYNTHVID_VERSION(3, 2)
+
+#define SYNTHVID_DEPTH_WIN7 16
+#define SYNTHVID_DEPTH_WIN8 32
+
+#define SYNTHVID_FB_SIZE_WIN7 (4 * 1024 * 1024)
+#define SYNTHVID_WIDTH_MAX_WIN7 1600
+#define SYNTHVID_HEIGHT_MAX_WIN7 1200
+
+#define SYNTHVID_FB_SIZE_WIN8 (8 * 1024 * 1024)
+
+#define PCI_VENDOR_ID_MICROSOFT 0x1414
+#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
+
+
+enum pipe_msg_type {
+	PIPE_MSG_INVALID,
+	PIPE_MSG_DATA,
+	PIPE_MSG_MAX
+};
+
+struct pipe_msg_hdr {
+	u32 type;
+	u32 size; /* size of message after this field */
+} __packed;
+
+
+enum synthvid_msg_type {
+	SYNTHVID_ERROR			= 0,
+	SYNTHVID_VERSION_REQUEST	= 1,
+	SYNTHVID_VERSION_RESPONSE	= 2,
+	SYNTHVID_VRAM_LOCATION		= 3,
+	SYNTHVID_VRAM_LOCATION_ACK	= 4,
+	SYNTHVID_SITUATION_UPDATE	= 5,
+	SYNTHVID_SITUATION_UPDATE_ACK	= 6,
+	SYNTHVID_POINTER_POSITION	= 7,
+	SYNTHVID_POINTER_SHAPE		= 8,
+	SYNTHVID_FEATURE_CHANGE		= 9,
+	SYNTHVID_DIRT			= 10,
+
+	SYNTHVID_MAX			= 11
+};
+
+struct synthvid_msg_hdr {
+	u32 type;
+	u32 size;  /* size of this header + payload after this field*/
+} __packed;
+
+
+struct synthvid_version_req {
+	u32 version;
+} __packed;
+
+struct synthvid_version_resp {
+	u32 version;
+	u8 is_accepted;
+	u8 max_video_outputs;
+} __packed;
+
+struct synthvid_vram_location {
+	u64 user_ctx;
+	u8 is_vram_gpa_specified;
+	u64 vram_gpa;
+} __packed;
+
+struct synthvid_vram_location_ack {
+	u64 user_ctx;
+} __packed;
+
+struct video_output_situation {
+	u8 active;
+	u32 vram_offset;
+	u8 depth_bits;
+	u32 width_pixels;
+	u32 height_pixels;
+	u32 pitch_bytes;
+} __packed;
+
+struct synthvid_situation_update {
+	u64 user_ctx;
+	u8 video_output_count;
+	struct video_output_situation video_output[1];
+} __packed;
+
+struct synthvid_situation_update_ack {
+	u64 user_ctx;
+} __packed;
+
+struct synthvid_pointer_position {
+	u8 is_visible;
+	u8 video_output;
+	s32 image_x;
+	s32 image_y;
+} __packed;
+
+
+#define CURSOR_MAX_X 96
+#define CURSOR_MAX_Y 96
+#define CURSOR_ARGB_PIXEL_SIZE 4
+#define CURSOR_MAX_SIZE (CURSOR_MAX_X * CURSOR_MAX_Y * CURSOR_ARGB_PIXEL_SIZE)
+#define CURSOR_COMPLETE (-1)
+
+struct synthvid_pointer_shape {
+	u8 part_idx;
+	u8 is_argb;
+	u32 width; /* CURSOR_MAX_X at most */
+	u32 height; /* CURSOR_MAX_Y at most */
+	u32 hot_x; /* hotspot relative to upper-left of pointer image */
+	u32 hot_y;
+	u8 data[4];
+} __packed;
+
+struct synthvid_feature_change {
+	u8 is_dirt_needed;
+	u8 is_ptr_pos_needed;
+	u8 is_ptr_shape_needed;
+	u8 is_situ_needed;
+} __packed;
+
+struct rect {
+	s32 x1, y1; /* top left corner */
+	s32 x2, y2; /* bottom right corner, exclusive */
+} __packed;
+
+struct synthvid_dirt {
+	u8 video_output;
+	u8 dirt_count;
+	struct rect rect[1];
+} __packed;
+
+struct synthvid_msg {
+	struct pipe_msg_hdr pipe_hdr;
+	struct synthvid_msg_hdr vid_hdr;
+	union {
+		struct synthvid_version_req ver_req;
+		struct synthvid_version_resp ver_resp;
+		struct synthvid_vram_location vram;
+		struct synthvid_vram_location_ack vram_ack;
+		struct synthvid_situation_update situ;
+		struct synthvid_situation_update_ack situ_ack;
+		struct synthvid_pointer_position ptr_pos;
+		struct synthvid_pointer_shape ptr_shape;
+		struct synthvid_feature_change feature_chg;
+		struct synthvid_dirt dirt;
+	};
+} __packed;
+
+
+
+/* FB driver definitions and structures */
+#define HVFB_WIDTH 1152 /* default screen width */
+#define HVFB_HEIGHT 864 /* default screen height */
+#define HVFB_WIDTH_MIN 640
+#define HVFB_HEIGHT_MIN 480
+
+#define RING_BUFSIZE (256 * 1024)
+#define VSP_TIMEOUT (10 * HZ)
+#define HVFB_UPDATE_DELAY (HZ / 20)
+
+struct hvfb_par {
+	struct fb_info *info;
+	bool fb_ready; /* fb device is ready */
+	struct completion wait;
+	u32 synthvid_version;
+
+	struct delayed_work dwork;
+	bool update;
+
+	u32 pseudo_palette[16];
+	u8 init_buf[MAX_VMBUS_PKT_SIZE];
+	u8 recv_buf[MAX_VMBUS_PKT_SIZE];
+};
+
+static uint screen_width = HVFB_WIDTH;
+static uint screen_height = HVFB_HEIGHT;
+static uint screen_depth;
+static uint screen_fb_size;
+
+/* Send message to Hyper-V host */
+static inline int synthvid_send(struct hv_device *hdev,
+				struct synthvid_msg *msg)
+{
+	static atomic64_t request_id = ATOMIC64_INIT(0);
+	int ret;
+
+	msg->pipe_hdr.type = PIPE_MSG_DATA;
+	msg->pipe_hdr.size = msg->vid_hdr.size;
+
+	ret = vmbus_sendpacket(hdev->channel, msg,
+			       msg->vid_hdr.size + sizeof(struct pipe_msg_hdr),
+			       atomic64_inc_return(&request_id),
+			       VM_PKT_DATA_INBAND, 0);
+
+	if (ret)
+		pr_err("Unable to send packet via vmbus\n");
+
+	return ret;
+}
+
+
+/* Send screen resolution info to host */
+static int synthvid_send_situ(struct hv_device *hdev)
+{
+	struct fb_info *info = hv_get_drvdata(hdev);
+	struct synthvid_msg msg;
+
+	if (!info)
+		return -ENODEV;
+
+	memset(&msg, 0, sizeof(struct synthvid_msg));
+
+	msg.vid_hdr.type = SYNTHVID_SITUATION_UPDATE;
+	msg.vid_hdr.size = sizeof(struct synthvid_msg_hdr) +
+		sizeof(struct synthvid_situation_update);
+	msg.situ.user_ctx = 0;
+	msg.situ.video_output_count = 1;
+	msg.situ.video_output[0].active = 1;
+	msg.situ.video_output[0].vram_offset = 0;
+	msg.situ.video_output[0].depth_bits = info->var.bits_per_pixel;
+	msg.situ.video_output[0].width_pixels = info->var.xres;
+	msg.situ.video_output[0].height_pixels = info->var.yres;
+	msg.situ.video_output[0].pitch_bytes = info->fix.line_length;
+
+	synthvid_send(hdev, &msg);
+
+	return 0;
+}
+
+/* Send mouse pointer info to host */
+static int synthvid_send_ptr(struct hv_device *hdev)
+{
+	struct synthvid_msg msg;
+
+	memset(&msg, 0, sizeof(struct synthvid_msg));
+	msg.vid_hdr.type = SYNTHVID_POINTER_POSITION;
+	msg.vid_hdr.size = sizeof(struct synthvid_msg_hdr) +
+		sizeof(struct synthvid_pointer_position);
+	msg.ptr_pos.is_visible = 1;
+	msg.ptr_pos.video_output = 0;
+	msg.ptr_pos.image_x = 0;
+	msg.ptr_pos.image_y = 0;
+	synthvid_send(hdev, &msg);
+
+	memset(&msg, 0, sizeof(struct synthvid_msg));
+	msg.vid_hdr.type = SYNTHVID_POINTER_SHAPE;
+	msg.vid_hdr.size = sizeof(struct synthvid_msg_hdr) +
+		sizeof(struct synthvid_pointer_shape);
+	msg.ptr_shape.part_idx = CURSOR_COMPLETE;
+	msg.ptr_shape.is_argb = 1;
+	msg.ptr_shape.width = 1;
+	msg.ptr_shape.height = 1;
+	msg.ptr_shape.hot_x = 0;
+	msg.ptr_shape.hot_y = 0;
+	msg.ptr_shape.data[0] = 0;
+	msg.ptr_shape.data[1] = 1;
+	msg.ptr_shape.data[2] = 1;
+	msg.ptr_shape.data[3] = 1;
+	synthvid_send(hdev, &msg);
+
+	return 0;
+}
+
+/* Send updated screen area (dirty rectangle) location to host */
+static int synthvid_update(struct fb_info *info)
+{
+	struct hv_device *hdev = device_to_hv_device(info->device);
+	struct synthvid_msg msg;
+
+	memset(&msg, 0, sizeof(struct synthvid_msg));
+
+	msg.vid_hdr.type = SYNTHVID_DIRT;
+	msg.vid_hdr.size = sizeof(struct synthvid_msg_hdr) +
+		sizeof(struct synthvid_dirt);
+	msg.dirt.video_output = 0;
+	msg.dirt.dirt_count = 1;
+	msg.dirt.rect[0].x1 = 0;
+	msg.dirt.rect[0].y1 = 0;
+	msg.dirt.rect[0].x2 = info->var.xres;
+	msg.dirt.rect[0].y2 = info->var.yres;
+
+	synthvid_send(hdev, &msg);
+
+	return 0;
+}
+
+
+/*
+ * Actions on received messages from host:
+ * Complete the wait event.
+ * Or, reply with screen and cursor info.
+ */
+static void synthvid_recv_sub(struct hv_device *hdev)
+{
+	struct fb_info *info = hv_get_drvdata(hdev);
+	struct hvfb_par *par;
+	struct synthvid_msg *msg;
+
+	if (!info)
+		return;
+
+	par = info->par;
+	msg = (struct synthvid_msg *)par->recv_buf;
+
+	/* Complete the wait event */
+	if (msg->vid_hdr.type = SYNTHVID_VERSION_RESPONSE ||
+	    msg->vid_hdr.type = SYNTHVID_VRAM_LOCATION_ACK) {
+		memcpy(par->init_buf, msg, MAX_VMBUS_PKT_SIZE);
+		complete(&par->wait);
+		return;
+	}
+
+	/* Reply with screen and cursor info */
+	if (msg->vid_hdr.type = SYNTHVID_FEATURE_CHANGE) {
+		if (par->fb_ready) {
+			synthvid_send_ptr(hdev);
+			synthvid_send_situ(hdev);
+		}
+
+		par->update = msg->feature_chg.is_dirt_needed;
+		if (par->update)
+			schedule_delayed_work(&par->dwork, HVFB_UPDATE_DELAY);
+	}
+}
+
+/* Receive callback for messages from the host */
+static void synthvid_receive(void *ctx)
+{
+	struct hv_device *hdev = ctx;
+	struct fb_info *info = hv_get_drvdata(hdev);
+	struct hvfb_par *par;
+	struct synthvid_msg *recv_buf;
+	u32 bytes_recvd;
+	u64 req_id;
+	int ret;
+
+	if (!info)
+		return;
+
+	par = info->par;
+	recv_buf = (struct synthvid_msg *)par->recv_buf;
+
+	do {
+		ret = vmbus_recvpacket(hdev->channel, recv_buf,
+				       MAX_VMBUS_PKT_SIZE,
+				       &bytes_recvd, &req_id);
+		if (bytes_recvd > 0 &&
+		    recv_buf->pipe_hdr.type = PIPE_MSG_DATA)
+			synthvid_recv_sub(hdev);
+	} while (bytes_recvd > 0 && ret = 0);
+}
+
+/* Check synthetic video protocol version with the host */
+static int synthvid_negotiate_ver(struct hv_device *hdev, u32 ver)
+{
+	struct fb_info *info = hv_get_drvdata(hdev);
+	struct hvfb_par *par = info->par;
+	struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
+	int t, ret = 0;
+
+	memset(msg, 0, sizeof(struct synthvid_msg));
+	msg->vid_hdr.type = SYNTHVID_VERSION_REQUEST;
+	msg->vid_hdr.size = sizeof(struct synthvid_msg_hdr) +
+		sizeof(struct synthvid_version_req);
+	msg->ver_req.version = ver;
+	synthvid_send(hdev, msg);
+
+	t = wait_for_completion_timeout(&par->wait, VSP_TIMEOUT);
+	if (!t) {
+		pr_err("Time out on waiting version response\n");
+		ret = -ETIMEDOUT;
+		goto out;
+	}
+	if (!msg->ver_resp.is_accepted) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	par->synthvid_version = ver;
+
+out:
+	return ret;
+}
+
+/* Connect to VSP (Virtual Service Provider) on host */
+static int synthvid_connect_vsp(struct hv_device *hdev)
+{
+	struct fb_info *info = hv_get_drvdata(hdev);
+	struct hvfb_par *par = info->par;
+	int ret;
+
+	ret = vmbus_open(hdev->channel, RING_BUFSIZE, RING_BUFSIZE,
+			 NULL, 0, synthvid_receive, hdev);
+	if (ret) {
+		pr_err("Unable to open vmbus channel\n");
+		return ret;
+	}
+
+	/* Negotiate the protocol version with host */
+	if (vmbus_proto_version = VERSION_WS2008 ||
+	    vmbus_proto_version = VERSION_WIN7)
+		ret = synthvid_negotiate_ver(hdev, SYNTHVID_VERSION_WIN7);
+	else
+		ret = synthvid_negotiate_ver(hdev, SYNTHVID_VERSION_WIN8);
+
+	if (ret) {
+		pr_err("Synthetic video device version not accepted\n");
+		goto error;
+	}
+
+	if (par->synthvid_version = SYNTHVID_VERSION_WIN7) {
+		screen_depth = SYNTHVID_DEPTH_WIN7;
+		screen_fb_size = SYNTHVID_FB_SIZE_WIN7;
+	} else {
+		screen_depth = SYNTHVID_DEPTH_WIN8;
+		screen_fb_size = SYNTHVID_FB_SIZE_WIN8;
+	}
+
+	return 0;
+
+error:
+	vmbus_close(hdev->channel);
+	return ret;
+}
+
+/* Send VRAM and Situation messages to the host */
+static int synthvid_send_config(struct hv_device *hdev)
+{
+	struct fb_info *info = hv_get_drvdata(hdev);
+	struct hvfb_par *par = info->par;
+	struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
+	int t, ret = 0;
+
+	/* Send VRAM location */
+	memset(msg, 0, sizeof(struct synthvid_msg));
+	msg->vid_hdr.type = SYNTHVID_VRAM_LOCATION;
+	msg->vid_hdr.size = sizeof(struct synthvid_msg_hdr) +
+		sizeof(struct synthvid_vram_location);
+	msg->vram.user_ctx = msg->vram.vram_gpa = info->fix.smem_start;
+	msg->vram.is_vram_gpa_specified = 1;
+	synthvid_send(hdev, msg);
+
+	t = wait_for_completion_timeout(&par->wait, VSP_TIMEOUT);
+	if (!t) {
+		pr_err("Time out on waiting vram location ack\n");
+		ret = -ETIMEDOUT;
+		goto out;
+	}
+	if (msg->vram_ack.user_ctx != info->fix.smem_start) {
+		pr_err("Unable to set VRAM location\n");
+		ret = -ENODEV;
+		goto out;
+	}
+
+	/* Send pointer and situation update */
+	synthvid_send_ptr(hdev);
+	synthvid_send_situ(hdev);
+
+out:
+	return ret;
+}
+
+
+/*
+ * Delayed work callback:
+ * It is called at HVFB_UPDATE_DELAY or longer time interval to process
+ * screen updates. It is re-scheduled if further update is necessary.
+ */
+static void hvfb_update_work(struct work_struct *w)
+{
+	struct hvfb_par *par = container_of(w, struct hvfb_par, dwork.work);
+	struct fb_info *info = par->info;
+
+	if (par->fb_ready)
+		synthvid_update(info);
+
+	if (par->update)
+		schedule_delayed_work(&par->dwork, HVFB_UPDATE_DELAY);
+}
+
+
+/* Framebuffer operation handlers */
+
+static int hvfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
+{
+	if (var->xres < HVFB_WIDTH_MIN || var->yres < HVFB_HEIGHT_MIN ||
+	    var->xres > screen_width || var->yres >  screen_height ||
+	    var->bits_per_pixel != screen_depth)
+		return -EINVAL;
+
+	var->xres_virtual = var->xres;
+	var->yres_virtual = var->yres;
+
+	return 0;
+}
+
+static int hvfb_set_par(struct fb_info *info)
+{
+	struct hv_device *hdev = device_to_hv_device(info->device);
+
+	return synthvid_send_situ(hdev);
+}
+
+
+static inline u32 chan_to_field(u32 chan, struct fb_bitfield *bf)
+{
+	return ((chan & 0xffff) >> (16 - bf->length)) << bf->offset;
+}
+
+static int hvfb_setcolreg(unsigned regno, unsigned red, unsigned green,
+			  unsigned blue, unsigned transp, struct fb_info *info)
+{
+	u32 *pal = info->pseudo_palette;
+
+	if (regno > 15)
+		return -EINVAL;
+
+	pal[regno] = chan_to_field(red, &info->var.red)
+		| chan_to_field(green, &info->var.green)
+		| chan_to_field(blue, &info->var.blue)
+		| chan_to_field(transp, &info->var.transp);
+
+	return 0;
+}
+
+
+static struct fb_ops hvfb_ops = {
+	.owner = THIS_MODULE,
+	.fb_check_var = hvfb_check_var,
+	.fb_set_par = hvfb_set_par,
+	.fb_setcolreg = hvfb_setcolreg,
+	.fb_fillrect = cfb_fillrect,
+	.fb_copyarea = cfb_copyarea,
+	.fb_imageblit = cfb_imageblit,
+};
+
+
+/* Get options from kernel paramenter "video=" */
+static void hvfb_get_option(struct fb_info *info)
+{
+	struct hvfb_par *par = info->par;
+	char *opt = NULL, *p;
+	uint x = 0, y = 0;
+
+	if (fb_get_options(KBUILD_MODNAME, &opt) || !opt || !*opt)
+		return;
+
+	p = strsep(&opt, "x");
+	if (!*p || kstrtouint(p, 0, &x) ||
+	    !opt || !*opt || kstrtouint(opt, 0, &y)) {
+		pr_err("Screen option is invalid: skipped\n");
+		return;
+	}
+
+	if (x < HVFB_WIDTH_MIN || y < HVFB_HEIGHT_MIN ||
+	    (par->synthvid_version = SYNTHVID_VERSION_WIN8 &&
+	     x * y * screen_depth / 8 > SYNTHVID_FB_SIZE_WIN8) ||
+	    (par->synthvid_version = SYNTHVID_VERSION_WIN7 &&
+	     (x > SYNTHVID_WIDTH_MAX_WIN7 || y > SYNTHVID_HEIGHT_MAX_WIN7))) {
+		pr_err("Screen resolution option is out of range: skipped\n");
+		return;
+	}
+
+	screen_width = x;
+	screen_height = y;
+	return;
+}
+
+
+/* Get framebuffer memory from Hyper-V video pci space */
+static int hvfb_getmem(struct fb_info *info)
+{
+	struct pci_dev *pdev;
+	ulong fb_phys;
+	void *fb_virt;
+
+	pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
+			      PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
+	if (!pdev) {
+		pr_err("Unable to find PCI Hyper-V video\n");
+		return -ENODEV;
+	}
+
+	if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM) ||
+	    pci_resource_len(pdev, 0) < screen_fb_size)
+		goto err1;
+
+	fb_phys = pci_resource_end(pdev, 0) - screen_fb_size + 1;
+	if (!request_mem_region(fb_phys, screen_fb_size, KBUILD_MODNAME))
+		goto err1;
+
+	fb_virt = ioremap(fb_phys, screen_fb_size);
+	if (!fb_virt)
+		goto err2;
+
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures)
+		goto err3;
+
+	info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
+	info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
+	info->fix.smem_start = fb_phys;
+	info->fix.smem_len = screen_fb_size;
+	info->screen_base = fb_virt;
+	info->screen_size = screen_fb_size;
+
+	pci_dev_put(pdev);
+	return 0;
+
+err3:
+	iounmap(fb_virt);
+err2:
+	release_mem_region(fb_phys, screen_fb_size);
+err1:
+	pci_dev_put(pdev);
+	return -ENOMEM;
+}
+
+/* Release the framebuffer */
+static void hvfb_putmem(struct fb_info *info)
+{
+	iounmap(info->screen_base);
+	release_mem_region(info->fix.smem_start, screen_fb_size);
+}
+
+
+static int hvfb_probe(struct hv_device *hdev,
+		      const struct hv_vmbus_device_id *dev_id)
+{
+	struct fb_info *info;
+	struct hvfb_par *par;
+	int ret;
+
+	info = framebuffer_alloc(sizeof(struct hvfb_par), &hdev->device);
+	if (!info) {
+		pr_err("No memory for framebuffer info\n");
+		return -ENOMEM;
+	}
+
+	par = info->par;
+	par->info = info;
+	par->fb_ready = false;
+	init_completion(&par->wait);
+	INIT_DELAYED_WORK(&par->dwork, hvfb_update_work);
+
+	/* Connect to VSP */
+	hv_set_drvdata(hdev, info);
+	ret = synthvid_connect_vsp(hdev);
+	if (ret) {
+		pr_err("Unable to connect to VSP\n");
+		goto error1;
+	}
+
+	ret = hvfb_getmem(info);
+	if (ret) {
+		pr_err("No memory for framebuffer\n");
+		goto error2;
+	}
+
+	hvfb_get_option(info);
+	pr_info("Screen resolution: %dx%d, Color depth: %d\n",
+		screen_width, screen_height, screen_depth);
+
+
+	/* Set up fb_info */
+	info->flags = FBINFO_DEFAULT;
+
+	info->var.xres_virtual = info->var.xres = screen_width;
+	info->var.yres_virtual = info->var.yres = screen_height;
+	info->var.bits_per_pixel = screen_depth;
+
+	if (info->var.bits_per_pixel = 16) {
+		info->var.red = (struct fb_bitfield){11, 5, 0};
+		info->var.green = (struct fb_bitfield){5, 6, 0};
+		info->var.blue = (struct fb_bitfield){0, 5, 0};
+		info->var.transp = (struct fb_bitfield){0, 0, 0};
+	} else {
+		info->var.red = (struct fb_bitfield){16, 8, 0};
+		info->var.green = (struct fb_bitfield){8, 8, 0};
+		info->var.blue = (struct fb_bitfield){0, 8, 0};
+		info->var.transp = (struct fb_bitfield){24, 8, 0};
+	}
+
+	info->var.activate = FB_ACTIVATE_NOW;
+	info->var.height = -1;
+	info->var.width = -1;
+	info->var.vmode = FB_VMODE_NONINTERLACED;
+
+	strcpy(info->fix.id, KBUILD_MODNAME);
+	info->fix.type = FB_TYPE_PACKED_PIXELS;
+	info->fix.visual = FB_VISUAL_TRUECOLOR;
+	info->fix.line_length = screen_width * screen_depth / 8;
+	info->fix.accel = FB_ACCEL_NONE;
+
+	info->fbops = &hvfb_ops;
+	info->pseudo_palette = par->pseudo_palette;
+
+	/* Send config to host */
+	ret = synthvid_send_config(hdev);
+	if (ret)
+		goto error;
+
+	ret = register_framebuffer(info);
+	if (ret) {
+		pr_err("Unable to register framebuffer\n");
+		goto error;
+	}
+
+	par->fb_ready = true;
+
+	return 0;
+
+error:
+	hvfb_putmem(info);
+error2:
+	vmbus_close(hdev->channel);
+error1:
+	cancel_delayed_work_sync(&par->dwork);
+	hv_set_drvdata(hdev, NULL);
+	framebuffer_release(info);
+	return ret;
+}
+
+
+static int hvfb_remove(struct hv_device *hdev)
+{
+	struct fb_info *info = hv_get_drvdata(hdev);
+	struct hvfb_par *par = info->par;
+
+	par->update = false;
+	par->fb_ready = false;
+
+	unregister_framebuffer(info);
+	cancel_delayed_work_sync(&par->dwork);
+
+	vmbus_close(hdev->channel);
+	hv_set_drvdata(hdev, NULL);
+
+	hvfb_putmem(info);
+	framebuffer_release(info);
+
+	return 0;
+}
+
+
+static const struct hv_vmbus_device_id id_table[] = {
+	/* Synthetic Video Device GUID */
+	{HV_SYNTHVID_GUID},
+	{}
+};
+
+MODULE_DEVICE_TABLE(vmbus, id_table);
+
+static struct hv_driver hvfb_drv = {
+	.name = KBUILD_MODNAME,
+	.id_table = id_table,
+	.probe = hvfb_probe,
+	.remove = hvfb_remove,
+};
+
+
+static int __init hvfb_drv_init(void)
+{
+	return vmbus_driver_register(&hvfb_drv);
+}
+
+static void __exit hvfb_drv_exit(void)
+{
+	vmbus_driver_unregister(&hvfb_drv);
+}
+
+module_init(hvfb_drv_init);
+module_exit(hvfb_drv_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_VERSION(HV_DRV_VERSION);
+MODULE_DESCRIPTION("Microsoft Hyper-V Synthetic Video Frame Buffer Driver");
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index df77ba9..a460ee4 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1253,6 +1253,17 @@ void vmbus_driver_unregister(struct hv_driver *hv_driver);
 		}
 
 /*
+ * Synthetic Video GUID
+ * {DA0A7802-E377-4aac-8E77-0558EB1073F8}
+ */
+#define HV_SYNTHVID_GUID \
+	.guid = { \
+			0x02, 0x78, 0x0a, 0xda, 0x77, 0xe3, 0xac, 0x4a, \
+			0x8e, 0x77, 0x05, 0x58, 0xeb, 0x10, 0x73, 0xf8 \
+		}
+
+
+/*
  * Common header for Hyper-V ICs
  */
 
-- 
1.7.4.1


^ permalink raw reply related


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