Re: [PATCH] ARM: dts: exynos5422-odroid*: remove fimd node

2015-11-23 Thread Javier Martinez Canillas
Hello Krzysztof,

On 11/24/2015 12:50 AM, Krzysztof Kozlowski wrote:
> On 23.11.2015 22:56, Javier Martinez Canillas wrote:
>> Hello Krzysztof,
>>
>> On 11/10/2015 09:43 PM, Krzysztof Kozlowski wrote:
>>
>> [snip]
>>
>>>
>>>
>>> BTW, do you know why we don't have EXYNOS_IOMMU enabled in defconfig?
>>> Any reasons against?
>>>
>>
>> It was explicitly disabled by commit 6562f3bd396a ("ARM: exynos_defconfig:
>> Disable IOMMU support") because Exynos IOMMU support was broken and caused
>> a BUG on boot, the discussion of the patch is [0].
> 
> Right, now I remember.
> 
> 
>> But I just tested booting a v4.4-rc2 kernel on an Exynos5800 Peach Pi with
>> Exynos IOMMU enabled and the machine boots, display is working and
>> /sys/kernel/iommu_grups/*/devices shows that the devices were correctly
>> attached to an IOMMU group so things seems to have been sorted out now.
>>
>> So it seems that EXYNOS_IOMMU could be enabled again. It would be good to
>> give such a patch a spin at kernelci before posting IMHO though just to be
>> sure there are no issues remaining.
> 
> Yes for enabling. No for testing only on kernelci. Booting is not a
> sufficient test in this case. I would expect testing also display - at

Sorry if I didn't explain myself clearly. I didn't mean that kernelci was
enough to test Exynos IOMMU support, what I said is that would be nice to
have some boot coverage besides the normal manual (or automated) display
testing that someone could do on available platforms as discussed over IRC.

> least some frame buffer console on DP or HDMI (or whatever output could
> be generated... Xorg/Wayland would be better of course). You need it

Yes, as I mentioned in the previous email, I tested display (with X) on an
Exynos5800 Peach Pi. I don't have a rootfs with wayland/weston handy but I
could prepare one tomorrow to give a try.

> because display and camera (including complementary modules like JPEG,
> MFC etc) are actually the only users of Exynos IOMMU in mainline.
>

Do you have some test cases for MFC? I know that Gstreamer has support
for it but I don't know what Gst pipelines I can use to test if all is
working correctly.

> Best regards,
> Krzysztof
>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: dts: exynos5422-odroid*: remove fimd node

2015-11-23 Thread Krzysztof Kozlowski
On 23.11.2015 22:56, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 11/10/2015 09:43 PM, Krzysztof Kozlowski wrote:
> 
> [snip]
> 
>>
>>
>> BTW, do you know why we don't have EXYNOS_IOMMU enabled in defconfig?
>> Any reasons against?
>>
> 
> It was explicitly disabled by commit 6562f3bd396a ("ARM: exynos_defconfig:
> Disable IOMMU support") because Exynos IOMMU support was broken and caused
> a BUG on boot, the discussion of the patch is [0].

Right, now I remember.


> But I just tested booting a v4.4-rc2 kernel on an Exynos5800 Peach Pi with
> Exynos IOMMU enabled and the machine boots, display is working and
> /sys/kernel/iommu_grups/*/devices shows that the devices were correctly
> attached to an IOMMU group so things seems to have been sorted out now.
> 
> So it seems that EXYNOS_IOMMU could be enabled again. It would be good to
> give such a patch a spin at kernelci before posting IMHO though just to be
> sure there are no issues remaining.

Yes for enabling. No for testing only on kernelci. Booting is not a
sufficient test in this case. I would expect testing also display - at
least some frame buffer console on DP or HDMI (or whatever output could
be generated... Xorg/Wayland would be better of course). You need it
because display and camera (including complementary modules like JPEG,
MFC etc) are actually the only users of Exynos IOMMU in mainline.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 0/7] drm/exynos: add pm runtime support

2015-11-23 Thread Inki Dae
Hi Javier,

2015년 11월 24일 03:38에 Javier Martinez Canillas 이(가) 쓴 글:
> Hello Inki,
> 
> On 11/23/2015 01:47 PM, Inki Dae wrote:
>> 2015-11-23 21:25 GMT+09:00 Javier Martinez Canillas :
>>> Hello,
>>>
>>> On 11/21/2015 11:59 AM, Inki Dae wrote:
 Hi Daniel,


 2015-11-21 22:40 GMT+09:00 Daniel Stone :
> Hi Inki,
>
> On 21 November 2015 at 09:38, Inki Dae  wrote:
>> 2015-11-21 1:44 GMT+09:00 Javier Martinez Canillas 
>> :
>>> On 11/20/2015 08:13 AM, Inki Dae wrote:
 The boot log says,
 [5.754493] vdd_ldo9: supplied by vdd_2v
 [5.765510] of_graph_get_next_endpoint(): no port node found in 
 /dp-controller@145B

>>>
>>> This message is a red herring for the reported issue, the message is 
>>> also
>>> present when the machine boots and the display is brought correctly.
>>>
 Seems this error is because exynos5800-peach-pit.dts file doesn't have 
 'ports' node in dp node.

 Below is dp node description of exynos5420-peach-pit.dts file.
 &dp {
   status = "okay";
   pinctrl-names = "default";
   pinctrl-0 = <&dp_hpd_gpio>;
   samsung,color-space = <0>;
   samsung,dynamic-range = <0>;
   samsung,ycbcr-coeff = <0>;
   samsung,color-depth = <1>;
   samsung,link-rate = <0x06>;
   samsung,lane-count = <2>;
   samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;

   ports {
   port@0 {
   dp_out: endpoint {
   remote-endpoint = <&bridge_in>;
   };
   };
   };
 };

 And below is for exynos5800-peash-pit.dts,
 &dp {
   status = "okay";
   pinctrl-names = "default";
   pinctrl-0 = <&dp_hpd_gpio>;
   samsung,color-space = <0>;
   samsung,dynamic-range = <0>;
   samsung,ycbcr-coeff = <0>;
   samsung,color-depth = <1>;
   samsung,link-rate = <0x0a>;
   samsung,lane-count = <2>;
   samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
   panel = <&panel>;
 };

>>>
>>> The difference is because the Exynos5420 Peach Pit Display Port is not
>>> attached directly to the display panel, there is an eDP/LVDS bridge chip
>>> in the middle (PS8622) while the Exynos5800 Peach Pi doesn't have that.
>>>
>>> The Exynos DP driver lookups for either a panel phandle or an OF graph
>>> endpoint that points to a bridge chip and the bridge enpoint has a port
>>> that points to the panel.
>>>
>>> So the DT is correct but of_graph_get_next_endpoint() always prints an
>>
>> Then, the DT is really incorrect. As you mentioned, if the Exynos5800 
>> Peach PI
>> board doesn't use eDP, then the dp node __should be removed__ from
>> exynos5800-peach-pit.dts.
>>
>> From a common-sense standpoint, there is no any reason to build
>> and probe dp driver if the board doesn't use dp hardware.
>
> I agree with what you say, but unfortunately you've slightly misread
> what Javier has said. :) exynos5420-peach-pit has an LVDS panel, with
> the eDP -> LVDS bridge in between (ps8622). exynos5800-peach-pi (from
> which I am writing this) has an eDP panel directly connected. The DT
>>>
>>> Thanks a lot Daniel for clarifying my comments to Inki :)
>>>
> describes both the eDP connector from FIMD and the eDP panel, except
> that there is no connection between the DT nodes.
>>>
>>> There *is* a connection between the FIMD eDP connector and the eDP panel
>>> nodes but these are connected using a phandle while the connection for
>>> the FIMD eDP connector and the eDP/LVDS bridge is using the OF graph DT
>>> bindings (Documentation/devicetree/bindings/graph.txt).
>>>
>>> And also the connection between the eDP/LVDS bridge and the LVDS panel
>>> is using an OF graph node, so what I meant is that it would be much more
>>> consistent if both the eDP -> panel and eDP -> eDP/LVDS bridge -> panel
>>> connections used the OF graph DT bindings.
>>>

 Right. I misread what Javier said. :)

>
>>> error if the port so OF graph endpoints it seems can't be optional as
>>> used in this driver. Maybe that message should be change to debug then?
>>>
>>> Another option is to extend the DP driver DT binding to be more generic
>>> supporting having a port to a panel besides a bridge, so we could have
>>> something like this for Exynos5800 Peach and be consistent in both 
>>> cases:
>>
>> It's really not good. This would make it more complex. The best
>> solution is just to
>> remove the dt node from the device tree file.
>
> Given the a

Re: [PATCH] thermal: exynos: use of_property_read_u8()

2015-11-23 Thread Krzysztof Kozlowski
On 24.11.2015 08:29, Eduardo Valentin wrote:
> On Mon, Nov 23, 2015 at 07:40:41PM +0530, Saurabh Sengar wrote:
>> use of_property_read_u8() for u8 variables,
>> also changed the return type to void as this function return type
>> is nowhere used.
>>
> 
> I would be good if you could split both changes into two patches.
> 
> Also, I still do not understand properly the proposal to use u8.
> 
> Why are they u8?
> 
> If they are so, please document them in their DT bindings. Today, the
> bindings tell us they are s32 (as there is nothing really specifying the
> type).

It looks like u8 came from platform data from old times when the driver
did not support DT. All of our Exynos mainline boards are converted to
DT, so I think legacy platform data can be also changed to match DT.
Which means changing from u8 to u32.

Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] thermal: exynos: use of_property_read_u8()

2015-11-23 Thread Eduardo Valentin
On Mon, Nov 23, 2015 at 07:40:41PM +0530, Saurabh Sengar wrote:
> use of_property_read_u8() for u8 variables,
> also changed the return type to void as this function return type
> is nowhere used.
> 

I would be good if you could split both changes into two patches.

Also, I still do not understand properly the proposal to use u8.

Why are they u8?

If they are so, please document them in their DT bindings. Today, the
bindings tell us they are s32 (as there is nothing really specifying the
type).


BR


> Signed-off-by: Saurabh Sengar 
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 30 +-
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c 
> b/drivers/thermal/samsung/exynos_tmu.c
> index fa61eff..b692c95 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -1128,20 +1128,16 @@ static int exynos_of_get_soc_type(struct device_node 
> *np)
>   return -EINVAL;
>  }
>  
> -static int exynos_of_sensor_conf(struct device_node *np,
> +static void exynos_of_sensor_conf(struct device_node *np,
>struct exynos_tmu_platform_data *pdata)
>  {
> - u32 value;
> - int ret;
> -
>   of_node_get(np);
>  
> - ret = of_property_read_u32(np, "samsung,tmu_gain", &value);
> - pdata->gain = (u8)value;
> - of_property_read_u32(np, "samsung,tmu_reference_voltage", &value);
> - pdata->reference_voltage = (u8)value;
> - of_property_read_u32(np, "samsung,tmu_noise_cancel_mode", &value);
> - pdata->noise_cancel_mode = (u8)value;
> + of_property_read_u8(np, "samsung,tmu_gain", &pdata->gain);
> + of_property_read_u8(np, "samsung,tmu_reference_voltage",
> + &pdata->reference_voltage);
> + of_property_read_u8(np, "samsung,tmu_noise_cancel_mode",
> + &pdata->noise_cancel_mode);
>  
>   of_property_read_u32(np, "samsung,tmu_efuse_value",
>&pdata->efuse_value);
> @@ -1150,18 +1146,18 @@ static int exynos_of_sensor_conf(struct device_node 
> *np,
>   of_property_read_u32(np, "samsung,tmu_max_efuse_value",
>&pdata->max_efuse_value);
>  
> - of_property_read_u32(np, "samsung,tmu_first_point_trim", &value);
> - pdata->first_point_trim = (u8)value;
> - of_property_read_u32(np, "samsung,tmu_second_point_trim", &value);
> - pdata->second_point_trim = (u8)value;
> - of_property_read_u32(np, "samsung,tmu_default_temp_offset", &value);
> - pdata->default_temp_offset = (u8)value;
> + of_property_read_u8(np, "samsung,tmu_first_point_trim",
> + &pdata->first_point_trim);
> + of_property_read_u8(np, "samsung,tmu_second_point_trim",
> + &pdata->second_point_trim);
> + of_property_read_u8(np, "samsung,tmu_default_temp_offset",
> + &pdata->default_temp_offset);
>  
>   of_property_read_u32(np, "samsung,tmu_cal_type", &pdata->cal_type);
>   of_property_read_u32(np, "samsung,tmu_cal_mode", &pdata->cal_mode);
>  
>   of_node_put(np);
> - return 0;
> + return;

The landing return; is not needed.

>  }
>  
>  static int exynos_map_dt_data(struct platform_device *pdev)
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V3 5/5] ARM: dts: exynos4412: Rename OPP nodes as opp@

2015-11-23 Thread Rafael J. Wysocki
On Tuesday, November 24, 2015 12:04:00 AM Rafael J. Wysocki wrote:
> On Wednesday, November 11, 2015 08:10:58 AM Viresh Kumar wrote:
> > OPP bindings got updated to name OPP nodes this way, make changes
> > according to that.
> > 
> > Reviewed-by: Krzysztof Kozlowski 
> > Signed-off-by: Viresh Kumar 
> > ---
> >  arch/arm/boot/dts/exynos4412.dtsi | 28 ++--
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/exynos4412.dtsi 
> > b/arch/arm/boot/dts/exynos4412.dtsi
> > index 294cfe40388d..40beede46e55 100644
> > --- a/arch/arm/boot/dts/exynos4412.dtsi
> > +++ b/arch/arm/boot/dts/exynos4412.dtsi
> > @@ -64,73 +64,73 @@
> > compatible = "operating-points-v2";
> > opp-shared;
> >  
> > -   opp00 {
> > +   opp@2 {
> > opp-hz = /bits/ 64 <2>;
> > opp-microvolt = <90>;
> > clock-latency-ns = <20>;
> > };
> > -   opp01 {
> > +   opp@3 {
> > opp-hz = /bits/ 64 <3>;
> > opp-microvolt = <90>;
> > clock-latency-ns = <20>;
> > };
> > -   opp02 {
> > +   opp@4 {
> > opp-hz = /bits/ 64 <4>;
> > opp-microvolt = <925000>;
> > clock-latency-ns = <20>;
> > };
> > -   opp03 {
> > +   opp@5 {
> > opp-hz = /bits/ 64 <5>;
> > opp-microvolt = <95>;
> > clock-latency-ns = <20>;
> > };
> > -   opp04 {
> > +   opp@6 {
> > opp-hz = /bits/ 64 <6>;
> > opp-microvolt = <975000>;
> > clock-latency-ns = <20>;
> > };
> > -   opp05 {
> > +   opp@7 {
> > opp-hz = /bits/ 64 <7>;
> > opp-microvolt = <987500>;
> > clock-latency-ns = <20>;
> > };
> > -   opp06 {
> > +   opp@8 {
> > opp-hz = /bits/ 64 <8>;
> > opp-microvolt = <100>;
> > clock-latency-ns = <20>;
> > opp-suspend;
> > };
> > -   opp07 {
> > +   opp@9 {
> > opp-hz = /bits/ 64 <9>;
> > opp-microvolt = <1037500>;
> > clock-latency-ns = <20>;
> > };
> > -   opp08 {
> > +   opp@10 {
> > opp-hz = /bits/ 64 <10>;
> > opp-microvolt = <1087500>;
> > clock-latency-ns = <20>;
> > };
> > -   opp09 {
> > +   opp@11 {
> > opp-hz = /bits/ 64 <11>;
> > opp-microvolt = <1137500>;
> > clock-latency-ns = <20>;
> > };
> > -   opp10 {
> > +   opp@12 {
> > opp-hz = /bits/ 64 <12>;
> > opp-microvolt = <1187500>;
> > clock-latency-ns = <20>;
> > };
> > -   opp11 {
> > +   opp@13 {
> > opp-hz = /bits/ 64 <13>;
> > opp-microvolt = <125>;
> > clock-latency-ns = <20>;
> > };
> > -   opp12 {
> > +   opp@14 {
> > opp-hz = /bits/ 64 <14>;
> > opp-microvolt = <1287500>;
> > clock-latency-ns = <20>;
> > };
> > -   opp13 {
> > +   opp@15 {
> > opp-hz = /bits/ 64 <15>;
> > opp-microvolt = <135>;
> > clock-latency-ns = <20>;
> > 
> 
> This one doesn't apply for me on top of the current Linus' tree, so please
> update.

Scratch that, my bad.  Sorry.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V3 5/5] ARM: dts: exynos4412: Rename OPP nodes as opp@

2015-11-23 Thread Rafael J. Wysocki
On Wednesday, November 11, 2015 08:10:58 AM Viresh Kumar wrote:
> OPP bindings got updated to name OPP nodes this way, make changes
> according to that.
> 
> Reviewed-by: Krzysztof Kozlowski 
> Signed-off-by: Viresh Kumar 
> ---
>  arch/arm/boot/dts/exynos4412.dtsi | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi 
> b/arch/arm/boot/dts/exynos4412.dtsi
> index 294cfe40388d..40beede46e55 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -64,73 +64,73 @@
>   compatible = "operating-points-v2";
>   opp-shared;
>  
> - opp00 {
> + opp@2 {
>   opp-hz = /bits/ 64 <2>;
>   opp-microvolt = <90>;
>   clock-latency-ns = <20>;
>   };
> - opp01 {
> + opp@3 {
>   opp-hz = /bits/ 64 <3>;
>   opp-microvolt = <90>;
>   clock-latency-ns = <20>;
>   };
> - opp02 {
> + opp@4 {
>   opp-hz = /bits/ 64 <4>;
>   opp-microvolt = <925000>;
>   clock-latency-ns = <20>;
>   };
> - opp03 {
> + opp@5 {
>   opp-hz = /bits/ 64 <5>;
>   opp-microvolt = <95>;
>   clock-latency-ns = <20>;
>   };
> - opp04 {
> + opp@6 {
>   opp-hz = /bits/ 64 <6>;
>   opp-microvolt = <975000>;
>   clock-latency-ns = <20>;
>   };
> - opp05 {
> + opp@7 {
>   opp-hz = /bits/ 64 <7>;
>   opp-microvolt = <987500>;
>   clock-latency-ns = <20>;
>   };
> - opp06 {
> + opp@8 {
>   opp-hz = /bits/ 64 <8>;
>   opp-microvolt = <100>;
>   clock-latency-ns = <20>;
>   opp-suspend;
>   };
> - opp07 {
> + opp@9 {
>   opp-hz = /bits/ 64 <9>;
>   opp-microvolt = <1037500>;
>   clock-latency-ns = <20>;
>   };
> - opp08 {
> + opp@10 {
>   opp-hz = /bits/ 64 <10>;
>   opp-microvolt = <1087500>;
>   clock-latency-ns = <20>;
>   };
> - opp09 {
> + opp@11 {
>   opp-hz = /bits/ 64 <11>;
>   opp-microvolt = <1137500>;
>   clock-latency-ns = <20>;
>   };
> - opp10 {
> + opp@12 {
>   opp-hz = /bits/ 64 <12>;
>   opp-microvolt = <1187500>;
>   clock-latency-ns = <20>;
>   };
> - opp11 {
> + opp@13 {
>   opp-hz = /bits/ 64 <13>;
>   opp-microvolt = <125>;
>   clock-latency-ns = <20>;
>   };
> - opp12 {
> + opp@14 {
>   opp-hz = /bits/ 64 <14>;
>   opp-microvolt = <1287500>;
>   clock-latency-ns = <20>;
>   };
> - opp13 {
> + opp@15 {
>   opp-hz = /bits/ 64 <15>;
>   opp-microvolt = <135>;
>   clock-latency-ns = <20>;
> 

This one doesn't apply for me on top of the current Linus' tree, so please
update.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pwm: Avoid double mutex lock on pwm_enable

2015-11-23 Thread Jonathan Richardson
On 15-11-23 02:07 AM, Thierry Reding wrote:
> On Sun, Nov 22, 2015 at 09:13:17AM +0900, Krzysztof Kozlowski wrote:
>> 2015-11-22 3:14 GMT+09:00 Anand Moon :
>>> Hi Krzysztof,
>>>
>>> On 21 November 2015 at 18:37, Krzysztof Kozlowski
>>>  wrote:
 2015-11-21 21:11 GMT+09:00 Anand Moon :
> hi Krzysztof,
>
> On 21 November 2015 at 15:22, Krzysztof Kozlowski
>  wrote:
>> 2015-11-21 18:40 GMT+09:00 Anand Moon :
>>> hi Krzysztof,
>>>
>>> On 21 November 2015 at 09:56, Krzysztof Kozlowski 
>>> 
>>> wrote:

 W dniu 21.11.2015 o 01:59, Anand Moon pisze:
> Commit d1cd21427747f15920cd726f5f67a07880e7dee4
> (pwm: Set enable state properly on failed call to enable)
> introduce double lock of mutex on pwm_enable
>
> Changes fix the following debug lock warning.
>
> [2.701720] WARNING: CPU: 3 PID: 0 at kernel/locking/mutex.c:526
> __mutex_lock_slowpath+0x3bc/0x404()
> [2.701731] DEBUG_LOCKS_WARN_ON(in_interrupt())

 Hi Anand!

 Yeah, I am hitting this as well. Annoying. However your description is
 inaccurate. Double lock of mutex does not cause warnings of sleeping or
 locking in atomic context (if you build with debug sleep atomic you 
 will
 see different warning). More over you are partially reverting the 
 commit
 d1cd21427747 ("pwm: Set enable state properly on failed call to 
 enable")
 without proper explanation:
 1. why this locking is inappropriate;
 2. why it is safe to remove the mutex_lock().

 Please find the real problem, fix the real problem and throughly 
 explain
 the real issue.

 I was suspecting some weird changes in timers. For !irqsafe timer I
 think it is safe to use mutexes... but apparently not. Maybe a work
 should be scheduled from function called by timer?

 Warning with debug atomic sleep:
 [   16.047517] BUG: sleeping function called from invalid context at
 ../kernel/locking/mutex.c:617
 [   16.054866] in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: 
 swapper/0
 [   16.061402] INFO: lockdep is turned off.
 [   16.065281] Preemption disabled at:[<  (null)>]   (null)
 [   16.070524]
 [   16.072002] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
 4.4.0-rc1-00040-g28c429565d4f #290
 [   16.080150] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
 [   16.086215] [] (unwind_backtrace) from []
 (show_stack+0x10/0x14)
 [   16.093938] [] (show_stack) from []
 (dump_stack+0x70/0xbc)
 [   16.101122] [] (dump_stack) from []
 (mutex_lock_nested+0x24/0x474)
 [   16.109009] [] (mutex_lock_nested) from []
 (pwm_enable+0x20/0x7c)
 [   16.116799] [] (pwm_enable) from []
 (led_heartbeat_function+0xdc/0x140)
 [   16.125119] [] (led_heartbeat_function) from []
 (call_timer_fn+0x6c/0xf4)
 [   16.133606] [] (call_timer_fn) from []
 (run_timer_softirq+0x1a8/0x230)
 [   16.141846] [] (run_timer_softirq) from []
 (__do_softirq+0x134/0x2c0)
 [   16.149982] [] (__do_softirq) from []
 (irq_exit+0xd0/0x114)
 [   16.157255] [] (irq_exit) from []
 (__handle_domain_irq+0x70/0xe4)
 [   16.165056] [] (__handle_domain_irq) from []
 (gic_handle_irq+0x4c/0x94)
 [   16.173376] [] (gic_handle_irq) from []
 (__irq_svc+0x58/0x98)
 [   16.180817] Exception stack(0xc08cdf58 to 0xc08cdfa0)
 [   16.185823] df40:
c0010dcc 
 [   16.193997] df60: c08cdfa8  c05f57c4  c08ca520
 c08ce4bc c08c7364 c08ce4b4
 [   16.202141] df80: c09121ca  0001 c08cdfa8 c0010dcc
 c0010dd0 600f0013 
 [   16.210291] [] (__irq_svc) from []
 (arch_cpu_idle+0x20/0x3c)
 [   16.217661] [] (arch_cpu_idle) from []
 (cpu_startup_entry+0x17c/0x26c)
 [   16.225899] [] (cpu_startup_entry) from []
 (start_kernel+0x368/0x3d0)

 Best regards,
 Krzysztof


> [2.701737] Modules linked in:
> [2.701748] CPU: 3 PID: 0 Comm: swapper/3 Not tainted 
> 4.4.0-rc1-xu4f
> #24
> [2.701753] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [2.701787] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [2.701808] [] (show_stack) from []
> (dump_stack+0x84/0xc4)
> [2.701824] [] (dump_stack) from []
> (warn_slowpath_common+0x80/0xb0)
> [2.701836] [] (warn_slowpath_common) from []
> (warn_slowpath_fmt+0x30/0x40)
> [2.701849] [] (warn_slowpath_fmt) from []
>

Re: [PATCH v2 0/7] drm/exynos: add pm runtime support

2015-11-23 Thread Javier Martinez Canillas
Hello Inki,

On 11/23/2015 01:47 PM, Inki Dae wrote:
> 2015-11-23 21:25 GMT+09:00 Javier Martinez Canillas :
>> Hello,
>>
>> On 11/21/2015 11:59 AM, Inki Dae wrote:
>>> Hi Daniel,
>>>
>>>
>>> 2015-11-21 22:40 GMT+09:00 Daniel Stone :
 Hi Inki,

 On 21 November 2015 at 09:38, Inki Dae  wrote:
> 2015-11-21 1:44 GMT+09:00 Javier Martinez Canillas 
> :
>> On 11/20/2015 08:13 AM, Inki Dae wrote:
>>> The boot log says,
>>> [5.754493] vdd_ldo9: supplied by vdd_2v
>>> [5.765510] of_graph_get_next_endpoint(): no port node found in 
>>> /dp-controller@145B
>>>
>>
>> This message is a red herring for the reported issue, the message is also
>> present when the machine boots and the display is brought correctly.
>>
>>> Seems this error is because exynos5800-peach-pit.dts file doesn't have 
>>> 'ports' node in dp node.
>>>
>>> Below is dp node description of exynos5420-peach-pit.dts file.
>>> &dp {
>>>   status = "okay";
>>>   pinctrl-names = "default";
>>>   pinctrl-0 = <&dp_hpd_gpio>;
>>>   samsung,color-space = <0>;
>>>   samsung,dynamic-range = <0>;
>>>   samsung,ycbcr-coeff = <0>;
>>>   samsung,color-depth = <1>;
>>>   samsung,link-rate = <0x06>;
>>>   samsung,lane-count = <2>;
>>>   samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
>>>
>>>   ports {
>>>   port@0 {
>>>   dp_out: endpoint {
>>>   remote-endpoint = <&bridge_in>;
>>>   };
>>>   };
>>>   };
>>> };
>>>
>>> And below is for exynos5800-peash-pit.dts,
>>> &dp {
>>>   status = "okay";
>>>   pinctrl-names = "default";
>>>   pinctrl-0 = <&dp_hpd_gpio>;
>>>   samsung,color-space = <0>;
>>>   samsung,dynamic-range = <0>;
>>>   samsung,ycbcr-coeff = <0>;
>>>   samsung,color-depth = <1>;
>>>   samsung,link-rate = <0x0a>;
>>>   samsung,lane-count = <2>;
>>>   samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
>>>   panel = <&panel>;
>>> };
>>>
>>
>> The difference is because the Exynos5420 Peach Pit Display Port is not
>> attached directly to the display panel, there is an eDP/LVDS bridge chip
>> in the middle (PS8622) while the Exynos5800 Peach Pi doesn't have that.
>>
>> The Exynos DP driver lookups for either a panel phandle or an OF graph
>> endpoint that points to a bridge chip and the bridge enpoint has a port
>> that points to the panel.
>>
>> So the DT is correct but of_graph_get_next_endpoint() always prints an
>
> Then, the DT is really incorrect. As you mentioned, if the Exynos5800 
> Peach PI
> board doesn't use eDP, then the dp node __should be removed__ from
> exynos5800-peach-pit.dts.
>
> From a common-sense standpoint, there is no any reason to build
> and probe dp driver if the board doesn't use dp hardware.

 I agree with what you say, but unfortunately you've slightly misread
 what Javier has said. :) exynos5420-peach-pit has an LVDS panel, with
 the eDP -> LVDS bridge in between (ps8622). exynos5800-peach-pi (from
 which I am writing this) has an eDP panel directly connected. The DT
>>
>> Thanks a lot Daniel for clarifying my comments to Inki :)
>>
 describes both the eDP connector from FIMD and the eDP panel, except
 that there is no connection between the DT nodes.
>>
>> There *is* a connection between the FIMD eDP connector and the eDP panel
>> nodes but these are connected using a phandle while the connection for
>> the FIMD eDP connector and the eDP/LVDS bridge is using the OF graph DT
>> bindings (Documentation/devicetree/bindings/graph.txt).
>>
>> And also the connection between the eDP/LVDS bridge and the LVDS panel
>> is using an OF graph node, so what I meant is that it would be much more
>> consistent if both the eDP -> panel and eDP -> eDP/LVDS bridge -> panel
>> connections used the OF graph DT bindings.
>>
>>>
>>> Right. I misread what Javier said. :)
>>>

>> error if the port so OF graph endpoints it seems can't be optional as
>> used in this driver. Maybe that message should be change to debug then?
>>
>> Another option is to extend the DP driver DT binding to be more generic
>> supporting having a port to a panel besides a bridge, so we could have
>> something like this for Exynos5800 Peach and be consistent in both cases:
>
> It's really not good. This would make it more complex. The best
> solution is just to
> remove the dt node from the device tree file.

 Given the above, not really. Javier's patch seems correct to me - as
 you can see, there is a panel node, and that is the panel that's
 really connected.
>>>
>>> Indeed. Javier's patch will correct it.
>>>
>>
>> J

Re: [PATCH v2 4/5] drm/exynos: mixer: do blending setup in mixer_cfg_layer()

2015-11-23 Thread Tobias Jakobi
Hey Inki,


Inki Dae wrote:
> 
> 
> 2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
>> This updates the blending setup when the layer configuration
>> changes (triggered by mixer_win_{commit,disable}).
>>
>> To avoid unnecesary reconfigurations we cache the layer
>> state in the mixer context.
>>
>> Extra care has to be taken for the layer that is currently
>> being enabled/disabled.
>>
>> Signed-off-by: Tobias Jakobi 
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 41 
>> +--
>>  1 file changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index ec9659e..1c24fb5 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -99,6 +99,7 @@ struct mixer_context {
>>  struct exynos_drm_plane planes[MIXER_WIN_NR];
>>  const struct layer_cfg  *layer_cfg;
>>  unsigned intnum_layer;
>> +u32 layer_state;
>>  int pipe;
>>  unsigned long   flags;
>>  boolinterlace;
>> @@ -189,6 +190,27 @@ static inline bool is_alpha_format(const struct 
>> mixer_context* ctx, unsigned int
>>  }
>>  }
>>  
>> +static inline u32 get_layer_state(const struct mixer_context *ctx,
>> +unsigned int win, bool enable)
>> +{
>> +u32 enable_state, alpha_state;
>> +
>> +enable_state = ctx->layer_state & 0x;
>> +alpha_state = ctx->layer_state >> 16;
>> +
>> +if (enable)
>> +enable_state |= (1 << win);
>> +else
>> +enable_state &= ~(1 << win);
>> +
>> +if (enable && is_alpha_format(ctx, win))
>> +alpha_state |= (1 << win);
>> +else
>> +alpha_state &= ~(1 << win);
>> +
>> +return ((alpha_state << 16) | enable_state);
>> +}
>> +
>>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>>  {
>>  return readl(res->vp_regs + reg_id);
>> @@ -370,8 +392,9 @@ static void mixer_general_layer(struct mixer_context 
>> *ctx,
>>  {
>>  u32 val;
>>  struct mixer_resources *res = &ctx->mixer_res;
>> +const u32 alpha_state = ctx->layer_state >> 16;
>>  
>> -if (is_alpha_format(ctx, cfg->index)) {
>> +if (alpha_state & (1 << cfg->index)) {
>>  val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>>  val |= MXR_GRP_CFG_BLEND_PRE_MUL;
>>  val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel 
>> alpha */
>> @@ -397,10 +420,11 @@ static void mixer_general_layer(struct mixer_context 
>> *ctx,
>>  }
>>  }
>>  
>> -static void mixer_layer_blending(struct mixer_context *ctx, unsigned int 
>> enable_state)
>> +static void mixer_layer_blending(struct mixer_context *ctx)
>>  {
>>  unsigned int i, index;
>>  bool bottom_layer = false;
>> +const u32 enable_state = ctx->layer_state & 0x;
>>  
>>  for (i = 0; i < ctx->num_layer; ++i) {
>>  index = ctx->layer_cfg[i].index;
>> @@ -503,8 +527,19 @@ static void mixer_cfg_layer(struct mixer_context *ctx, 
>> unsigned int win,
>>  bool enable)
>>  {
>>  struct mixer_resources *res = &ctx->mixer_res;
>> +u32 new_layer_state;
>>  u32 val = enable ? ~0 : 0;
>>  
>> +new_layer_state = get_layer_state(ctx, win, enable);
>> +if (new_layer_state == ctx->layer_state)
>> +return;
>> +
>> +/*
>> + * Update the layer state so that mixer_layer_blending()
>> + * below can use it.
>> + */
>> +ctx->layer_state = new_layer_state;
> 
> It may be trivial but I think it'd be better to move above line to most 
> bottom of this function.
Sure, I can do that, but I would rather know what's the rationale here.


With best wishes,
Tobias

>> +
>>  switch (win) {
>>  case 0:
>>  mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
>> @@ -520,6 +555,8 @@ static void mixer_cfg_layer(struct mixer_context *ctx, 
>> unsigned int win,
>>  }
>>  break;
>>  }
>> +
>> +mixer_layer_blending(ctx);
> 
> Here.
> 
> Thanks,
> Inki Dae
> 
>>  }
>>  
>>  static void mixer_run(struct mixer_context *ctx)
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/5] drm/exynos: mixer: introduce mixer_layer_blending()

2015-11-23 Thread Tobias Jakobi
Hey Inki,


Inki Dae wrote:
> 
> 
> 2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
>> This analyses the current layer configuration (which layers
>> are enabled, which have alpha-pixelformat, etc.) and setups
>> blending accordingly.
>>
>> We currently disable all kinds of blending for the bottom-most
>> layer, since configuration of the mixer background is not
>> yet exposed.
>> Also blending is only enabled when the layer has a pixelformat
>> with alpha attached.
>>
>> Signed-off-by: Tobias Jakobi 
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 88 
>> +++
>>  drivers/gpu/drm/exynos/regs-mixer.h   |  1 +
>>  2 files changed, 89 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 2c1cea3..ec77aad 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -174,6 +174,21 @@ static const u8 filter_cr_horiz_tap4[] = {
>>  70, 59, 48, 37, 27, 19, 11, 5,
>>  };
>>  
>> +static inline bool is_alpha_format(const struct mixer_context* ctx, 
>> unsigned int win)
>> +{
>> +const struct drm_plane_state *state = ctx->planes[win].base.state;
>> +const struct drm_framebuffer *fb = state->fb;
>> +
>> +switch (fb->pixel_format) {
>> +case DRM_FORMAT_ARGB:
>> +case DRM_FORMAT_ARGB1555:
>> +case DRM_FORMAT_ARGB:
>> +return true;
>> +default:
>> +return false;
>> +}
>> +}
>> +
>>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>>  {
>>  return readl(res->vp_regs + reg_id);
>> @@ -331,6 +346,79 @@ static void mixer_layer_priority(struct mixer_context 
>> *ctx)
>>  mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val);
>>  }
>>  
>> +/* Configure blending for bottom-most layer. */
>> +static void mixer_bottom_layer(struct mixer_context *ctx,
>> +const struct layer_cfg *cfg)
>> +{
>> +u32 val;
>> +struct mixer_resources *res = &ctx->mixer_res;
>> +
>> +if (cfg->index == 2) {
>> +val = 0; /* use defaults for video layer */
>> +mixer_reg_write(res, MXR_VIDEO_CFG, val);
>> +} else {
>> +val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>> +
>> +/* Don't blend bottom-most layer onto the mixer background. */
>> +mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
>> +val, MXR_GRP_CFG_MISC_MASK);
>> +}
>> +}
>> +
>> +static void mixer_general_layer(struct mixer_context *ctx,
>> +const struct layer_cfg *cfg)
>> +{
>> +u32 val;
>> +struct mixer_resources *res = &ctx->mixer_res;
>> +
>> +if (is_alpha_format(ctx, cfg->index)) {
>> +val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>> +val |= MXR_GRP_CFG_BLEND_PRE_MUL;
>> +val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel 
>> alpha */
>> +
>> +/* The video layer never has an alpha pixelformat. */
> 
> Looks like above comment isn't related to graphics layer.
Why do you think so? Because it certainly is.


>> +mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
>> +val, MXR_GRP_CFG_MISC_MASK);
>> +} else {
>> +if (cfg->index == 2) {
>> +/*
>> + * No blending at the moment since the NV12/NV21 
>> pixelformats don't
>> + * have an alpha channel. However the mixer supports a 
>> global alpha
>> + * value for a layer. Once this functionality is 
>> exposed, we can
>> + * support blending of the video layer through this.
>> + */
>> +val = 0;
>> +mixer_reg_write(res, MXR_VIDEO_CFG, val);
> 
> Above 'if statement' wouldn't be called because cfg->index has always a value 
> less than 2
> so it should be removed.
Can you explain why you think that index is always < 2 here?


>> +} else {
>> +val = MXR_GRP_CFG_COLOR_KEY_DISABLE;
>> +mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
>> +val, MXR_GRP_CFG_MISC_MASK);
>> +}
>> +}
>> +}
>> +
>> +static void mixer_layer_blending(struct mixer_context *ctx, unsigned int 
>> enable_state)
>> +{
>> +unsigned int i, index;
>> +bool bottom_layer = false;
>> +
>> +for (i = 0; i < ctx->num_layer; ++i) {
>> +index = ctx->layer_cfg[i].index;
>> +
>> +/* Skip layer if it's not enabled. */
>> +if (!(enable_state & (1 << index)))
>> +continue;
>> +
>> +/* Bottom layer needs special handling. */
>> +if (bottom_layer) {
>> +mixer_general_layer(ctx, &ctx->layer_cfg[i]);
>> +} else {
>> +mixer_bottom_layer(ctx

Re: [PATCH v2 1/5] drm/exynos: mixer: refactor layer setup

2015-11-23 Thread Tobias Jakobi
Hey Inki,


Inki Dae wrote:
> Hi Tobias,
> 
> 2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
>> First step in allowing a more generic way to setup complex
>> blending for the different layers.
>>
>> Signed-off-by: Tobias Jakobi 
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 84 
>> ++-
>>  1 file changed, 73 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 7498c6e..2c1cea3 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -63,6 +63,11 @@ struct mixer_resources {
>>  struct clk  *mout_mixer;
>>  };
>>  
>> +struct layer_cfg {
>> +unsigned int index;
>> +unsigned int priority;
>> +};
>> +
>>  enum mixer_version_id {
>>  MXR_VER_0_0_0_16,
>>  MXR_VER_16_0_33_0,
>> @@ -92,6 +97,8 @@ struct mixer_context {
>>  struct drm_device   *drm_dev;
>>  struct exynos_drm_crtc  *crtc;
>>  struct exynos_drm_plane planes[MIXER_WIN_NR];
>> +const struct layer_cfg  *layer_cfg;
>> +unsigned intnum_layer;
>>  int pipe;
>>  unsigned long   flags;
>>  boolinterlace;
>> @@ -110,6 +117,34 @@ struct mixer_drv_data {
>>  boolhas_sclk;
>>  };
>>  
>> +/*
>> + * The default layer priorities for non-VP (video processor)
>> + * and VP mixer configurations.
>> + *
>> + * A higher priority means that the layer is at the top of
>> + * the layer stack.
>> + * Configurations have to be specified with its entries
>> + * sorted with increasing priority.
>> + *
>> + * The default config assumes the following usage scenario:
>> + * layer1: OSD [top]
>> + * layer0: main framebuffer
>> + * video layer: video overlay [bottom]
>> + * Note that the video layer is only usable when the
>> + * VP is available.
>> + */
>> +
>> +static const struct layer_cfg nonvp_default_cfg[] = {
>> +{ .index = 0, .priority = 1 },  /* layer0 */
>> +{ .index = 1, .priority = 2 },  /* layer1 */
>> +};
>> +
>> +static const struct layer_cfg vp_default_cfg[] = {
>> +{ .index = 2, .priority = 1 },  /* video layer */
>> +{ .index = 0, .priority = 2 },  /* layer0 */
>> +{ .index = 1, .priority = 3 },  /* layer1 */
>> +};
>> +
>>  static const u8 filter_y_horiz_tap8[] = {
>>  0,  -1, -1, -1, -1, -1, -1, -1,
>>  -1, -1, -1, -1, -1, 0,  0,  0,
>> @@ -268,6 +303,34 @@ static void vp_default_filter(struct mixer_resources 
>> *res)
>>  filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4));
>>  }
>>  
>> +static void mixer_layer_priority(struct mixer_context *ctx)
>> +{
>> +u32 val = 0;
>> +unsigned int i, priority;
>> +
>> +for (i = 0; i < ctx->num_layer; ++i) {
>> +priority = ctx->layer_cfg[i].priority;
>> +BUG_ON(priority > 15);
> 
> What doesn constant, 15 mean? You need to clarify the meaning
> and please, use a macro instead.
If I read the specs correctly the layer priority is encoded with just
4-bits in the corresponding register. I'll add a define to make this
more clear.


> And it'd better to just return in this case so that the layer
> configuration can be set with a default value.
The point is that these (currently) are the default values that
mixer_layer_priority() sets.
The BUG_ON() is there to make sure that nobody changes
{vp,nonvp}_default_cfg[] later with illegal values.


With best wishes,
Tobias


> I think it'd be enough to print out warning message.
> 
> Thanks,
> Inki Dae
> 
>> +
>> +switch (ctx->layer_cfg[i].index) {
>> +case 0:
>> +val |= MXR_LAYER_CFG_GRP0_VAL(priority);
>> +break;
>> +case 1:
>> +val |= MXR_LAYER_CFG_GRP1_VAL(priority);
>> +break;
>> +case 2:
>> +val |= MXR_LAYER_CFG_VP_VAL(priority);
>> +break;
>> +default:
>> +BUG_ON(true);
>> +break;
>> +}
>> +}
>> +
>> +mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val);
>> +}
>> +
>>  static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
>>  {
>>  struct mixer_resources *res = &ctx->mixer_res;
>> @@ -673,17 +736,7 @@ static void mixer_win_reset(struct mixer_context *ctx)
>>  mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST,
>>  MXR_STATUS_BURST_MASK);
>>  
>> -/* setting default layer priority: layer1 > layer0 > video
>> - * because typical usage scenario would be
>> - * layer1 - OSD
>> - * layer0 - framebuffer
>> - * video - video overlay
>> - */
>> -val = MXR_LAYER_CFG_GRP1_VAL(3);
>> -val |= MXR_LAYER_CFG_GRP0_VAL(2);
>> -if (ctx->vp_enabled)
>> -val |= MXR

Re: [PATCH v2 0/7] drm/exynos: add pm runtime support

2015-11-23 Thread Inki Dae
2015-11-23 21:25 GMT+09:00 Javier Martinez Canillas :
> Hello,
>
> On 11/21/2015 11:59 AM, Inki Dae wrote:
>> Hi Daniel,
>>
>>
>> 2015-11-21 22:40 GMT+09:00 Daniel Stone :
>>> Hi Inki,
>>>
>>> On 21 November 2015 at 09:38, Inki Dae  wrote:
 2015-11-21 1:44 GMT+09:00 Javier Martinez Canillas 
 :
> On 11/20/2015 08:13 AM, Inki Dae wrote:
>> The boot log says,
>> [5.754493] vdd_ldo9: supplied by vdd_2v
>> [5.765510] of_graph_get_next_endpoint(): no port node found in 
>> /dp-controller@145B
>>
>
> This message is a red herring for the reported issue, the message is also
> present when the machine boots and the display is brought correctly.
>
>> Seems this error is because exynos5800-peach-pit.dts file doesn't have 
>> 'ports' node in dp node.
>>
>> Below is dp node description of exynos5420-peach-pit.dts file.
>> &dp {
>>   status = "okay";
>>   pinctrl-names = "default";
>>   pinctrl-0 = <&dp_hpd_gpio>;
>>   samsung,color-space = <0>;
>>   samsung,dynamic-range = <0>;
>>   samsung,ycbcr-coeff = <0>;
>>   samsung,color-depth = <1>;
>>   samsung,link-rate = <0x06>;
>>   samsung,lane-count = <2>;
>>   samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
>>
>>   ports {
>>   port@0 {
>>   dp_out: endpoint {
>>   remote-endpoint = <&bridge_in>;
>>   };
>>   };
>>   };
>> };
>>
>> And below is for exynos5800-peash-pit.dts,
>> &dp {
>>   status = "okay";
>>   pinctrl-names = "default";
>>   pinctrl-0 = <&dp_hpd_gpio>;
>>   samsung,color-space = <0>;
>>   samsung,dynamic-range = <0>;
>>   samsung,ycbcr-coeff = <0>;
>>   samsung,color-depth = <1>;
>>   samsung,link-rate = <0x0a>;
>>   samsung,lane-count = <2>;
>>   samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
>>   panel = <&panel>;
>> };
>>
>
> The difference is because the Exynos5420 Peach Pit Display Port is not
> attached directly to the display panel, there is an eDP/LVDS bridge chip
> in the middle (PS8622) while the Exynos5800 Peach Pi doesn't have that.
>
> The Exynos DP driver lookups for either a panel phandle or an OF graph
> endpoint that points to a bridge chip and the bridge enpoint has a port
> that points to the panel.
>
> So the DT is correct but of_graph_get_next_endpoint() always prints an

 Then, the DT is really incorrect. As you mentioned, if the Exynos5800 
 Peach PI
 board doesn't use eDP, then the dp node __should be removed__ from
 exynos5800-peach-pit.dts.

 From a common-sense standpoint, there is no any reason to build
 and probe dp driver if the board doesn't use dp hardware.
>>>
>>> I agree with what you say, but unfortunately you've slightly misread
>>> what Javier has said. :) exynos5420-peach-pit has an LVDS panel, with
>>> the eDP -> LVDS bridge in between (ps8622). exynos5800-peach-pi (from
>>> which I am writing this) has an eDP panel directly connected. The DT
>
> Thanks a lot Daniel for clarifying my comments to Inki :)
>
>>> describes both the eDP connector from FIMD and the eDP panel, except
>>> that there is no connection between the DT nodes.
>
> There *is* a connection between the FIMD eDP connector and the eDP panel
> nodes but these are connected using a phandle while the connection for
> the FIMD eDP connector and the eDP/LVDS bridge is using the OF graph DT
> bindings (Documentation/devicetree/bindings/graph.txt).
>
> And also the connection between the eDP/LVDS bridge and the LVDS panel
> is using an OF graph node, so what I meant is that it would be much more
> consistent if both the eDP -> panel and eDP -> eDP/LVDS bridge -> panel
> connections used the OF graph DT bindings.
>
>>
>> Right. I misread what Javier said. :)
>>
>>>
> error if the port so OF graph endpoints it seems can't be optional as
> used in this driver. Maybe that message should be change to debug then?
>
> Another option is to extend the DP driver DT binding to be more generic
> supporting having a port to a panel besides a bridge, so we could have
> something like this for Exynos5800 Peach and be consistent in both cases:

 It's really not good. This would make it more complex. The best
 solution is just to
 remove the dt node from the device tree file.
>>>
>>> Given the above, not really. Javier's patch seems correct to me - as
>>> you can see, there is a panel node, and that is the panel that's
>>> really connected.
>>
>> Indeed. Javier's patch will correct it.
>>
>
> Just to be clear, my patch is not correct since the Exynos DP driver and
> its DT binding does not support to connect an FIMD eDP connector to an
> eDP panel directly using OF grap

[GIT PULL] Immutable branch between MFD, Regulator and RTC

2015-11-23 Thread Lee Jones
Better $SUBJECT for ya'll -- so it doesn't get lost in the noise.

> Mark, Alexandre,
> 
> Enjoy!
> 
> The following changes since commit 1ec218373b8ebda821aec00bb156a9c94fad9cd4:
> 
>   Linux 4.4-rc2 (2015-11-22 16:45:59 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git 
> tags/ib-mfd-regulator-rtc-v4.5
> 
> for you to fetch changes up to a65e5efa7c5faa8c320fe56cc351d47fcd006749:
> 
>   rtc: s5m.c: Add support for S2MPS15 RTC (2015-11-23 10:34:38 +)
> 
> 
> Immutable branch between MFD, Regulator and RTC for the v4.5 merge window
> 
> 
> Alim Akhtar (1):
>   rtc: s5m.c: Add support for S2MPS15 RTC
> 
> Thomas Abraham (2):
>   mfd: sec: Add support for S2MPS15 PMIC
>   regulator: s2mps11: Add support for S2MPS15 regulators
> 
>  drivers/mfd/sec-core.c  |  31 +
>  drivers/mfd/sec-irq.c   |   8 ++
>  drivers/regulator/Kconfig   |   4 +-
>  drivers/regulator/s2mps11.c | 135 +-
>  drivers/rtc/rtc-s5m.c   |  37 -
>  include/linux/mfd/samsung/core.h|   1 +
>  include/linux/mfd/samsung/rtc.h |   2 +
>  include/linux/mfd/samsung/s2mps15.h | 158 ++
>  8 files changed, 369 insertions(+), 7 deletions(-)
>  create mode 100644 include/linux/mfd/samsung/s2mps15.h
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RESEND PATCH v5 0/3] mfd: sec: add S2MPS15 PMIC support

2015-11-23 Thread Lee Jones
Mark, Alexandre,

Enjoy!

The following changes since commit 1ec218373b8ebda821aec00bb156a9c94fad9cd4:

  Linux 4.4-rc2 (2015-11-22 16:45:59 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git 
tags/ib-mfd-regulator-rtc-v4.5

for you to fetch changes up to a65e5efa7c5faa8c320fe56cc351d47fcd006749:

  rtc: s5m.c: Add support for S2MPS15 RTC (2015-11-23 10:34:38 +)


Immutable branch between MFD, Regulator and RTC for the v4.5 merge window


Alim Akhtar (1):
  rtc: s5m.c: Add support for S2MPS15 RTC

Thomas Abraham (2):
  mfd: sec: Add support for S2MPS15 PMIC
  regulator: s2mps11: Add support for S2MPS15 regulators

 drivers/mfd/sec-core.c  |  31 +
 drivers/mfd/sec-irq.c   |   8 ++
 drivers/regulator/Kconfig   |   4 +-
 drivers/regulator/s2mps11.c | 135 +-
 drivers/rtc/rtc-s5m.c   |  37 -
 include/linux/mfd/samsung/core.h|   1 +
 include/linux/mfd/samsung/rtc.h |   2 +
 include/linux/mfd/samsung/s2mps15.h | 158 ++
 8 files changed, 369 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/mfd/samsung/s2mps15.h

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] thermal: exynos: use of_property_read_u8()

2015-11-23 Thread Saurabh Sengar
use of_property_read_u8() for u8 variables,
also changed the return type to void as this function return type
is nowhere used.

Signed-off-by: Saurabh Sengar 
---
 drivers/thermal/samsung/exynos_tmu.c | 30 +-
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index fa61eff..b692c95 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1128,20 +1128,16 @@ static int exynos_of_get_soc_type(struct device_node 
*np)
return -EINVAL;
 }
 
-static int exynos_of_sensor_conf(struct device_node *np,
+static void exynos_of_sensor_conf(struct device_node *np,
 struct exynos_tmu_platform_data *pdata)
 {
-   u32 value;
-   int ret;
-
of_node_get(np);
 
-   ret = of_property_read_u32(np, "samsung,tmu_gain", &value);
-   pdata->gain = (u8)value;
-   of_property_read_u32(np, "samsung,tmu_reference_voltage", &value);
-   pdata->reference_voltage = (u8)value;
-   of_property_read_u32(np, "samsung,tmu_noise_cancel_mode", &value);
-   pdata->noise_cancel_mode = (u8)value;
+   of_property_read_u8(np, "samsung,tmu_gain", &pdata->gain);
+   of_property_read_u8(np, "samsung,tmu_reference_voltage",
+   &pdata->reference_voltage);
+   of_property_read_u8(np, "samsung,tmu_noise_cancel_mode",
+   &pdata->noise_cancel_mode);
 
of_property_read_u32(np, "samsung,tmu_efuse_value",
 &pdata->efuse_value);
@@ -1150,18 +1146,18 @@ static int exynos_of_sensor_conf(struct device_node *np,
of_property_read_u32(np, "samsung,tmu_max_efuse_value",
 &pdata->max_efuse_value);
 
-   of_property_read_u32(np, "samsung,tmu_first_point_trim", &value);
-   pdata->first_point_trim = (u8)value;
-   of_property_read_u32(np, "samsung,tmu_second_point_trim", &value);
-   pdata->second_point_trim = (u8)value;
-   of_property_read_u32(np, "samsung,tmu_default_temp_offset", &value);
-   pdata->default_temp_offset = (u8)value;
+   of_property_read_u8(np, "samsung,tmu_first_point_trim",
+   &pdata->first_point_trim);
+   of_property_read_u8(np, "samsung,tmu_second_point_trim",
+   &pdata->second_point_trim);
+   of_property_read_u8(np, "samsung,tmu_default_temp_offset",
+   &pdata->default_temp_offset);
 
of_property_read_u32(np, "samsung,tmu_cal_type", &pdata->cal_type);
of_property_read_u32(np, "samsung,tmu_cal_mode", &pdata->cal_mode);
 
of_node_put(np);
-   return 0;
+   return;
 }
 
 static int exynos_map_dt_data(struct platform_device *pdev)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: dts: exynos5422-odroid*: remove fimd node

2015-11-23 Thread Javier Martinez Canillas
Hello Krzysztof,

On 11/10/2015 09:43 PM, Krzysztof Kozlowski wrote:

[snip]

> 
> 
> BTW, do you know why we don't have EXYNOS_IOMMU enabled in defconfig?
> Any reasons against?
>

It was explicitly disabled by commit 6562f3bd396a ("ARM: exynos_defconfig:
Disable IOMMU support") because Exynos IOMMU support was broken and caused
a BUG on boot, the discussion of the patch is [0].

But I just tested booting a v4.4-rc2 kernel on an Exynos5800 Peach Pi with
Exynos IOMMU enabled and the machine boots, display is working and
/sys/kernel/iommu_grups/*/devices shows that the devices were correctly
attached to an IOMMU group so things seems to have been sorted out now.

So it seems that EXYNOS_IOMMU could be enabled again. It would be good to
give such a patch a spin at kernelci before posting IMHO though just to be
sure there are no issues remaining.

> Best regards,
> Krzysztof
> 
> 

[0]: https://lkml.org/lkml/2015/2/17/163

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 0/7] drm/exynos: add pm runtime support

2015-11-23 Thread Javier Martinez Canillas
Hello,

On 11/21/2015 11:59 AM, Inki Dae wrote:
> Hi Daniel,
> 
> 
> 2015-11-21 22:40 GMT+09:00 Daniel Stone :
>> Hi Inki,
>>
>> On 21 November 2015 at 09:38, Inki Dae  wrote:
>>> 2015-11-21 1:44 GMT+09:00 Javier Martinez Canillas :
 On 11/20/2015 08:13 AM, Inki Dae wrote:
> The boot log says,
> [5.754493] vdd_ldo9: supplied by vdd_2v
> [5.765510] of_graph_get_next_endpoint(): no port node found in 
> /dp-controller@145B
>

 This message is a red herring for the reported issue, the message is also
 present when the machine boots and the display is brought correctly.

> Seems this error is because exynos5800-peach-pit.dts file doesn't have 
> 'ports' node in dp node.
>
> Below is dp node description of exynos5420-peach-pit.dts file.
> &dp {
>   status = "okay";
>   pinctrl-names = "default";
>   pinctrl-0 = <&dp_hpd_gpio>;
>   samsung,color-space = <0>;
>   samsung,dynamic-range = <0>;
>   samsung,ycbcr-coeff = <0>;
>   samsung,color-depth = <1>;
>   samsung,link-rate = <0x06>;
>   samsung,lane-count = <2>;
>   samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
>
>   ports {
>   port@0 {
>   dp_out: endpoint {
>   remote-endpoint = <&bridge_in>;
>   };
>   };
>   };
> };
>
> And below is for exynos5800-peash-pit.dts,
> &dp {
>   status = "okay";
>   pinctrl-names = "default";
>   pinctrl-0 = <&dp_hpd_gpio>;
>   samsung,color-space = <0>;
>   samsung,dynamic-range = <0>;
>   samsung,ycbcr-coeff = <0>;
>   samsung,color-depth = <1>;
>   samsung,link-rate = <0x0a>;
>   samsung,lane-count = <2>;
>   samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
>   panel = <&panel>;
> };
>

 The difference is because the Exynos5420 Peach Pit Display Port is not
 attached directly to the display panel, there is an eDP/LVDS bridge chip
 in the middle (PS8622) while the Exynos5800 Peach Pi doesn't have that.

 The Exynos DP driver lookups for either a panel phandle or an OF graph
 endpoint that points to a bridge chip and the bridge enpoint has a port
 that points to the panel.

 So the DT is correct but of_graph_get_next_endpoint() always prints an
>>>
>>> Then, the DT is really incorrect. As you mentioned, if the Exynos5800 Peach 
>>> PI
>>> board doesn't use eDP, then the dp node __should be removed__ from
>>> exynos5800-peach-pit.dts.
>>>
>>> From a common-sense standpoint, there is no any reason to build
>>> and probe dp driver if the board doesn't use dp hardware.
>>
>> I agree with what you say, but unfortunately you've slightly misread
>> what Javier has said. :) exynos5420-peach-pit has an LVDS panel, with
>> the eDP -> LVDS bridge in between (ps8622). exynos5800-peach-pi (from
>> which I am writing this) has an eDP panel directly connected. The DT

Thanks a lot Daniel for clarifying my comments to Inki :)

>> describes both the eDP connector from FIMD and the eDP panel, except
>> that there is no connection between the DT nodes.

There *is* a connection between the FIMD eDP connector and the eDP panel
nodes but these are connected using a phandle while the connection for
the FIMD eDP connector and the eDP/LVDS bridge is using the OF graph DT
bindings (Documentation/devicetree/bindings/graph.txt).

And also the connection between the eDP/LVDS bridge and the LVDS panel
is using an OF graph node, so what I meant is that it would be much more
consistent if both the eDP -> panel and eDP -> eDP/LVDS bridge -> panel
connections used the OF graph DT bindings.

>
> Right. I misread what Javier said. :)
>
>>
 error if the port so OF graph endpoints it seems can't be optional as
 used in this driver. Maybe that message should be change to debug then?

 Another option is to extend the DP driver DT binding to be more generic
 supporting having a port to a panel besides a bridge, so we could have
 something like this for Exynos5800 Peach and be consistent in both cases:
>>>
>>> It's really not good. This would make it more complex. The best
>>> solution is just to
>>> remove the dt node from the device tree file.
>>
>> Given the above, not really. Javier's patch seems correct to me - as
>> you can see, there is a panel node, and that is the panel that's
>> really connected.
> 
> Indeed. Javier's patch will correct it.
>

Just to be clear, my patch is not correct since the Exynos DP driver and
its DT binding does not support to connect an FIMD eDP connector to an
eDP panel directly using OF graph ports / endpoints (only a phandle). But
is an example of how the DT will look like if we extend to support that.

IIRC at the beginning only eDP -> panel was supported and the phandle
was used a

Re: PING: [PATCH v9 0/4] Exynos SROMc configuration and Ethernet support for SMDK5410

2015-11-23 Thread Krzysztof Kozlowski
2015-11-23 17:35 GMT+09:00 Pavel Fedin :
>  Hello! So what's up? I see SROM driver suddenly removed from everywhere and 
> no single word about it. What is the status?

What do you mean by everywhere? That sounds a little bit drastic... I
did not apply it before at the first place... so if you have in mind
only Kukjin's tree (and linux-next as a result) then indeed - Pankaj's
patches were not accepted during 4.3 merge window, remained in
Kukjin's tree in a separate branch and finally were not included in
current 'for-next' after rebasing over 4.4-rc1 (because they stayed in
separate branch).

As for the current status I reviewed all the patches but, since
Pankaj's work is present in Kukjin's branch, I won't duplicate the
work.

The question is then to Kukjin - what is your plan? Two more patchsets
are depending on it:
1. Pavel's extension of SROM,
2. Pankaj's movement of PM to drivers/soc.

What do you need to apply them?

Best regards,
Krzysztof

>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>> -Original Message-
>> From: linux-samsung-soc-ow...@vger.kernel.org 
>> [mailto:linux-samsung-soc-ow...@vger.kernel.org]
>> On Behalf Of Pavel Fedin
>> Sent: Monday, November 16, 2015 10:43 AM
>> To: devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
>> linux-samsung-
>> s...@vger.kernel.org; linux-ker...@vger.kernel.org
>> Cc: Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar Gala; Kukjin 
>> Kim; Krzysztof
>> Kozlowski
>> Subject: [PATCH v9 0/4] Exynos SROMc configuration and Ethernet support for 
>> SMDK5410
>>
>> This patch extends Exynos SROM controller driver with ability to configure
>> controller outputs and enables SMSC9115 Ethernet chip on SMDK5410 board,
>> which is connected via SROMc bank #3.
>>
>> With this patchset, support for the whole existing SMDK range can be added.
>> Actually, only bank number is different.
>>
>> This patchset also depends on Exynos 5410 pinctrl support, introduced by
>> patches 0003 and 0004 from this set:
>> [PATCH v4 0/5] ARM: EXYNOS: ODROID-XU DT and LEDs
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/330862.html
>>
>> Pinctrl support is necessary in order to correctly configure
>> multifunctional pins of the Exynos chip.
>>
>> v8 => v9:
>> - Fixed node suffix in ethernet chip DT node (@3,0 instead of @3)
>>
>> v7 => v8:
>> - More fixes to documentation
>>
>> v6 => v7:
>> - Fixed stupid error in Tacc description in the documentation
>>
>> v5 => v6:
>> - Even more improvements to the documentation, fixed some errors and typos.
>> - Separated adding bus ranges from generic SROMc support
>> - Some stuff renamed for even better code readability
>> - Stylistic cleanups in the DTS (everything in alphabetic order, use named
>>   constant name for interrupt config byte)
>>
>> v4 => v5:
>> - Some cosmetic changes in the dtsi ("compatible" goes first)
>> - Use correctly specified ranges for the SROMc node
>> - Reuse existing properties where possible ("reg" for bank# and
>>   "reg-io-width" for data width)
>> - Separated page-mode property from timings array
>> - More improvements to the documentation
>>
>> v3 => v4:
>> - Devices are now added as subnodes, with additional properties. This allows
>>   to cleary specify dependency. If configuration fails, error will be 
>> reported
>>   and child devices will not be probed.
>> - These additional properties now have "samsung,srom-XXX" format
>> - Fixed code style, now better understood.
>>
>> v2 => v3:
>> - Fixed up SROMc region size in the device tree
>> - Reordered patches, documentation goes first now
>>
>> v1 => v2:
>> - Fixed some typos and bad labels in device tree
>> - Improved documentation
>>
>> Pavel Fedin (4):
>>   Documentation: dt-bindings: Describe SROMc configuration
>>   ARM: dts: Add SROMc to Exynos 5410
>>   drivers: exynos-srom: Add support for bank configuration
>>   ARM: dts: Add Ethernet chip to SMDK5410
>>
>>  .../bindings/arm/samsung/exynos-srom.txt   | 73 
>> +-
>>  arch/arm/boot/dts/exynos5410-smdk5410.dts  | 41 
>>  arch/arm/boot/dts/exynos5410.dtsi  | 11 
>>  arch/arm/mach-exynos/Kconfig   |  2 +-
>>  drivers/soc/samsung/Kconfig|  2 +-
>>  drivers/soc/samsung/exynos-srom.c  | 61 +-
>>  6 files changed, 184 insertions(+), 6 deletions(-)
>>
>> --
>> 2.4.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 
>> in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
t

Re: [PATCH] pwm: Avoid double mutex lock on pwm_enable

2015-11-23 Thread Thierry Reding
On Sun, Nov 22, 2015 at 09:13:17AM +0900, Krzysztof Kozlowski wrote:
> 2015-11-22 3:14 GMT+09:00 Anand Moon :
> > Hi Krzysztof,
> >
> > On 21 November 2015 at 18:37, Krzysztof Kozlowski
> >  wrote:
> >> 2015-11-21 21:11 GMT+09:00 Anand Moon :
> >>> hi Krzysztof,
> >>>
> >>> On 21 November 2015 at 15:22, Krzysztof Kozlowski
> >>>  wrote:
>  2015-11-21 18:40 GMT+09:00 Anand Moon :
> > hi Krzysztof,
> >
> > On 21 November 2015 at 09:56, Krzysztof Kozlowski 
> > 
> > wrote:
> >>
> >> W dniu 21.11.2015 o 01:59, Anand Moon pisze:
> >> > Commit d1cd21427747f15920cd726f5f67a07880e7dee4
> >> > (pwm: Set enable state properly on failed call to enable)
> >> > introduce double lock of mutex on pwm_enable
> >> >
> >> > Changes fix the following debug lock warning.
> >> >
> >> > [2.701720] WARNING: CPU: 3 PID: 0 at kernel/locking/mutex.c:526
> >> > __mutex_lock_slowpath+0x3bc/0x404()
> >> > [2.701731] DEBUG_LOCKS_WARN_ON(in_interrupt())
> >>
> >> Hi Anand!
> >>
> >> Yeah, I am hitting this as well. Annoying. However your description is
> >> inaccurate. Double lock of mutex does not cause warnings of sleeping or
> >> locking in atomic context (if you build with debug sleep atomic you 
> >> will
> >> see different warning). More over you are partially reverting the 
> >> commit
> >> d1cd21427747 ("pwm: Set enable state properly on failed call to 
> >> enable")
> >> without proper explanation:
> >> 1. why this locking is inappropriate;
> >> 2. why it is safe to remove the mutex_lock().
> >>
> >> Please find the real problem, fix the real problem and throughly 
> >> explain
> >> the real issue.
> >>
> >> I was suspecting some weird changes in timers. For !irqsafe timer I
> >> think it is safe to use mutexes... but apparently not. Maybe a work
> >> should be scheduled from function called by timer?
> >>
> >> Warning with debug atomic sleep:
> >> [   16.047517] BUG: sleeping function called from invalid context at
> >> ../kernel/locking/mutex.c:617
> >> [   16.054866] in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: 
> >> swapper/0
> >> [   16.061402] INFO: lockdep is turned off.
> >> [   16.065281] Preemption disabled at:[<  (null)>]   (null)
> >> [   16.070524]
> >> [   16.072002] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> >> 4.4.0-rc1-00040-g28c429565d4f #290
> >> [   16.080150] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> >> [   16.086215] [] (unwind_backtrace) from []
> >> (show_stack+0x10/0x14)
> >> [   16.093938] [] (show_stack) from []
> >> (dump_stack+0x70/0xbc)
> >> [   16.101122] [] (dump_stack) from []
> >> (mutex_lock_nested+0x24/0x474)
> >> [   16.109009] [] (mutex_lock_nested) from []
> >> (pwm_enable+0x20/0x7c)
> >> [   16.116799] [] (pwm_enable) from []
> >> (led_heartbeat_function+0xdc/0x140)
> >> [   16.125119] [] (led_heartbeat_function) from []
> >> (call_timer_fn+0x6c/0xf4)
> >> [   16.133606] [] (call_timer_fn) from []
> >> (run_timer_softirq+0x1a8/0x230)
> >> [   16.141846] [] (run_timer_softirq) from []
> >> (__do_softirq+0x134/0x2c0)
> >> [   16.149982] [] (__do_softirq) from []
> >> (irq_exit+0xd0/0x114)
> >> [   16.157255] [] (irq_exit) from []
> >> (__handle_domain_irq+0x70/0xe4)
> >> [   16.165056] [] (__handle_domain_irq) from []
> >> (gic_handle_irq+0x4c/0x94)
> >> [   16.173376] [] (gic_handle_irq) from []
> >> (__irq_svc+0x58/0x98)
> >> [   16.180817] Exception stack(0xc08cdf58 to 0xc08cdfa0)
> >> [   16.185823] df40:
> >>c0010dcc 
> >> [   16.193997] df60: c08cdfa8  c05f57c4  c08ca520
> >> c08ce4bc c08c7364 c08ce4b4
> >> [   16.202141] df80: c09121ca  0001 c08cdfa8 c0010dcc
> >> c0010dd0 600f0013 
> >> [   16.210291] [] (__irq_svc) from []
> >> (arch_cpu_idle+0x20/0x3c)
> >> [   16.217661] [] (arch_cpu_idle) from []
> >> (cpu_startup_entry+0x17c/0x26c)
> >> [   16.225899] [] (cpu_startup_entry) from []
> >> (start_kernel+0x368/0x3d0)
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >>
> >> > [2.701737] Modules linked in:
> >> > [2.701748] CPU: 3 PID: 0 Comm: swapper/3 Not tainted 
> >> > 4.4.0-rc1-xu4f
> >> > #24
> >> > [2.701753] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> >> > [2.701787] [] (unwind_backtrace) from []
> >> > (show_stack+0x10/0x14)
> >> > [2.701808] [] (show_stack) from []
> >> > (dump_stack+0x84/0xc4)
> >> > [2.701824] [] (dump_stack) from []
> >> > (warn_slowpath_common+0x80/0xb0)
> >> > [2.701836] [] (warn_slowpath_common) from []
> >> > (warn_slowpath_fmt+0x30/0x40)
> >> > [2.701849] [] (warn_slowpath_fmt) from []
> >> > (__mutex_lock_slow

PING: [PATCH v9 0/4] Exynos SROMc configuration and Ethernet support for SMDK5410

2015-11-23 Thread Pavel Fedin
 Hello! So what's up? I see SROM driver suddenly removed from everywhere and no 
single word about it. What is the status?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -Original Message-
> From: linux-samsung-soc-ow...@vger.kernel.org 
> [mailto:linux-samsung-soc-ow...@vger.kernel.org]
> On Behalf Of Pavel Fedin
> Sent: Monday, November 16, 2015 10:43 AM
> To: devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
> linux-samsung-
> s...@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar Gala; Kukjin 
> Kim; Krzysztof
> Kozlowski
> Subject: [PATCH v9 0/4] Exynos SROMc configuration and Ethernet support for 
> SMDK5410
> 
> This patch extends Exynos SROM controller driver with ability to configure
> controller outputs and enables SMSC9115 Ethernet chip on SMDK5410 board,
> which is connected via SROMc bank #3.
> 
> With this patchset, support for the whole existing SMDK range can be added.
> Actually, only bank number is different.
> 
> This patchset also depends on Exynos 5410 pinctrl support, introduced by
> patches 0003 and 0004 from this set:
> [PATCH v4 0/5] ARM: EXYNOS: ODROID-XU DT and LEDs
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/330862.html
> 
> Pinctrl support is necessary in order to correctly configure
> multifunctional pins of the Exynos chip.
> 
> v8 => v9:
> - Fixed node suffix in ethernet chip DT node (@3,0 instead of @3)
> 
> v7 => v8:
> - More fixes to documentation
> 
> v6 => v7:
> - Fixed stupid error in Tacc description in the documentation
> 
> v5 => v6:
> - Even more improvements to the documentation, fixed some errors and typos.
> - Separated adding bus ranges from generic SROMc support
> - Some stuff renamed for even better code readability
> - Stylistic cleanups in the DTS (everything in alphabetic order, use named
>   constant name for interrupt config byte)
> 
> v4 => v5:
> - Some cosmetic changes in the dtsi ("compatible" goes first)
> - Use correctly specified ranges for the SROMc node
> - Reuse existing properties where possible ("reg" for bank# and
>   "reg-io-width" for data width)
> - Separated page-mode property from timings array
> - More improvements to the documentation
> 
> v3 => v4:
> - Devices are now added as subnodes, with additional properties. This allows
>   to cleary specify dependency. If configuration fails, error will be reported
>   and child devices will not be probed.
> - These additional properties now have "samsung,srom-XXX" format
> - Fixed code style, now better understood.
> 
> v2 => v3:
> - Fixed up SROMc region size in the device tree
> - Reordered patches, documentation goes first now
> 
> v1 => v2:
> - Fixed some typos and bad labels in device tree
> - Improved documentation
> 
> Pavel Fedin (4):
>   Documentation: dt-bindings: Describe SROMc configuration
>   ARM: dts: Add SROMc to Exynos 5410
>   drivers: exynos-srom: Add support for bank configuration
>   ARM: dts: Add Ethernet chip to SMDK5410
> 
>  .../bindings/arm/samsung/exynos-srom.txt   | 73 
> +-
>  arch/arm/boot/dts/exynos5410-smdk5410.dts  | 41 
>  arch/arm/boot/dts/exynos5410.dtsi  | 11 
>  arch/arm/mach-exynos/Kconfig   |  2 +-
>  drivers/soc/samsung/Kconfig|  2 +-
>  drivers/soc/samsung/exynos-srom.c  | 61 +-
>  6 files changed, 184 insertions(+), 6 deletions(-)
> 
> --
> 2.4.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/5] drm/exynos: mixer: introduce mixer_layer_blending()

2015-11-23 Thread Inki Dae


2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
> This analyses the current layer configuration (which layers
> are enabled, which have alpha-pixelformat, etc.) and setups
> blending accordingly.
> 
> We currently disable all kinds of blending for the bottom-most
> layer, since configuration of the mixer background is not
> yet exposed.
> Also blending is only enabled when the layer has a pixelformat
> with alpha attached.
> 
> Signed-off-by: Tobias Jakobi 
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 88 
> +++
>  drivers/gpu/drm/exynos/regs-mixer.h   |  1 +
>  2 files changed, 89 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 2c1cea3..ec77aad 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -174,6 +174,21 @@ static const u8 filter_cr_horiz_tap4[] = {
>   70, 59, 48, 37, 27, 19, 11, 5,
>  };
>  
> +static inline bool is_alpha_format(const struct mixer_context* ctx, unsigned 
> int win)
> +{
> + const struct drm_plane_state *state = ctx->planes[win].base.state;
> + const struct drm_framebuffer *fb = state->fb;
> +
> + switch (fb->pixel_format) {
> + case DRM_FORMAT_ARGB:
> + case DRM_FORMAT_ARGB1555:
> + case DRM_FORMAT_ARGB:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>  {
>   return readl(res->vp_regs + reg_id);
> @@ -331,6 +346,79 @@ static void mixer_layer_priority(struct mixer_context 
> *ctx)
>   mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val);
>  }
>  
> +/* Configure blending for bottom-most layer. */
> +static void mixer_bottom_layer(struct mixer_context *ctx,
> + const struct layer_cfg *cfg)
> +{
> + u32 val;
> + struct mixer_resources *res = &ctx->mixer_res;
> +
> + if (cfg->index == 2) {
> + val = 0; /* use defaults for video layer */
> + mixer_reg_write(res, MXR_VIDEO_CFG, val);
> + } else {
> + val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
> +
> + /* Don't blend bottom-most layer onto the mixer background. */
> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
> + val, MXR_GRP_CFG_MISC_MASK);
> + }
> +}
> +
> +static void mixer_general_layer(struct mixer_context *ctx,
> + const struct layer_cfg *cfg)
> +{
> + u32 val;
> + struct mixer_resources *res = &ctx->mixer_res;
> +
> + if (is_alpha_format(ctx, cfg->index)) {
> + val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
> + val |= MXR_GRP_CFG_BLEND_PRE_MUL;
> + val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel 
> alpha */
> +
> + /* The video layer never has an alpha pixelformat. */

Looks like above comment isn't related to graphics layer.

> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
> + val, MXR_GRP_CFG_MISC_MASK);
> + } else {
> + if (cfg->index == 2) {
> + /*
> +  * No blending at the moment since the NV12/NV21 
> pixelformats don't
> +  * have an alpha channel. However the mixer supports a 
> global alpha
> +  * value for a layer. Once this functionality is 
> exposed, we can
> +  * support blending of the video layer through this.
> +  */
> + val = 0;
> + mixer_reg_write(res, MXR_VIDEO_CFG, val);

Above 'if statement' wouldn't be called because cfg->index has always a value 
less than 2
so it should be removed.

> + } else {
> + val = MXR_GRP_CFG_COLOR_KEY_DISABLE;
> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
> + val, MXR_GRP_CFG_MISC_MASK);
> + }
> + }
> +}
> +
> +static void mixer_layer_blending(struct mixer_context *ctx, unsigned int 
> enable_state)
> +{
> + unsigned int i, index;
> + bool bottom_layer = false;
> +
> + for (i = 0; i < ctx->num_layer; ++i) {
> + index = ctx->layer_cfg[i].index;
> +
> + /* Skip layer if it's not enabled. */
> + if (!(enable_state & (1 << index)))
> + continue;
> +
> + /* Bottom layer needs special handling. */
> + if (bottom_layer) {
> + mixer_general_layer(ctx, &ctx->layer_cfg[i]);
> + } else {
> + mixer_bottom_layer(ctx, &ctx->layer_cfg[i]);
> + bottom_layer = true;
> + }

I think above codes could be more cleanned up like below if I understood 
correctly,

if (cfg->index == 2) {

Re: [RESEND PATCH v2] mfd: sec-core: Rename MFD and regulator names differently

2015-11-23 Thread Lee Jones
On Fri, 20 Nov 2015, Alim Akhtar wrote:

> Currently S2MPSXX multifunction device is named as *-pmic,
> and these MFDs also supports regulator as a one of its MFD cell which
> has the same name, because current name is confusing and we want to
> sort it out.
> 
> We did discussed different approaches about how the MFD and it
> cells need to be named here [1].
> Based in the discussion this patch rename MFD regulator name as
> *-regulator instead of current *-pmic.
> 
> This patch also changes the corresponding entries in the regulator driver
> to keep git-bisect happy.
> 
> [1]-> https://lkml.org/lkml/2015/10/28/417
> 
> Suggested-by: Lee Jones 
> Signed-off-by: Alim Akhtar 
> Reviewed-by: Krzysztof Kozlowski 
> Acked-by: Mark Brown 
> Acked-by: Lee Jones 
> ---
> Resending with collected Reviwed-by, Acked-by tags.
> 
>  drivers/mfd/sec-core.c  |8 
>  drivers/regulator/s2mps11.c |8 
>  2 files changed, 8 insertions(+), 8 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index 7c4e7be17f1e..c9802ba9be72 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -68,7 +68,7 @@ static const struct mfd_cell s5m8767_devs[] = {
>  
>  static const struct mfd_cell s2mps11_devs[] = {
>   {
> - .name = "s2mps11-pmic",
> + .name = "s2mps11-regulator",
>   }, {
>   .name = "s2mps14-rtc",
>   }, {
> @@ -78,7 +78,7 @@ static const struct mfd_cell s2mps11_devs[] = {
>  };
>  
>  static const struct mfd_cell s2mps13_devs[] = {
> - { .name = "s2mps13-pmic", },
> + { .name = "s2mps13-regulator", },
>   { .name = "s2mps13-rtc", },
>   {
>   .name = "s2mps13-clk",
> @@ -88,7 +88,7 @@ static const struct mfd_cell s2mps13_devs[] = {
>  
>  static const struct mfd_cell s2mps14_devs[] = {
>   {
> - .name = "s2mps14-pmic",
> + .name = "s2mps14-regulator",
>   }, {
>   .name = "s2mps14-rtc",
>   }, {
> @@ -116,7 +116,7 @@ static const struct mfd_cell s2mpa01_devs[] = {
>  
>  static const struct mfd_cell s2mpu02_devs[] = {
>   {
> - .name = "s2mpu02-pmic",
> + .name = "s2mpu02-regulator",
>   },
>  };
>  
> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
> index b2f3a28e720c..3242ffc0cb25 100644
> --- a/drivers/regulator/s2mps11.c
> +++ b/drivers/regulator/s2mps11.c
> @@ -1199,11 +1199,11 @@ out:
>  }
>  
>  static const struct platform_device_id s2mps11_pmic_id[] = {
> - { "s2mps11-pmic", S2MPS11X},
> - { "s2mps13-pmic", S2MPS13X},
> - { "s2mps14-pmic", S2MPS14X},
> + { "s2mps11-regulator", S2MPS11X},
> + { "s2mps13-regulator", S2MPS13X},
> + { "s2mps14-regulator", S2MPS14X},
>   { "s2mps15-regulator", S2MPS15X},
> - { "s2mpu02-pmic", S2MPU02},
> + { "s2mpu02-regulator", S2MPU02},
>   { },
>  };
>  MODULE_DEVICE_TABLE(platform, s2mps11_pmic_id);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html