Re: [RFC,libdrm 1/3] tegra: Add stream library

2012-12-27 Thread Mark Zhang
On 12/28/2012 03:45 PM, Arto Merilainen wrote:
> On 12/28/2012 08:47 AM, Mark Zhang wrote:
>>> +int tegra_fence_is_valid(const struct tegra_fence *fence)
>>> +{
>>> +int valid = fence ? 1 : 0;
>>> +valid = valid && fence->id != (uint32_t) -1;
>>> +valid = valid && fence->id < 32;
>>
>> Hardcode here. Assume always has 32 syncpts.
>> Change to a micro wrapped with tegra version ifdef will be better.
>>
> 
> You are correct. I will fix this.
> 
>>> +return valid;
>>> +}
>>> +
>> [...]
>>> +
>>> +/* Add fences */
>>> +if (num_fences) {
>>> +
>>> +tegra_stream_push(stream,
>>> +nvhost_opcode_setclass(NV_HOST1X_CLASS_ID,
>>> +host1x_uclass_wait_syncpt_r(), num_fences));
>>> +
>>> +for (; num_fences; num_fences--, fence++) {
>>> +assert(tegra_fence_is_valid(fence));
>>
>> This is useless. We already add "1 + num_fences" to num_words above. So
>> move this "assert" before adding "1 + num_fences" to num_words makes
>> sense.
>>
> 
> The assertion checks the validity of a single fence - not if there is
> room in the command buffer.
> 
> The goal is to prevent having invalid fences in the command stream. If
> this check were not here it would be possible to initialise a fence with
> tegra_fence_clear() and put that fence into the stream.

My idea is, if one fence is invalid, then we should not count this in
"num_words". In current code, if one fence is invalid, then this fence
will not be pushed into the command stream, and the "num_words" shows a
wrong command buffer size.

So I think we should:
- validate the fences, remove the invalid fence
- update num_words
- then you don't need to check fence here - I mean, before push a host1x
syncpt wait command into the active buffer of stream.

> 
>>> +
>>> +tegra_stream_push(stream,
>>> nvhost_class_host_wait_syncpt(fence->id,
>>> +fence->value));
>>> +}
>>> +}
>>> +
>>> +if (class_id)
>>> +tegra_stream_push(stream, nvhost_opcode_setclass(class_id,
>>> 0, 0));
>>> +
>>> +return 0;
>>> +}
>>> +
>> [...]
>>> +
>>> +#endif /* TEGRA_DRMIF_H */
>>>
> 
> - Arto
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC,libdrm 1/3] tegra: Add stream library

2012-12-27 Thread Arto Merilainen

On 12/28/2012 08:47 AM, Mark Zhang wrote:

+int tegra_fence_is_valid(const struct tegra_fence *fence)
+{
+int valid = fence ? 1 : 0;
+valid = valid && fence->id != (uint32_t) -1;
+valid = valid && fence->id < 32;


Hardcode here. Assume always has 32 syncpts.
Change to a micro wrapped with tegra version ifdef will be better.



You are correct. I will fix this.


+return valid;
+}
+

[...]

+
+/* Add fences */
+if (num_fences) {
+
+tegra_stream_push(stream,
+nvhost_opcode_setclass(NV_HOST1X_CLASS_ID,
+host1x_uclass_wait_syncpt_r(), num_fences));
+
+for (; num_fences; num_fences--, fence++) {
+assert(tegra_fence_is_valid(fence));


This is useless. We already add "1 + num_fences" to num_words above. So
move this "assert" before adding "1 + num_fences" to num_words makes sense.



The assertion checks the validity of a single fence - not if there is 
room in the command buffer.


The goal is to prevent having invalid fences in the command stream. If 
this check were not here it would be possible to initialise a fence with 
tegra_fence_clear() and put that fence into the stream.



+
+tegra_stream_push(stream, nvhost_class_host_wait_syncpt(fence->id,
+fence->value));
+}
+}
+
+if (class_id)
+tegra_stream_push(stream, nvhost_opcode_setclass(class_id, 0, 0));
+
+return 0;
+}
+

[...]

+
+#endif /* TEGRA_DRMIF_H */



- Arto
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/exynos: Add device tree based discovery support for Exynos G2D

2012-12-27 Thread Ajay Kumar
This patch adds device tree match table for Exynos G2D controller.

Signed-off-by: Ajay Kumar 
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 6ffa076..aa3d2e4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -1240,6 +1241,14 @@ static int g2d_resume(struct device *dev)

 static SIMPLE_DEV_PM_OPS(g2d_pm_ops, g2d_suspend, g2d_resume);

+#ifdef CONFIG_OF
+static const struct of_device_id exynos_g2d_match[] = {
+   { .compatible = "samsung,exynos-g2d-41" },
+   {},
+};
+MODULE_DEVICE_TABLE(of, exynos_g2d_match);
+#endif
+
 struct platform_driver g2d_driver = {
.probe  = g2d_probe,
.remove = __devexit_p(g2d_remove),
@@ -1247,5 +1256,6 @@ struct platform_driver g2d_driver = {
.name   = "s5p-g2d",
.owner  = THIS_MODULE,
.pm = &g2d_pm_ops,
+   .of_match_table = of_match_ptr(exynos_g2d_match),
},
 };
-- 
1.8.0



Re: [RFC,libdrm 2/3] tegra: Add 2d library

2012-12-27 Thread Mark Zhang
I just skimmed the code of libdrm while I'm trying to understand the
host1x driver. So below is what I found.

Mark
On 12/13/2012 10:01 PM, Arto Meriläinen wrote:
> From: Francis Hart 
> 
> This patch introduces a simple 2d library on top of stream library.
> 
> Signed-off-by: Francis Hart 
[...]
> +void
> +g2fill_dispatch(const struct tegra_2d_g2fill *hw,
> +struct tegra_stream *stream)
> +{
> +ASSERT(hw);
> +ASSERT(hw->is_valid);
> +ASSERT(stream);
> +
> +tegra_stream_push_setclass(stream, NV_GRAPHICS_2D_CLASS_ID);

I think at the end of the "tegra_stream_begin", we already pushed a
setclass opcode into the stream's active buffer.
So why we need another one here?

> +
> +tegra_stream_push_words(stream,
> +&hw->block,
> +sizeof(hw->block) / sizeof(uint32_t),
> +1,
> +tegra_reloc(&hw->block.dstba,
> +hw->dst_handle,
> +hw->dst_offset));
> +}
> +
[...]
> +#endif // TEGRA_2D_H
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC,libdrm 1/3] tegra: Add stream library

2012-12-27 Thread Mark Zhang
I just skimmed the code of libdrm while I'm trying to understand the
host1x driver. So below is what I found.

Mark
On 12/13/2012 10:01 PM, Arto Meriläinen wrote:
> From: Arto Merilainen 
> 
> This patch introduces tegra stream library. The library is used for
> buffer management, command stream construction and work
> synchronization.
> 
[...]
> +
> +/*
> + * tegra_fence_is_valid(fence)
> + *
> + * Check validity of a fence. We just check that the fence range
> + * is valid w.r.t. host1x hardware.
> + */
> +
> +int tegra_fence_is_valid(const struct tegra_fence *fence)
> +{
> +int valid = fence ? 1 : 0;
> +valid = valid && fence->id != (uint32_t) -1;
> +valid = valid && fence->id < 32;

Hardcode here. Assume always has 32 syncpts.
Change to a micro wrapped with tegra version ifdef will be better.

> +return valid;
> +}
> +
[...]
> +
> +/* Add fences */
> +if (num_fences) {
> +
> +tegra_stream_push(stream,
> +nvhost_opcode_setclass(NV_HOST1X_CLASS_ID,
> +host1x_uclass_wait_syncpt_r(), num_fences));
> +
> +for (; num_fences; num_fences--, fence++) {
> +assert(tegra_fence_is_valid(fence));

This is useless. We already add "1 + num_fences" to num_words above. So
move this "assert" before adding "1 + num_fences" to num_words makes sense.

> +
> +tegra_stream_push(stream, 
> nvhost_class_host_wait_syncpt(fence->id,
> +fence->value));
> +}
> +}
> +
> +if (class_id)
> +tegra_stream_push(stream, nvhost_opcode_setclass(class_id, 0, 0));
> +
> +return 0;
> +}
> +
[...]
> +
> +#endif /* TEGRA_DRMIF_H */
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] video: drm: exynos: mie bypass enable for fimd

2012-12-27 Thread Inki Dae
2012/12/28 Leela Krishna Amudala :
> Hello Inki Dae,
>
> On Thu, Dec 27, 2012 at 11:47 AM, Inki Dae  wrote:
>>
>> Hi,
>>
>> DISP1BLK_CFG register is related to GScaler, HDCP and MIXER as well. So
>> it's not good that this register is controlled in fimd module. And I think
>> the function to control the register should be placed in SoC common file .
>> In other words, other drivers should be able to control the register through
>> common thing also.
>>
>
> Thanks for reviewing the patch.
> You mean to say that this functionality should be implemented at arch side
> and called by drivers using call back functions ?
>
> If so, then if we moved the driver to full DT version, all the call
> backs will be removed.
> Then how to make a call to this function?
>
> So I thought other drivers (apart from Fimd) also parses the
> appropriate nodes and
> program the register as per the need.
>
> Please correct me if my understanding is wrong.
>

The base address of DISP1BLK_CFG already was iorempped by
iotable_init() at machine init.  So you can control DISP1BLK_CFG
register using only offset. But your patch does ioremap again and also
even not iounmap. This is ugly. Please see
arch/arm/plat-samsung/setup-mipiphy.c how the common register is
controlled and this is a good example. And please abuse dt.

> Best Wishes,
> Leela Krishna Amudala.
>
>
>> Thanks,
>> Inki Dae
>>
>> 2012/12/26 Leela Krishna Amudala 
>>>
>>> Bypasses the mie for fimd by parsing the register and bit offset values
>>> from "mie-bypass" node, if "mie-bypass" node is present in the dts file.
>>>
>>> Signed-off-by: Leela Krishna Amudala 
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 55
>>> 
>>>  1 file changed, 55 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index bf0d9ba..f8ad259 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -118,6 +118,12 @@ static const struct of_device_id
>>> fimd_driver_dt_match[] = {
>>>  MODULE_DEVICE_TABLE(of, fimd_driver_dt_match);
>>>  #endif
>>>
>>> +struct mie_bypass {
>>> +   u32 enable_bypass;
>>> +   void __iomem*bypass_reg;
>>> +   u32 bypass_bit_offset;
>>> +};
>>> +
>>>  static inline struct fimd_driver_data *drm_fimd_get_driver_data(
>>> struct platform_device *pdev)
>>>  {
>>> @@ -133,6 +139,41 @@ static inline struct fimd_driver_data
>>> *drm_fimd_get_driver_data(
>>> platform_get_device_id(pdev)->driver_data;
>>>  }
>>>
>>> +static struct mie_bypass *parse_mie_bypass_for_fimd(struct device *dev,
>>> +   struct device_node
>>> *mie_bypass_node)
>>> +{
>>> +   struct mie_bypass *bypass_data;
>>> +   u32 phy_address;
>>> +
>>> +   bypass_data = devm_kzalloc(dev, sizeof(*bypass_data),
>>> GFP_KERNEL);
>>> +   if (!bypass_data) {
>>> +   dev_err(dev, "memory allocation for bypass data
>>> failed\n");
>>> +   return ERR_PTR(-ENOMEM);
>>> +   }
>>> +   of_property_read_u32(mie_bypass_node,
>>> "samsung,mie-bypass-enable",
>>> +   &bypass_data->enable_bypass);
>>> +   of_property_read_u32(mie_bypass_node, "samsung,disp1blk-cfg-reg",
>>> +   &phy_address);
>>> +   of_property_read_u32(mie_bypass_node,
>>> "samsung,bypass-bit-offset",
>>> +   &bypass_data->bypass_bit_offset);
>>> +
>>> +   bypass_data->bypass_reg = ioremap(phy_address, SZ_4);
>>>
>>> +   if (!bypass_data->bypass_reg) {
>>> +   dev_err(dev, "failed to ioremap phy_address\n");
>>> +   return ERR_PTR(-ENOMEM);
>>> +   }
>>> +   return bypass_data;
>>> +}
>>>
>>> +
>>> +static void mie_bypass_for_fimd(struct mie_bypass *bypass_data)
>>> +{
>>> +   u32 reg;
>>> +
>>> +   reg = __raw_readl(bypass_data->bypass_reg);
>>> +   reg |= (1 << bypass_data->bypass_bit_offset);
>>> +   __raw_writel(reg, bypass_data->bypass_reg);
>>> +}
>>> +
>>>  static bool fimd_display_is_connected(struct device *dev)
>>>  {
>>> DRM_DEBUG_KMS("%s\n", __FILE__);
>>> @@ -906,12 +947,26 @@ static int __devinit fimd_probe(struct
>>> platform_device *pdev)
>>> struct exynos_drm_fimd_pdata *pdata;
>>> struct exynos_drm_panel_info *panel;
>>> struct resource *res;
>>> +   struct device_node *mie_bypass_node;
>>> +   struct mie_bypass *bypass_data = NULL;
>>> int win;
>>> int ret = -EINVAL;
>>>
>>> DRM_DEBUG_KMS("%s\n", __FILE__);
>>>
>>> pdata = pdev->dev.platform_data;
>>> +   if (pdev->dev.of_node) {
>>> +   mie_bypass_node = of_find_node_by_name(pdev->dev.of_node,
>>> +   "mie-bypass");
>>> +   if (mie_bypass_node) {
>>> +   

[PATCH] drm/exynos: fix gem buffer allocation type checking

2012-12-27 Thread Inki Dae
2012/12/27 Prathyush K 

>
>
>
> On Thu, Dec 27, 2012 at 4:27 PM, Inki Dae  wrote:
>
>> This patch fixes gem buffer allocation type checking.
>> EXYNOS_BO_CONTIG has 0 so the checking should be fixed
>> to 'if (!(flags & EXYNOS_BO_NONCONTIG))'
>>
>> Signed-off-by: Inki Dae 
>> Signed-off-by: Kyungmin Park 
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_buf.c |2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> b/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> index 74592d1..911f7fd 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> @@ -38,7 +38,7 @@ static int lowlevel_buffer_allocate(struct drm_device
>> *dev,
>>  * region will be allocated else physically contiguous
>>  * as possible.
>>  */
>> -   if (flags & EXYNOS_BO_CONTIG)
>> +   if (!(flags & EXYNOS_BO_NONCONTIG))
>> dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &buf->dma_attrs);
>>
>>
> Hi Mr. Dae,
>
> If iommu is supported, we would have called arm_iommu_attach_device.
> So dma_alloc_attrs will always call arm_iommu_alloc_attrs.
>
> If iommu is not supported, dma_alloc_attrs will call arm_dma_alloc which
> will
> anyway allocate a contiguous buffer if possible.
>
> With this code, we are forcing the root framebuffer (fbdev) to be
> contiguous since fbdev
> allocation does not pass noncontig flag.
>
>
we could use noncontig flag simply like below,

  if (iommu is supported)
  exynos_drm_gem_create(dev, EXYNOS_BO_NONCONTIG, size);
  else
  exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG, size);



> Why do we need to force the allocation of a contiguous buffer when iommu
> is supported?
>
>
Simply saying, the reason is that we want to pass boot logo drew by boot
loader without iommu to kernel side without memory copy.


> Regards,
> Prathyush
>
>
>> /*
>> --
>> 1.7.4.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121227/9948b578/attachment-0001.html>


Re: [PATCH] video: drm: exynos: mie bypass enable for fimd

2012-12-27 Thread Leela Krishna Amudala
Hello Inki Dae,

On Thu, Dec 27, 2012 at 11:47 AM, Inki Dae  wrote:
>
> Hi,
>
> DISP1BLK_CFG register is related to GScaler, HDCP and MIXER as well. So
> it's not good that this register is controlled in fimd module. And I think
> the function to control the register should be placed in SoC common file .
> In other words, other drivers should be able to control the register through
> common thing also.
>

Thanks for reviewing the patch.
You mean to say that this functionality should be implemented at arch side
and called by drivers using call back functions ?

If so, then if we moved the driver to full DT version, all the call
backs will be removed.
Then how to make a call to this function?

So I thought other drivers (apart from Fimd) also parses the
appropriate nodes and
program the register as per the need.

Please correct me if my understanding is wrong.

Best Wishes,
Leela Krishna Amudala.


> Thanks,
> Inki Dae
>
> 2012/12/26 Leela Krishna Amudala 
>>
>> Bypasses the mie for fimd by parsing the register and bit offset values
>> from "mie-bypass" node, if "mie-bypass" node is present in the dts file.
>>
>> Signed-off-by: Leela Krishna Amudala 
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 55
>> 
>>  1 file changed, 55 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index bf0d9ba..f8ad259 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -118,6 +118,12 @@ static const struct of_device_id
>> fimd_driver_dt_match[] = {
>>  MODULE_DEVICE_TABLE(of, fimd_driver_dt_match);
>>  #endif
>>
>> +struct mie_bypass {
>> +   u32 enable_bypass;
>> +   void __iomem*bypass_reg;
>> +   u32 bypass_bit_offset;
>> +};
>> +
>>  static inline struct fimd_driver_data *drm_fimd_get_driver_data(
>> struct platform_device *pdev)
>>  {
>> @@ -133,6 +139,41 @@ static inline struct fimd_driver_data
>> *drm_fimd_get_driver_data(
>> platform_get_device_id(pdev)->driver_data;
>>  }
>>
>> +static struct mie_bypass *parse_mie_bypass_for_fimd(struct device *dev,
>> +   struct device_node
>> *mie_bypass_node)
>> +{
>> +   struct mie_bypass *bypass_data;
>> +   u32 phy_address;
>> +
>> +   bypass_data = devm_kzalloc(dev, sizeof(*bypass_data),
>> GFP_KERNEL);
>> +   if (!bypass_data) {
>> +   dev_err(dev, "memory allocation for bypass data
>> failed\n");
>> +   return ERR_PTR(-ENOMEM);
>> +   }
>> +   of_property_read_u32(mie_bypass_node,
>> "samsung,mie-bypass-enable",
>> +   &bypass_data->enable_bypass);
>> +   of_property_read_u32(mie_bypass_node, "samsung,disp1blk-cfg-reg",
>> +   &phy_address);
>> +   of_property_read_u32(mie_bypass_node,
>> "samsung,bypass-bit-offset",
>> +   &bypass_data->bypass_bit_offset);
>> +
>> +   bypass_data->bypass_reg = ioremap(phy_address, SZ_4);
>>
>> +   if (!bypass_data->bypass_reg) {
>> +   dev_err(dev, "failed to ioremap phy_address\n");
>> +   return ERR_PTR(-ENOMEM);
>> +   }
>> +   return bypass_data;
>> +}
>>
>> +
>> +static void mie_bypass_for_fimd(struct mie_bypass *bypass_data)
>> +{
>> +   u32 reg;
>> +
>> +   reg = __raw_readl(bypass_data->bypass_reg);
>> +   reg |= (1 << bypass_data->bypass_bit_offset);
>> +   __raw_writel(reg, bypass_data->bypass_reg);
>> +}
>> +
>>  static bool fimd_display_is_connected(struct device *dev)
>>  {
>> DRM_DEBUG_KMS("%s\n", __FILE__);
>> @@ -906,12 +947,26 @@ static int __devinit fimd_probe(struct
>> platform_device *pdev)
>> struct exynos_drm_fimd_pdata *pdata;
>> struct exynos_drm_panel_info *panel;
>> struct resource *res;
>> +   struct device_node *mie_bypass_node;
>> +   struct mie_bypass *bypass_data = NULL;
>> int win;
>> int ret = -EINVAL;
>>
>> DRM_DEBUG_KMS("%s\n", __FILE__);
>>
>> pdata = pdev->dev.platform_data;
>> +   if (pdev->dev.of_node) {
>> +   mie_bypass_node = of_find_node_by_name(pdev->dev.of_node,
>> +   "mie-bypass");
>> +   if (mie_bypass_node) {
>> +   bypass_data =
>> parse_mie_bypass_for_fimd(&pdev->dev,
>> +   mie_bypass_node);
>>
>> +   if (IS_ERR(bypass_data))
>> +   return PTR_ERR(bypass_data);
>> +   if (bypass_data->enable_bypass)
>> +   mie_bypass_for_fimd(bypass_data);
>> +   }
>> +   }
>> if (!pdata) {
>> dev_err(dev, "no platform data specified\n");
>> return -EINVAL;

[PATCH] drm/exynos: Add device tree based discovery support for Exynos G2D

2012-12-27 Thread Ajay Kumar
This patch adds device tree match table for Exynos G2D controller.

Signed-off-by: Ajay Kumar 
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 6ffa076..aa3d2e4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1240,6 +1241,14 @@ static int g2d_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(g2d_pm_ops, g2d_suspend, g2d_resume);
 
+#ifdef CONFIG_OF
+static const struct of_device_id exynos_g2d_match[] = {
+   { .compatible = "samsung,exynos-g2d-41" },
+   {},
+};
+MODULE_DEVICE_TABLE(of, exynos_g2d_match);
+#endif
+
 struct platform_driver g2d_driver = {
.probe  = g2d_probe,
.remove = __devexit_p(g2d_remove),
@@ -1247,5 +1256,6 @@ struct platform_driver g2d_driver = {
.name   = "s5p-g2d",
.owner  = THIS_MODULE,
.pm = &g2d_pm_ops,
+   .of_match_table = of_match_ptr(exynos_g2d_match),
},
 };
-- 
1.8.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 41265] Radeon KMS does not work on thunderbolt media dock

2012-12-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=41265

--- Comment #53 from Gerald Nunn  ---
Does AMD need to make a change to the proprietary fglrx drivers in order for
the kernel patch to work with their drivers and if so has a bug been opened
with them? I'm using Ubuntu 12.10 with kernel 3.6.9 and no problem getting this
to work with the gallium (radeon) drivers, however when I try the fglrx drivers
I get the following messages in kern.log:

Dec 27 13:07:11 gnunn-laptop2 kernel: [   11.925204] <3>[fglrx:hal_read_VBIOS]
*ERROR* Invalid AMD VBIOS!
Dec 27 13:07:11 gnunn-laptop2 kernel: [   11.925207] <3>[fglrx:hal_init_gpu]
*ERROR* Failed to read VBIOS!
Dec 27 13:07:11 gnunn-laptop2 kernel: [   11.925208] <6>[fglrx] device open
failed with code -1

Which sounds like a similar issue to what was described here. While I generally
find the gallium drivers to be more stable and reliable then fglrx,
unfortunately fglrx significantly outperforms the open source drivers when it
comes to gaming.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC v2 0/5] Common Display Framework

2012-12-27 Thread Sascha Hauer
On Thu, Dec 27, 2012 at 10:04:22AM -0600, Rob Clark wrote:
> On Mon, Dec 24, 2012 at 11:27 AM, Laurent Pinchart
>  wrote:
> > On Wednesday 19 December 2012 16:57:56 Jani Nikula wrote:
> >> It just seems to me that, at least from a DRM/KMS perspective, adding
> >> another layer (=CDF) for HDMI or DP (or legacy outputs) would be
> >> overengineering it. They are pretty well standardized, and I don't see 
> >> there
> >> would be a need to write multiple display drivers for them. Each display
> >> controller has one, and can easily handle any chip specific requirements
> >> right there. It's my gut feeling that an additional framework would just 
> >> get
> >> in the way. Perhaps there could be more common HDMI/DP helper style code in
> >> DRM to reduce overlap across KMS drivers, but that's another thing.
> >>
> >> So is the HDMI/DP drivers using CDF a more interesting idea from a non-DRM
> >> perspective? Or, put another way, is it more of an alternative to using 
> >> DRM?
> >> Please enlighten me if there's some real benefit here that I fail to see!
> >
> > As Rob pointed out, you can have external HDMI/DP encoders, and even 
> > internal
> > HDMI/DP encoder IPs can be shared between SoCs and SoC vendors. CDF aims at
> > sharing a single driver between SoCs and boards for a given HDMI/DP encoder.
> 
> just fwiw, drm already has something a bit like this.. the i2c
> encoder-slave.  With support for a couple external i2c encoders which
> could in theory be shared between devices.

The problem with this code is that it only works when the i2c device is
registered by a master driver. Once the i2c device comes from the
devicetree there is no possibility to find it.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


[RFC v2 0/5] Common Display Framework

2012-12-27 Thread Sascha Hauer
On Thu, Dec 27, 2012 at 09:54:55AM -0600, Rob Clark wrote:
> On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
>  wrote:
> > Hi Rob,
> >
> > On Tuesday 18 December 2012 00:21:32 Rob Clark wrote:
> >> On Mon, Dec 17, 2012 at 11:04 PM, Dave Airlie  wrote:
> >> >> Many developers showed interest in the first RFC, and I've had the
> >> >> opportunity to discuss it with most of them. I would like to thank (in
> >> >> no particular order) Tomi Valkeinen for all the time he spend helping me
> >> >> to draft v2, Marcus Lorentzon for his useful input during Linaro Connect
> >> >> Q4 2012, and Linaro for inviting me to Connect and providing a venue to
> >> >> discuss this topic.
> >> >
> >> > So this might be a bit off topic but this whole CDF triggered me
> >> > looking at stuff I generally avoid:
> >> >
> >> > The biggest problem I'm having currently with the whole ARM graphics
> >> > and output world is the proliferation of platform drivers for every
> >> > little thing. The whole ordering of operations with respect to things
> >> > like suspend/resume or dynamic power management is going to be a real
> >> > nightmare if there are dependencies between the drivers. How do you
> >> > enforce ordering of s/r operations between all the various components?
> >>
> >> I tend to think that sub-devices are useful just to have a way to probe hw
> >> which may or may not be there, since on ARM we often don't have any
> >> alternative.. but beyond that, suspend/resume, and other life-cycle 
> >> aspects,
> >> they should really be treated as all one device. Especially to avoid
> >> undefined suspend/resume ordering.
> >
> > I tend to agree, except that I try to reuse the existing PM infrastructure
> > when possible to avoid reinventing the wheel. So far handling suspend/resume
> > ordering related to data busses in early suspend/late resume operations and
> > allowing the Linux PM core to handle control busses using the Linux device
> > tree worked pretty well.
> >
> >> CDF or some sort of mechanism to share panel drivers between drivers is
> >> useful.  Keeping it within drm, is probably a good idea, if nothing else to
> >> simplify re-use of helper fxns (like avi-infoframe stuff, for example) and
> >> avoid dealing with merging changes across multiple trees. Treating them 
> >> more
> >> like shared libraries and less like sub-devices which can be dynamically
> >> loaded/unloaded (ie. they should be not built as separate modules or
> >> suspend/resumed or probed/removed independently of the master driver) is a
> >> really good idea to avoid uncovering nasty synchronization issues later
> >> (remove vs modeset or pageflip) or surprising userspace in bad ways.
> >
> > We've tried that in V4L2 years ago and realized that the approach led to a
> > dead-end, especially when OF/DT got involved. With DT-based device probing,
> > I2C camera sensors started getting probed asynchronously to the main camera
> > device, as they are children of the I2C bus master. We will have similar
> > issues with I2C HDMI transmitters or panels, so we should be prepared for 
> > it.
> 
> What I've done to avoid that so far is that the master device
> registers the drivers for it's output sub-devices before registering
> it's own device.  At least this way I can control that they are probed
> first.  Not the prettiest thing, but avoids even uglier problems.

This implies that the master driver knows all potential subdevices,
something which is not true for SoCs which have external i2c encoders
attached to unrelated i2c controllers.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


[PATCH] drm/exynos: fix gem buffer allocation type checking

2012-12-27 Thread Inki Dae
This patch fixes gem buffer allocation type checking.
EXYNOS_BO_CONTIG has 0 so the checking should be fixed
to 'if (!(flags & EXYNOS_BO_NONCONTIG))'

Signed-off-by: Inki Dae 
Signed-off-by: Kyungmin Park 
---
 drivers/gpu/drm/exynos/exynos_drm_buf.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c 
b/drivers/gpu/drm/exynos/exynos_drm_buf.c
index 74592d1..911f7fd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
@@ -38,7 +38,7 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
 * region will be allocated else physically contiguous
 * as possible.
 */
-   if (flags & EXYNOS_BO_CONTIG)
+   if (!(flags & EXYNOS_BO_NONCONTIG))
dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &buf->dma_attrs);

/*
-- 
1.7.4.1



[PATCH 04/10] drm/exynos: Use devm_clk_get in exynos_drm_fimc.c

2012-12-27 Thread Inki Dae
t;> -   clk_put(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->fimc_clk);
> >> -   clk_put(ctx->wb_clk);
> >> -   clk_put(ctx->wb_b_clk);
> >> return -EINVAL;
> >> }
> >>
> >> -   clk_put(parent_clk);
> >> +   devm_clk_put(dev, parent_clk);
> >
> > ditto.
> >
> >>
> >> clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate);
> >>
> >> /* resource memory */
> >> @@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct
> platform_device *pdev)
> >> ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res);
> >> if (!ctx->regs) {
> >> dev_err(dev, "failed to map registers.\n");
> >> -   ret = -ENXIO;
> >> -   goto err_clk;
> >> +   return -ENXIO;
> >> }
> >>
> >> /* resource irq */
> >> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> >> if (!res) {
> >> dev_err(dev, "failed to request irq resource.\n");
> >> -   ret = -ENOENT;
> >> -   goto err_clk;
> >> +   return -ENOENT;
> >> }
> >>
> >> ctx->irq = res->start;
> >> @@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct
> platform_device *pdev)
> >> IRQF_ONESHOT, "drm_fimc", ctx);
> >> if (ret < 0) {
> >>     dev_err(dev, "failed to request irq.\n");
> >> -   goto err_clk;
> >> +   return ret;
> >> }
> >>
> >> /* context initailization */
> >> @@ -1867,11 +1851,6 @@ err_ippdrv_register:
> >> pm_runtime_disable(dev);
> >>  err_get_irq:
> >> free_irq(ctx->irq, ctx);
> >> -err_clk:
> >> -   clk_put(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->fimc_clk);
> >> -   clk_put(ctx->wb_clk);
> >> -   clk_put(ctx->wb_b_clk);
> >>
> >> return ret;
> >>  }
> >> @@ -1891,11 +1870,6 @@ static int __devexit fimc_remove(struct
> platform_device *pdev)
> >>
> >> free_irq(ctx->irq, ctx);
> >>
> >> -   clk_put(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->fimc_clk);
> >> -   clk_put(ctx->wb_clk);
> >> -   clk_put(ctx->wb_b_clk);
> >> -
> >> return 0;
> >>  }
> >>
> >> --
> >> 1.7.4.1
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
>
> --
> With warm regards,
> Sachin
>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121227/8b567a0e/attachment-0001.html>


[Bug 49981] [drm:radeon_cs_ioctl] *ERROR* Failed to parse relocation -12!

2012-12-27 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=49981





--- Comment #4 from Apostolos B.   2012-12-27 18:16:42 ---
I chasnged my DE from gnome 3 to E17 and i don't get that error anymore? Could
it be related?

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.


[Bug 43167] X intel driver causes wrong wraparound of console command line

2012-12-27 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=43167

Jani Nikula  changed:

   What|Removed |Added

 Status|REOPENED|NEEDINFO

--- Comment #42 from Jani Nikula  ---
(In reply to comment #34)
> 1.  I did _exactly_ what you asked me to do.  Twice.  The hard way.
> To BISECT so one can find out where/when exactly this problem happened.
> 
> The bisection requested by _you_, shows clearly and unequivocally
> that the Bug occurred on this change:
> 
> 04fee895ef98ffbb91a941b53a92d6949bb6d1c4 is the first bad commit
> commit 04fee895ef98ffbb91a941b53a92d6949bb6d1c4
> Author: Rolf Eike Beer 
> Date:   Wed Jun 15 11:27:02 2011 +0200
> 
> DRM: clean up and document parsing of video= parameter
> 
> That means that just BEFORE (i.e. any kernel release) there was no problem
> (drm, fb or _otherwise_) and AT THE TIME of this commit (04fee...6d1c4)
> and THEREAFTER this particular Bug showed up and persists to this day.

For completeness, the bisect result points at the first commit that actually
starts using the margins ('m') option of the mode. Before the commit, it was
silently ignored and unused. After the commit, the margins option is used and
clearly does not work for you.

Does dropping 'm' from the modeline solve your problem?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121227/4d4a6112/attachment.html>


Display resolutions missing on external VGA display [regressing]

2012-12-27 Thread Jani Nikula
On Mon, 24 Dec 2012, John  wrote:
> On launchpad they requested to report this bug upstream. Please let me know
> if any information is missing.

Please file a bug on DRM/Intel at [1]. Please reference your mail and
the launchpad bug, no need to attach all the info there again.

What's the latest kernel where this did work for you?

It could prove helpful if you attached the dmesg all the way from boot
with drm.debug=0xe module parameter to attaching the VGA display. The
3.8-rc1 kernel you're running is fine for starters.

Thanks,
Jani.


[1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI


[Bug 43167] X intel driver causes wrong wraparound of console command line

2012-12-27 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=43167

Jani Nikula  changed:

   What|Removed |Added

 CC||jani.nikula at intel.com

--- Comment #41 from Jani Nikula  ---
(In reply to comment #39)
> I just submitted

URL for convenience: http://bugzilla.kernel.org/show_bug.cgi?id=43239

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121227/f376c819/attachment.html>


[PATCH] drm/exynos: fix gem buffer allocation type checking

2012-12-27 Thread Prathyush K
On Thu, Dec 27, 2012 at 4:27 PM, Inki Dae  wrote:

> This patch fixes gem buffer allocation type checking.
> EXYNOS_BO_CONTIG has 0 so the checking should be fixed
> to 'if (!(flags & EXYNOS_BO_NONCONTIG))'
>
> Signed-off-by: Inki Dae 
> Signed-off-by: Kyungmin Park 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_buf.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
> b/drivers/gpu/drm/exynos/exynos_drm_buf.c
> index 74592d1..911f7fd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
> @@ -38,7 +38,7 @@ static int lowlevel_buffer_allocate(struct drm_device
> *dev,
>  * region will be allocated else physically contiguous
>  * as possible.
>  */
> -   if (flags & EXYNOS_BO_CONTIG)
> +   if (!(flags & EXYNOS_BO_NONCONTIG))
> dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &buf->dma_attrs);
>
>
Hi Mr. Dae,

If iommu is supported, we would have called arm_iommu_attach_device.
So dma_alloc_attrs will always call arm_iommu_alloc_attrs.

If iommu is not supported, dma_alloc_attrs will call arm_dma_alloc which
will
anyway allocate a contiguous buffer if possible.

With this code, we are forcing the root framebuffer (fbdev) to be
contiguous since fbdev
allocation does not pass noncontig flag.

Why do we need to force the allocation of a contiguous buffer when iommu is
supported?

Regards,
Prathyush


> /*
> --
> 1.7.4.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
------ next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121227/99de06c0/attachment.html>


[PATCH 04/10] drm/exynos: Use devm_clk_get in exynos_drm_fimc.c

2012-12-27 Thread Sachin Kamat
On 27 December 2012 15:43, Inki Dae  wrote:
>
>
> 2012/12/26 Sachin Kamat 
>>
>>
>>
>> On Wednesday, 26 December 2012, Inki Dae  wrote:
>> >
>> >
>> > 2012/12/24 Sachin Kamat 
>> >>
>> >> This eliminates the need for explicit clk_put and makes the
>> >> cleanup and exit path code simpler.
>> >>
>> >> Cc: Eunchul Kim 
>> >> Signed-off-by: Sachin Kamat 
>> >> ---
>> >>  drivers/gpu/drm/exynos/exynos_drm_fimc.c |   46
>> >> ++---
>> >>  1 files changed, 10 insertions(+), 36 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> >> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> >> index 972aa70..c4006b8 100644
>> >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> >> @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct
>> >> platform_device *pdev)
>> >> platform_get_device_id(pdev)->driver_data;
>> >>
>> >> /* clock control */
>> >> -   ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
>> >> +   ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc");
>> >> if (IS_ERR(ctx->sclk_fimc_clk)) {
>> >> dev_err(dev, "failed to get src fimc clock.\n");
>> >> return PTR_ERR(ctx->sclk_fimc_clk);
>> >> }
>> >> clk_enable(ctx->sclk_fimc_clk);
>> >>
>> >> -   ctx->fimc_clk = clk_get(dev, "fimc");
>> >> +   ctx->fimc_clk = devm_clk_get(dev, "fimc");
>> >> if (IS_ERR(ctx->fimc_clk)) {
>> >> dev_err(dev, "failed to get fimc clock.\n");
>> >> clk_disable(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->sclk_fimc_clk);
>> >> return PTR_ERR(ctx->fimc_clk);
>> >> }
>> >>
>> >> -   ctx->wb_clk = clk_get(dev, "pxl_async0");
>> >> +   ctx->wb_clk = devm_clk_get(dev, "pxl_async0");
>> >> if (IS_ERR(ctx->wb_clk)) {
>> >> dev_err(dev, "failed to get writeback a clock.\n");
>> >> clk_disable(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->fimc_clk);
>> >> return PTR_ERR(ctx->wb_clk);
>> >> }
>> >>
>> >> -   ctx->wb_b_clk = clk_get(dev, "pxl_async1");
>> >> +   ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1");
>> >> if (IS_ERR(ctx->wb_b_clk)) {
>> >> dev_err(dev, "failed to get writeback b clock.\n");
>> >> clk_disable(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->fimc_clk);
>> >> -   clk_put(ctx->wb_clk);
>> >> return PTR_ERR(ctx->wb_b_clk);
>> >> }
>> >>
>> >> -   parent_clk = clk_get(dev, ddata->parent_clk);
>> >> +   parent_clk = devm_clk_get(dev, ddata->parent_clk);
>> >>
>> >> if (IS_ERR(parent_clk)) {
>> >> dev_err(dev, "failed to get parent clock.\n");
>> >> clk_disable(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->fimc_clk);
>> >> -   clk_put(ctx->wb_clk);
>> >> -   clk_put(ctx->wb_b_clk);
>> >> return PTR_ERR(parent_clk);
>> >> }
>> >>
>> >> if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) {
>> >> dev_err(dev, "failed to set parent.\n");
>> >> -   clk_put(parent_clk);
>> >> +   devm_clk_put(dev, parent_clk);
>> >
>> > remove the above call. is there any reason that devm_clk_put should be
>> > called here?
>>
>> Devm resources are freed/released automatically only when the device
>> detaches. In the above case the clock was released explicitly (for whatever
>> reasons) earlier. Hence i have used devm call to keep the code logic same as
>> i do not know the behavior if this clock is 'put' when the device detaches
>> instead of here.
>>
>
> If probe is failed, devm resources are released by devres_release_all(). So
> that is unnecessary call. Remove it.

In this case you are right. It is not required. I will remove it.
But in the below case it is not part of cleanup, so I will keep it.
What is your opinion about it?

>
>>
>>
>> >
>> >>
>> >> clk_disable(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->sclk_fimc_clk);
>> >> -   clk_put(ctx->fimc_clk);
>> >> -   clk_put(ctx->wb_clk);
>> >> -   clk_put(ctx->wb_b_clk);
>> >> return -EINVAL;
>> >> }
>> >>
>> >> -   clk_put(parent_clk);
>> >> +   devm_clk_put(dev, parent_clk);
>> >
>> > ditto.
>> >
>> >>
>> >> clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate);
>> >>
>> >> /* resource memory */
>> >> @@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct
>> >> platform_device *pdev)
>> >> ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res);
>> >> if (!ctx->regs) {
>> >> dev_err(dev, "failed to m

Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Sascha Hauer
On Thu, Dec 27, 2012 at 01:57:56PM -0600, Rob Clark wrote:
> On Thu, Dec 27, 2012 at 1:18 PM, Sascha Hauer  wrote:
> > On Thu, Dec 27, 2012 at 09:54:55AM -0600, Rob Clark wrote:
> >> On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
> >
> > This implies that the master driver knows all potential subdevices,
> > something which is not true for SoCs which have external i2c encoders
> > attached to unrelated i2c controllers.
> 
> well, it can be brute-forced..  ie. drm driver calls common
> register_all_panels() fxn, which, well, registers all the
> panel/display subdev's based on their corresponding CONFIG_FOO_PANEL
> defines.  If you anyways aren't building the panels as separate
> modules, that would work.  Maybe not the most *elegant* approach, but
> simple and functional.
> 
> I guess it partly depends on the structure in devicetree.  If you are
> assuming that the i2c encoder belongs inside the i2c bus, like:
> 
>   &i2cN {
> foo-i2c-encoder {
>   
> };
>   };
> 
> and you are letting devicetree create the devices, then it doesn't
> quite work.  I'm not entirely convinced you should do it that way.
> Really any device like that is going to be hooked up to at least a
> couple busses..  i2c, some sort of bus carrying pixel data, maybe some
> gpio's, etc.  So maybe makes more sense for a virtual drm/kms bus, and
> then use phandle stuff to link it to the various other busses it
> needs:
> 
>   mydrmdev {
> foo-i2c-encoder {
>i2c = <&i2cN>;
>gpio = <&gpioM 2 3>
>...
> };
>   };

This seems to shift initialization order problem to another place.
Here we have to make sure the controller is initialized before the drm
driver. Same with suspend/resume.

It's not only i2c devices, also platform devices. On i.MX for example we
have a hdmi transmitter which is somewhere on the physical address
space.

I think grouping the different units together in a devicetree blob
because we think they might form a logical virtual device is not going
to work. It might make it easier from a drm perspective, but I think
doing this will make for a lot of special cases. What will happen for
example if you have two encoder devices in a row to configure? The
foo-i2c-encoder would then get another child node.

Right now the devicetree is strictly ordered by (control-, not data-)
bus topology. Linux has great helper code to support this model. Giving
up this help to brute force a different topology and then trying to fit
the result back into the Linux Bus hierarchy doesn't sound like a good
idea to me.

> 
> ok, admittedly that is a bit different from other proposals about how
> this all fits in devicetree.. but otoh, I'm not a huge believer in
> letting something that is supposed to make life easier (DT), actually
> make things harder or more complicated.  Plus this CDF stuff all needs
> to also work on platforms not using OF/DT.

Right, but every other platform I know of is also described by its bus
topology, be it platform device based or PCI or maybe even USB based.

CDF has to solve the same problem as ASoC and soc-camera: subdevices for
a virtual device can come from many different corners of the system. BTW
one example for a i2c encoder would be the SiI9022 which could not only
be part of a drm device, but also of an ASoC device.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC v2 0/5] Common Display Framework

2012-12-27 Thread Tomasz Figa
Hi Laurent,

On Monday 24 of December 2012 15:12:28 Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Friday 21 December 2012 11:00:52 Tomasz Figa wrote:
> > On Tuesday 18 of December 2012 08:31:30 Vikas Sajjan wrote:
> > > On 17 December 2012 20:55, Laurent Pinchart wrote:
> > > > Hi Vikas,
> > > > 
> > > > Sorry for the late reply. I now have more time to work on CDF, so
> > > > delays should be much shorter.
> > > > 
> > > > On Thursday 06 December 2012 10:51:15 Vikas Sajjan wrote:
> > > > > Hi Laurent,
> > > > > 
> > > > > I was thinking of porting CDF to samsung EXYNOS 5250 platform,
> > > > > what
> > > > > I found is that, the exynos display controller is MIPI DSI based
> > > > > controller.
> > > > > 
> > > > > But if I look at CDF patches, it has only support for MIPI DBI
> > > > > based
> > > > > Display controller.
> > > > > 
> > > > > So my question is, do we have any generic framework for MIPI DSI
> > > > > based display controller? basically I wanted to know, how to go
> > > > > about
> > > > > porting CDF for such kind of display controller.
> > > > 
> > > > MIPI DSI support is not available yet. The only reason for that is
> > > > that I don't have any MIPI DSI hardware to write and test the code
> > > > with :-)
> > > > 
> > > > The common display framework should definitely support MIPI DSI. I
> > > > think the existing MIPI DBI code could be used as a base, so the
> > > > implementation shouldn't be too high.
> > > > 
> > > > Yeah, i was also thinking in similar lines, below is my though for
> > > > MIPI DSI support in CDF.
> > > 
> > > o   MIPI DSI support as part of CDF framework will expose
> > > ?  mipi_dsi_register_device(mpi_device) (will be called
> > > mach-xxx-dt.c
> > > file )
> > > ?  mipi_dsi_register_driver(mipi_driver, bus ops) (will be called
> > > from
> > > platform specific init driver call )
> > > ?bus ops will be
> > > o   read data
> > > o   write data
> > > o   write command
> > > ?  MIPI DSI will be registered as bus_register()
> > > 
> > > When MIPI DSI probe is called, it (e.g., Exynos or OMAP MIPI DSI)
> > > will
> > > initialize the MIPI DSI HW IP.
> > > 
> > > This probe will also parse the DT file for MIPI DSI based panel, add
> > > the panel device (device_add() ) to kernel and register the display
> > > entity with its control and  video ops with CDF.
> > > 
> > > I can give this a try.
> > 
> > I am currently in progress of reworking Exynos MIPI DSIM code and
> > s6e8ax0 LCD driver to use the v2 RFC of Common Display Framework. I
> > have most of the work done, I have just to solve several remaining
> > problems.
> Do you already have code that you can publish ? I'm particularly
> interested (and I think Tomi Valkeinen would be as well) in looking at
> the DSI operations you expose to DSI sinks (panels, transceivers, ...).

Well, I'm afraid this might be little below your expectations, but here's 
an initial RFC of the part defining just the DSI bus. I need a bit more 
time for patches for Exynos MIPI DSI master and s6e8ax0 LCD.

The implementation is very simple and heavily based on your MIPI DBI 
support and existing Exynos MIPI DSIM framework. Provided operation set is 
based on operation set used by Exynos s6e8ax0 LCD driver. Unfortunately 
this is my only source of information about MIPI DSI.

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

>From bad07d8bdce0ff76cbc81a9da597c0d01e5244f7 Mon Sep 17 00:00:00 2001
From: Tomasz Figa 
Date: Thu, 27 Dec 2012 12:36:15 +0100
Subject: [RFC] video: display: Add generic MIPI DSI bus

Signed-off-by: Tomasz Figa 
---
 drivers/video/display/Kconfig|   4 +
 drivers/video/display/Makefile   |   1 +
 drivers/video/display/mipi-dsi-bus.c | 214 
+++
 include/video/display.h  |   1 +
 include/video/mipi-dsi-bus.h |  98 
 5 files changed, 318 insertions(+)
 create mode 100644 drivers/video/display/mipi-dsi-bus.c
 create mode 100644 include/video/mipi-dsi-bus.h

diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
index 13b6aaf..dbaff9d 100644
--- a/drivers/video/display/Kconfig
+++ b/drivers/video/display/Kconfig
@@ -9,6 +9,10 @@ config DISPLAY_MIPI_DBI
tristate
default n

+config DISPLAY_MIPI_DSI
+   tristate
+   default n
+
 config DISPLAY_PANEL_DPI
tristate "DPI (Parallel) Display Panels"
---help---
diff --git a/drivers/video/display/Makefile 
b/drivers/video/display/Makefile
index 482bec7..429b3ac8 100644
--- a/drivers/video/display/Makefile
+++ b/drivers/video/display/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_DISPLAY_CORE) += display-core.o
 obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o
+obj-$(CONFIG_DISPLAY_MIPI_DSI) += mipi-dsi-bus.o
 obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o
 obj-$(CONFIG_DISPLAY_PANEL_R61505) += panel-r61505.o
 obj-$(CONFIG_DISPLAY_PANEL_R61517) += panel-r61517.o
diff --git a/drivers/video/display/mipi-ds

[PATCH] video: drm: exynos: mie bypass enable for fimd

2012-12-27 Thread Inki Dae
Hi,

DISP1BLK_CFG register is related to GScaler, HDCP and MIXER as well. So
it's not good that this register is controlled in fimd module. And I think
the function to control the register should be placed in SoC common file .
In other words, other drivers should be able to control the register
through common thing also.

Thanks,
Inki Dae

2012/12/26 Leela Krishna Amudala 

> Bypasses the mie for fimd by parsing the register and bit offset values
> from "mie-bypass" node, if "mie-bypass" node is present in the dts file.
>
> Signed-off-by: Leela Krishna Amudala 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 55
> 
>  1 file changed, 55 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index bf0d9ba..f8ad259 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -118,6 +118,12 @@ static const struct of_device_id
> fimd_driver_dt_match[] = {
>  MODULE_DEVICE_TABLE(of, fimd_driver_dt_match);
>  #endif
>
> +struct mie_bypass {
> +   u32 enable_bypass;
> +   void __iomem*bypass_reg;
> +   u32 bypass_bit_offset;
> +};
> +
>  static inline struct fimd_driver_data *drm_fimd_get_driver_data(
> struct platform_device *pdev)
>  {
> @@ -133,6 +139,41 @@ static inline struct fimd_driver_data
> *drm_fimd_get_driver_data(
> platform_get_device_id(pdev)->driver_data;
>  }
>
> +static struct mie_bypass *parse_mie_bypass_for_fimd(struct device *dev,
> +   struct device_node
> *mie_bypass_node)
> +{
> +   struct mie_bypass *bypass_data;
> +   u32 phy_address;
> +
> +   bypass_data = devm_kzalloc(dev, sizeof(*bypass_data), GFP_KERNEL);
> +   if (!bypass_data) {
> +   dev_err(dev, "memory allocation for bypass data failed\n");
> +   return ERR_PTR(-ENOMEM);
> +   }
> +   of_property_read_u32(mie_bypass_node, "samsung,mie-bypass-enable",
> +   &bypass_data->enable_bypass);
> +   of_property_read_u32(mie_bypass_node, "samsung,disp1blk-cfg-reg",
> +   &phy_address);
> +   of_property_read_u32(mie_bypass_node, "samsung,bypass-bit-offset",
> +   &bypass_data->bypass_bit_offset);
> +
> +   bypass_data->bypass_reg = ioremap(phy_address, SZ_4);
>
+   if (!bypass_data->bypass_reg) {
> +   dev_err(dev, "failed to ioremap phy_address\n");
> +   return ERR_PTR(-ENOMEM);
> +   }
> +   return bypass_data;
> +}
>
+
> +static void mie_bypass_for_fimd(struct mie_bypass *bypass_data)
> +{
> +   u32 reg;
> +
> +   reg = __raw_readl(bypass_data->bypass_reg);
> +   reg |= (1 << bypass_data->bypass_bit_offset);
> +   __raw_writel(reg, bypass_data->bypass_reg);
> +}
> +
>  static bool fimd_display_is_connected(struct device *dev)
>  {
> DRM_DEBUG_KMS("%s\n", __FILE__);
> @@ -906,12 +947,26 @@ static int __devinit fimd_probe(struct
> platform_device *pdev)
> struct exynos_drm_fimd_pdata *pdata;
> struct exynos_drm_panel_info *panel;
> struct resource *res;
> +   struct device_node *mie_bypass_node;
> +   struct mie_bypass *bypass_data = NULL;
> int win;
> int ret = -EINVAL;
>
> DRM_DEBUG_KMS("%s\n", __FILE__);
>
> pdata = pdev->dev.platform_data;
> +   if (pdev->dev.of_node) {
> +   mie_bypass_node = of_find_node_by_name(pdev->dev.of_node,
> +   "mie-bypass");
> +   if (mie_bypass_node) {
> +   bypass_data = parse_mie_bypass_for_fimd(&pdev->dev,
> +   mie_bypass_node);
>
+   if (IS_ERR(bypass_data))
> +   return PTR_ERR(bypass_data);
> +   if (bypass_data->enable_bypass)
> +   mie_bypass_for_fimd(bypass_data);
> +   }
> +   }
> if (!pdata) {
> dev_err(dev, "no platform data specified\n");
> return -EINVAL;
> --
> 1.8.0
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121227/1e9d044c/attachment.html>


[RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Thu, Dec 27, 2012 at 1:18 PM, Sascha Hauer  wrote:
> On Thu, Dec 27, 2012 at 09:54:55AM -0600, Rob Clark wrote:
>> On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
>>  wrote:
>> > Hi Rob,
>> >
>> > On Tuesday 18 December 2012 00:21:32 Rob Clark wrote:
>> >> On Mon, Dec 17, 2012 at 11:04 PM, Dave Airlie  
>> >> wrote:
>> >> >> Many developers showed interest in the first RFC, and I've had the
>> >> >> opportunity to discuss it with most of them. I would like to thank (in
>> >> >> no particular order) Tomi Valkeinen for all the time he spend helping 
>> >> >> me
>> >> >> to draft v2, Marcus Lorentzon for his useful input during Linaro 
>> >> >> Connect
>> >> >> Q4 2012, and Linaro for inviting me to Connect and providing a venue to
>> >> >> discuss this topic.
>> >> >
>> >> > So this might be a bit off topic but this whole CDF triggered me
>> >> > looking at stuff I generally avoid:
>> >> >
>> >> > The biggest problem I'm having currently with the whole ARM graphics
>> >> > and output world is the proliferation of platform drivers for every
>> >> > little thing. The whole ordering of operations with respect to things
>> >> > like suspend/resume or dynamic power management is going to be a real
>> >> > nightmare if there are dependencies between the drivers. How do you
>> >> > enforce ordering of s/r operations between all the various components?
>> >>
>> >> I tend to think that sub-devices are useful just to have a way to probe hw
>> >> which may or may not be there, since on ARM we often don't have any
>> >> alternative.. but beyond that, suspend/resume, and other life-cycle 
>> >> aspects,
>> >> they should really be treated as all one device. Especially to avoid
>> >> undefined suspend/resume ordering.
>> >
>> > I tend to agree, except that I try to reuse the existing PM infrastructure
>> > when possible to avoid reinventing the wheel. So far handling 
>> > suspend/resume
>> > ordering related to data busses in early suspend/late resume operations and
>> > allowing the Linux PM core to handle control busses using the Linux device
>> > tree worked pretty well.
>> >
>> >> CDF or some sort of mechanism to share panel drivers between drivers is
>> >> useful.  Keeping it within drm, is probably a good idea, if nothing else 
>> >> to
>> >> simplify re-use of helper fxns (like avi-infoframe stuff, for example) and
>> >> avoid dealing with merging changes across multiple trees. Treating them 
>> >> more
>> >> like shared libraries and less like sub-devices which can be dynamically
>> >> loaded/unloaded (ie. they should be not built as separate modules or
>> >> suspend/resumed or probed/removed independently of the master driver) is a
>> >> really good idea to avoid uncovering nasty synchronization issues later
>> >> (remove vs modeset or pageflip) or surprising userspace in bad ways.
>> >
>> > We've tried that in V4L2 years ago and realized that the approach led to a
>> > dead-end, especially when OF/DT got involved. With DT-based device probing,
>> > I2C camera sensors started getting probed asynchronously to the main camera
>> > device, as they are children of the I2C bus master. We will have similar
>> > issues with I2C HDMI transmitters or panels, so we should be prepared for 
>> > it.
>>
>> What I've done to avoid that so far is that the master device
>> registers the drivers for it's output sub-devices before registering
>> it's own device.  At least this way I can control that they are probed
>> first.  Not the prettiest thing, but avoids even uglier problems.
>
> This implies that the master driver knows all potential subdevices,
> something which is not true for SoCs which have external i2c encoders
> attached to unrelated i2c controllers.

well, it can be brute-forced..  ie. drm driver calls common
register_all_panels() fxn, which, well, registers all the
panel/display subdev's based on their corresponding CONFIG_FOO_PANEL
defines.  If you anyways aren't building the panels as separate
modules, that would work.  Maybe not the most *elegant* approach, but
simple and functional.

I guess it partly depends on the structure in devicetree.  If you are
assuming that the i2c encoder belongs inside the i2c bus, like:

  &i2cN {
foo-i2c-encoder {
  
};
  };

and you are letting devicetree create the devices, then it doesn't
quite work.  I'm not entirely convinced you should do it that way.
Really any device like that is going to be hooked up to at least a
couple busses..  i2c, some sort of bus carrying pixel data, maybe some
gpio's, etc.  So maybe makes more sense for a virtual drm/kms bus, and
then use phandle stuff to link it to the various other busses it
needs:

  mydrmdev {
foo-i2c-encoder {
   i2c = <&i2cN>;
   gpio = <&gpioM 2 3>
   ...
};
  };

ok, admittedly that is a bit different from other proposals about how
this all fits in devicetree.. but otoh, I'm not a huge believer in
letting something that is supposed to make life easier (DT), actually
make things harde

Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Thu, Dec 27, 2012 at 1:18 PM, Sascha Hauer  wrote:
> On Thu, Dec 27, 2012 at 09:54:55AM -0600, Rob Clark wrote:
>> On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
>>  wrote:
>> > Hi Rob,
>> >
>> > On Tuesday 18 December 2012 00:21:32 Rob Clark wrote:
>> >> On Mon, Dec 17, 2012 at 11:04 PM, Dave Airlie  wrote:
>> >> >> Many developers showed interest in the first RFC, and I've had the
>> >> >> opportunity to discuss it with most of them. I would like to thank (in
>> >> >> no particular order) Tomi Valkeinen for all the time he spend helping 
>> >> >> me
>> >> >> to draft v2, Marcus Lorentzon for his useful input during Linaro 
>> >> >> Connect
>> >> >> Q4 2012, and Linaro for inviting me to Connect and providing a venue to
>> >> >> discuss this topic.
>> >> >
>> >> > So this might be a bit off topic but this whole CDF triggered me
>> >> > looking at stuff I generally avoid:
>> >> >
>> >> > The biggest problem I'm having currently with the whole ARM graphics
>> >> > and output world is the proliferation of platform drivers for every
>> >> > little thing. The whole ordering of operations with respect to things
>> >> > like suspend/resume or dynamic power management is going to be a real
>> >> > nightmare if there are dependencies between the drivers. How do you
>> >> > enforce ordering of s/r operations between all the various components?
>> >>
>> >> I tend to think that sub-devices are useful just to have a way to probe hw
>> >> which may or may not be there, since on ARM we often don't have any
>> >> alternative.. but beyond that, suspend/resume, and other life-cycle 
>> >> aspects,
>> >> they should really be treated as all one device. Especially to avoid
>> >> undefined suspend/resume ordering.
>> >
>> > I tend to agree, except that I try to reuse the existing PM infrastructure
>> > when possible to avoid reinventing the wheel. So far handling 
>> > suspend/resume
>> > ordering related to data busses in early suspend/late resume operations and
>> > allowing the Linux PM core to handle control busses using the Linux device
>> > tree worked pretty well.
>> >
>> >> CDF or some sort of mechanism to share panel drivers between drivers is
>> >> useful.  Keeping it within drm, is probably a good idea, if nothing else 
>> >> to
>> >> simplify re-use of helper fxns (like avi-infoframe stuff, for example) and
>> >> avoid dealing with merging changes across multiple trees. Treating them 
>> >> more
>> >> like shared libraries and less like sub-devices which can be dynamically
>> >> loaded/unloaded (ie. they should be not built as separate modules or
>> >> suspend/resumed or probed/removed independently of the master driver) is a
>> >> really good idea to avoid uncovering nasty synchronization issues later
>> >> (remove vs modeset or pageflip) or surprising userspace in bad ways.
>> >
>> > We've tried that in V4L2 years ago and realized that the approach led to a
>> > dead-end, especially when OF/DT got involved. With DT-based device probing,
>> > I2C camera sensors started getting probed asynchronously to the main camera
>> > device, as they are children of the I2C bus master. We will have similar
>> > issues with I2C HDMI transmitters or panels, so we should be prepared for 
>> > it.
>>
>> What I've done to avoid that so far is that the master device
>> registers the drivers for it's output sub-devices before registering
>> it's own device.  At least this way I can control that they are probed
>> first.  Not the prettiest thing, but avoids even uglier problems.
>
> This implies that the master driver knows all potential subdevices,
> something which is not true for SoCs which have external i2c encoders
> attached to unrelated i2c controllers.

well, it can be brute-forced..  ie. drm driver calls common
register_all_panels() fxn, which, well, registers all the
panel/display subdev's based on their corresponding CONFIG_FOO_PANEL
defines.  If you anyways aren't building the panels as separate
modules, that would work.  Maybe not the most *elegant* approach, but
simple and functional.

I guess it partly depends on the structure in devicetree.  If you are
assuming that the i2c encoder belongs inside the i2c bus, like:

  &i2cN {
foo-i2c-encoder {
  
};
  };

and you are letting devicetree create the devices, then it doesn't
quite work.  I'm not entirely convinced you should do it that way.
Really any device like that is going to be hooked up to at least a
couple busses..  i2c, some sort of bus carrying pixel data, maybe some
gpio's, etc.  So maybe makes more sense for a virtual drm/kms bus, and
then use phandle stuff to link it to the various other busses it
needs:

  mydrmdev {
foo-i2c-encoder {
   i2c = <&i2cN>;
   gpio = <&gpioM 2 3>
   ...
};
  };

ok, admittedly that is a bit different from other proposals about how
this all fits in devicetree.. but otoh, I'm not a huge believer in
letting something that is supposed to make life easier (DT), actually
make things harder or mo

Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Sascha Hauer
On Thu, Dec 27, 2012 at 10:04:22AM -0600, Rob Clark wrote:
> On Mon, Dec 24, 2012 at 11:27 AM, Laurent Pinchart
>  wrote:
> > On Wednesday 19 December 2012 16:57:56 Jani Nikula wrote:
> >> It just seems to me that, at least from a DRM/KMS perspective, adding
> >> another layer (=CDF) for HDMI or DP (or legacy outputs) would be
> >> overengineering it. They are pretty well standardized, and I don't see 
> >> there
> >> would be a need to write multiple display drivers for them. Each display
> >> controller has one, and can easily handle any chip specific requirements
> >> right there. It's my gut feeling that an additional framework would just 
> >> get
> >> in the way. Perhaps there could be more common HDMI/DP helper style code in
> >> DRM to reduce overlap across KMS drivers, but that's another thing.
> >>
> >> So is the HDMI/DP drivers using CDF a more interesting idea from a non-DRM
> >> perspective? Or, put another way, is it more of an alternative to using 
> >> DRM?
> >> Please enlighten me if there's some real benefit here that I fail to see!
> >
> > As Rob pointed out, you can have external HDMI/DP encoders, and even 
> > internal
> > HDMI/DP encoder IPs can be shared between SoCs and SoC vendors. CDF aims at
> > sharing a single driver between SoCs and boards for a given HDMI/DP encoder.
> 
> just fwiw, drm already has something a bit like this.. the i2c
> encoder-slave.  With support for a couple external i2c encoders which
> could in theory be shared between devices.

The problem with this code is that it only works when the i2c device is
registered by a master driver. Once the i2c device comes from the
devicetree there is no possibility to find it.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Sascha Hauer
On Thu, Dec 27, 2012 at 09:54:55AM -0600, Rob Clark wrote:
> On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
>  wrote:
> > Hi Rob,
> >
> > On Tuesday 18 December 2012 00:21:32 Rob Clark wrote:
> >> On Mon, Dec 17, 2012 at 11:04 PM, Dave Airlie  wrote:
> >> >> Many developers showed interest in the first RFC, and I've had the
> >> >> opportunity to discuss it with most of them. I would like to thank (in
> >> >> no particular order) Tomi Valkeinen for all the time he spend helping me
> >> >> to draft v2, Marcus Lorentzon for his useful input during Linaro Connect
> >> >> Q4 2012, and Linaro for inviting me to Connect and providing a venue to
> >> >> discuss this topic.
> >> >
> >> > So this might be a bit off topic but this whole CDF triggered me
> >> > looking at stuff I generally avoid:
> >> >
> >> > The biggest problem I'm having currently with the whole ARM graphics
> >> > and output world is the proliferation of platform drivers for every
> >> > little thing. The whole ordering of operations with respect to things
> >> > like suspend/resume or dynamic power management is going to be a real
> >> > nightmare if there are dependencies between the drivers. How do you
> >> > enforce ordering of s/r operations between all the various components?
> >>
> >> I tend to think that sub-devices are useful just to have a way to probe hw
> >> which may or may not be there, since on ARM we often don't have any
> >> alternative.. but beyond that, suspend/resume, and other life-cycle 
> >> aspects,
> >> they should really be treated as all one device. Especially to avoid
> >> undefined suspend/resume ordering.
> >
> > I tend to agree, except that I try to reuse the existing PM infrastructure
> > when possible to avoid reinventing the wheel. So far handling suspend/resume
> > ordering related to data busses in early suspend/late resume operations and
> > allowing the Linux PM core to handle control busses using the Linux device
> > tree worked pretty well.
> >
> >> CDF or some sort of mechanism to share panel drivers between drivers is
> >> useful.  Keeping it within drm, is probably a good idea, if nothing else to
> >> simplify re-use of helper fxns (like avi-infoframe stuff, for example) and
> >> avoid dealing with merging changes across multiple trees. Treating them 
> >> more
> >> like shared libraries and less like sub-devices which can be dynamically
> >> loaded/unloaded (ie. they should be not built as separate modules or
> >> suspend/resumed or probed/removed independently of the master driver) is a
> >> really good idea to avoid uncovering nasty synchronization issues later
> >> (remove vs modeset or pageflip) or surprising userspace in bad ways.
> >
> > We've tried that in V4L2 years ago and realized that the approach led to a
> > dead-end, especially when OF/DT got involved. With DT-based device probing,
> > I2C camera sensors started getting probed asynchronously to the main camera
> > device, as they are children of the I2C bus master. We will have similar
> > issues with I2C HDMI transmitters or panels, so we should be prepared for 
> > it.
> 
> What I've done to avoid that so far is that the master device
> registers the drivers for it's output sub-devices before registering
> it's own device.  At least this way I can control that they are probed
> first.  Not the prettiest thing, but avoids even uglier problems.

This implies that the master driver knows all potential subdevices,
something which is not true for SoCs which have external i2c encoders
attached to unrelated i2c controllers.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 57875] Second Life viewer bad rendering with git-ec83535

2012-12-27 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=57875

--- Comment #18 from Piero Finizio  ---
If i try to revert the commit e866bd1adea2c3b4971ad68e69c644752f2ab7b6  (above
mentioned workaround) with git
"HEAD at 7c35521 mesa: add missing texel fetch code for sRGB DXT formats"
the compilation ends with:
 ...
CC r300_screen.o
r300_screen.c: In function ?r300_get_param?:
r300_screen.c:130:14: error: ?PIPE_CAP_TIMER_QUERY? undeclared (first use in
this function)
r300_screen.c:130:14: note: each undeclared identifier is reported only once
for each function it appears in
r300_screen.c:88:5: warning: enumeration value ?PIPE_CAP_QUERY_TIME_ELAPSED?
not handled in switch [-Wswitch]
r300_screen.c:88:5: warning: enumeration value
?PIPE_CAP_TEXTURE_BUFFER_OBJECTS? not handled in switch [-Wswitch]
gmake[4]: *** [r300_screen.o] Error 1
gmake[4]: Leaving directory `/root/mesa/src/gallium/drivers/r300'
gmake[3]: *** [all-recursive] Error 1

 ...

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121227/3f996185/attachment.html>


[Bug 49981] [drm:radeon_cs_ioctl] *ERROR* Failed to parse relocation -12!

2012-12-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=49981





--- Comment #4 from Apostolos B.   2012-12-27 18:16:42 ---
I chasnged my DE from gnome 3 to E17 and i don't get that error anymore? Could
it be related?

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Mon, Dec 24, 2012 at 11:35 AM, Laurent Pinchart
 wrote:
> On Wednesday 19 December 2012 09:26:40 Rob Clark wrote:
>> And, there are also external HDMI encoders (for example connected over
>> i2c) that can also be shared between boards.  So I think there will be
>> a number of cases where CDF is appropriate for HDMI drivers.  Although
>> trying to keep this all independent of DRM (as opposed to just
>> something similar to what drivers/gpu/i2c is today) seems a bit
>> overkill for me.  Being able to use the helpers in drm and avoiding an
>> extra layer of translation seems like the better option to me.  So my
>> vote would be drivers/gpu/cdf.
>
> I don't think there will be any need for translation (except perhaps between
> the DRM mode structures and the common video mode structure that is being
> discussed). Add a drm_ prefix to the existing CDF functions and structures,
> and there you go :-)

well, and translation for any properties that we'd want to expose to
userspace, etc, etc.. I see there being a big potential for a lot of
needless glue

BR,
-R

> The reason why I'd like to keep CDF separate from DRM (or at least not
> requiring a drm_device) is that HDMI/DP encoders can be used by pure V4L2
> drivers.
>
>> > For DSI panels (or DSI-to-whatever bridges) it's of course another
>> > story. You typically need a panel specific driver. And here I see the
>> > main point of the whole CDF: decoupling display controllers and the
>> > panel drivers, and sharing panel (and converter chip) specific drivers
>> > across display controllers. Making it easy to write new drivers, as
>> > there would be a model to follow. I'm definitely in favour of coming up
>> > with some framework that would tackle that.
>
> --
> Regards,
>
> Laurent Pinchart
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Mon, Dec 24, 2012 at 11:27 AM, Laurent Pinchart
 wrote:
> On Wednesday 19 December 2012 16:57:56 Jani Nikula wrote:
>> It just seems to me that, at least from a DRM/KMS perspective, adding
>> another layer (=CDF) for HDMI or DP (or legacy outputs) would be
>> overengineering it. They are pretty well standardized, and I don't see there
>> would be a need to write multiple display drivers for them. Each display
>> controller has one, and can easily handle any chip specific requirements
>> right there. It's my gut feeling that an additional framework would just get
>> in the way. Perhaps there could be more common HDMI/DP helper style code in
>> DRM to reduce overlap across KMS drivers, but that's another thing.
>>
>> So is the HDMI/DP drivers using CDF a more interesting idea from a non-DRM
>> perspective? Or, put another way, is it more of an alternative to using DRM?
>> Please enlighten me if there's some real benefit here that I fail to see!
>
> As Rob pointed out, you can have external HDMI/DP encoders, and even internal
> HDMI/DP encoder IPs can be shared between SoCs and SoC vendors. CDF aims at
> sharing a single driver between SoCs and boards for a given HDMI/DP encoder.

just fwiw, drm already has something a bit like this.. the i2c
encoder-slave.  With support for a couple external i2c encoders which
could in theory be shared between devices.

BR,
-R


[RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Mon, Dec 24, 2012 at 11:09 AM, Laurent Pinchart
 wrote:
> On the topic of discussions, would anyone be interested in a
> BoF/brainstorming/whatever session during the FOSDEM ?

I will be at FOSDEM.. and from http://wiki.x.org/wiki/fosdem2013 it
looks like at least Daniel will be there.  If enough others are, it
could be a good idea.

BR,
-R


[RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Tuesday 18 December 2012 00:21:32 Rob Clark wrote:
>> On Mon, Dec 17, 2012 at 11:04 PM, Dave Airlie  wrote:
>> >> Many developers showed interest in the first RFC, and I've had the
>> >> opportunity to discuss it with most of them. I would like to thank (in
>> >> no particular order) Tomi Valkeinen for all the time he spend helping me
>> >> to draft v2, Marcus Lorentzon for his useful input during Linaro Connect
>> >> Q4 2012, and Linaro for inviting me to Connect and providing a venue to
>> >> discuss this topic.
>> >
>> > So this might be a bit off topic but this whole CDF triggered me
>> > looking at stuff I generally avoid:
>> >
>> > The biggest problem I'm having currently with the whole ARM graphics
>> > and output world is the proliferation of platform drivers for every
>> > little thing. The whole ordering of operations with respect to things
>> > like suspend/resume or dynamic power management is going to be a real
>> > nightmare if there are dependencies between the drivers. How do you
>> > enforce ordering of s/r operations between all the various components?
>>
>> I tend to think that sub-devices are useful just to have a way to probe hw
>> which may or may not be there, since on ARM we often don't have any
>> alternative.. but beyond that, suspend/resume, and other life-cycle aspects,
>> they should really be treated as all one device. Especially to avoid
>> undefined suspend/resume ordering.
>
> I tend to agree, except that I try to reuse the existing PM infrastructure
> when possible to avoid reinventing the wheel. So far handling suspend/resume
> ordering related to data busses in early suspend/late resume operations and
> allowing the Linux PM core to handle control busses using the Linux device
> tree worked pretty well.
>
>> CDF or some sort of mechanism to share panel drivers between drivers is
>> useful.  Keeping it within drm, is probably a good idea, if nothing else to
>> simplify re-use of helper fxns (like avi-infoframe stuff, for example) and
>> avoid dealing with merging changes across multiple trees. Treating them more
>> like shared libraries and less like sub-devices which can be dynamically
>> loaded/unloaded (ie. they should be not built as separate modules or
>> suspend/resumed or probed/removed independently of the master driver) is a
>> really good idea to avoid uncovering nasty synchronization issues later
>> (remove vs modeset or pageflip) or surprising userspace in bad ways.
>
> We've tried that in V4L2 years ago and realized that the approach led to a
> dead-end, especially when OF/DT got involved. With DT-based device probing,
> I2C camera sensors started getting probed asynchronously to the main camera
> device, as they are children of the I2C bus master. We will have similar
> issues with I2C HDMI transmitters or panels, so we should be prepared for it.

What I've done to avoid that so far is that the master device
registers the drivers for it's output sub-devices before registering
it's own device.  At least this way I can control that they are probed
first.  Not the prettiest thing, but avoids even uglier problems.

> On PC hardware the I2C devices are connected to an I2C master provided by the
> GPU, but on embedded devices they are usually connected to an independent I2C
> master. We thus can't have a single self-contained driver that controls
> everything internally, and need to interface with the rest of the SoC drivers.
>
> I agree that probing/removing devices independently of the master driver can
> lead to bad surprises, which is why I want to establish clear rules in CDF
> regarding what can and can't be done with display entities. Reference counting
> will be one way to make sure that devices don't disappear all of a sudden.

That at least helps cover some issues.. although it doesn't really
help userspace confusion.

Anyways, with enough work perhaps all problems could be solved..
otoh, there are plenty of other important problems to solve in the
world of gpus and kms, so my preference is always not to needlessly
over-complicate CDF and instead leave some time for other things

BR,
-R

>> > The other thing I'd like you guys to do is kill the idea of fbdev and
>> > v4l drivers that are "shared" with the drm codebase, really just
>> > implement fbdev and v4l on top of the drm layer, some people might
>> > think this is some sort of maintainer thing, but really nothing else
>> > makes sense, and having these shared display frameworks just to avoid
>> > having using drm/kms drivers seems totally pointless. Fix the drm
>> > fbdev emulation if an fbdev interface is needed. But creating a fourth
>> > framework because our previous 3 frameworks didn't work out doesn't
>> > seem like a situation I want to get behind too much.
>>
>> yeah, let's not have multiple frameworks to do the same thing.. For fbdev,
>> it is pretty clear that it is a dead end.  For v4l2 (subdev+mcf), it is
>> perha

[Bug 43167] X intel driver causes wrong wraparound of console command line

2012-12-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=43167

Jani Nikula  changed:

   What|Removed |Added

 Status|REOPENED|NEEDINFO

--- Comment #42 from Jani Nikula  ---
(In reply to comment #34)
> 1.  I did _exactly_ what you asked me to do.  Twice.  The hard way.
> To BISECT so one can find out where/when exactly this problem happened.
> 
> The bisection requested by _you_, shows clearly and unequivocally
> that the Bug occurred on this change:
> 
> 04fee895ef98ffbb91a941b53a92d6949bb6d1c4 is the first bad commit
> commit 04fee895ef98ffbb91a941b53a92d6949bb6d1c4
> Author: Rolf Eike Beer 
> Date:   Wed Jun 15 11:27:02 2011 +0200
> 
> DRM: clean up and document parsing of video= parameter
> 
> That means that just BEFORE (i.e. any kernel release) there was no problem
> (drm, fb or _otherwise_) and AT THE TIME of this commit (04fee...6d1c4)
> and THEREAFTER this particular Bug showed up and persists to this day.

For completeness, the bisect result points at the first commit that actually
starts using the margins ('m') option of the mode. Before the commit, it was
silently ignored and unused. After the commit, the margins option is used and
clearly does not work for you.

Does dropping 'm' from the modeline solve your problem?

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 43167] X intel driver causes wrong wraparound of console command line

2012-12-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=43167

Jani Nikula  changed:

   What|Removed |Added

 CC||jani.nik...@intel.com

--- Comment #41 from Jani Nikula  ---
(In reply to comment #39)
> I just submitted

URL for convenience: http://bugzilla.kernel.org/show_bug.cgi?id=43239

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm: make frame duration time calculation more precise

2012-12-27 Thread Daniel Kurtz
It is a bit more precise to compute the total number of pixels first and
then divide, rather than multiplying the line pixel count by the
already-rounded line duration.

Signed-off-by: Daniel Kurtz 
---
 drivers/gpu/drm/drm_irq.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 19c01ca..05c91e0 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -505,6 +505,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc)

/* Valid dotclock? */
if (dotclock > 0) {
+   int frame_size;
/* Convert scanline length in pixels and video dot clock to
 * line duration, frame duration and pixel duration in
 * nanoseconds:
@@ -512,7 +513,10 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc)
pixeldur_ns = (s64) div64_u64(10, dotclock);
linedur_ns  = (s64) div64_u64(((u64) crtc->hwmode.crtc_htotal *
  10), dotclock);
-   framedur_ns = (s64) crtc->hwmode.crtc_vtotal * linedur_ns;
+   frame_size = crtc->hwmode.crtc_htotal *
+   crtc->hwmode.crtc_vtotal;
+   framedur_ns = (s64) div64_u64((u64) frame_size * 10,
+ dotclock);
} else
DRM_ERROR("crtc %d: Can't calculate constants, dotclock = 0!\n",
  crtc->base.id);
-- 
1.7.7.3



[PATCH] drm: fixed access to PCI host bridges

2012-12-27 Thread Dave Airlie
On Thu, Dec 27, 2012 at 8:40 AM, Bjorn Helgaas  wrote:
> On Sat, Dec 22, 2012 at 12:01 PM, Lucas Kannebley Tavares
>  wrote:
>> During the process of obtaining the speed cap for the device, it
>> attempts go get the PCI Host bus. However on architectures such as PPC
>> or IA64, those do not appear as devices.
>>
>> Signed-off-by: Lucas Kannebley Tavares 
>> ---
>>  drivers/gpu/drm/drm_pci.c |5 +
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
>> index 754bc96..ea41234 100644
>> --- a/drivers/gpu/drm/drm_pci.c
>> +++ b/drivers/gpu/drm/drm_pci.c
>> @@ -479,8 +479,13 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev,
>> u32 *mask)
>> if (!pci_is_pcie(dev->pdev))
>> return -EINVAL;
>>
>> +   // find PCI device for capabilities
>> root = dev->pdev->bus->self;
>>
>> +   // some architectures might not have host bridges as PCI devices
>> +   if (root == NULL)
>> +   root = dev->pdev;
>
> You didn't address my question about this.  Obviously this will avoid
> a null pointer dereference.  But you have to also explain why this
> change is correct.
>
> If it's good enough to just look at the capabilities of the DRM device
> (not the upstream bridge) on PPC and ia64, why not do that everywhere
> and forget about the bridge completely?

Yeah this doesn't make sense, we need to know if the device and the
bridge are capable of doing PCIE gen2+ speeds.

At least I'm willing to accept spec pointers to why we might not need
to ask the bridge, but my current understanding is we need to know
both.

Dave.


Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Mon, Dec 24, 2012 at 11:35 AM, Laurent Pinchart
 wrote:
> On Wednesday 19 December 2012 09:26:40 Rob Clark wrote:
>> And, there are also external HDMI encoders (for example connected over
>> i2c) that can also be shared between boards.  So I think there will be
>> a number of cases where CDF is appropriate for HDMI drivers.  Although
>> trying to keep this all independent of DRM (as opposed to just
>> something similar to what drivers/gpu/i2c is today) seems a bit
>> overkill for me.  Being able to use the helpers in drm and avoiding an
>> extra layer of translation seems like the better option to me.  So my
>> vote would be drivers/gpu/cdf.
>
> I don't think there will be any need for translation (except perhaps between
> the DRM mode structures and the common video mode structure that is being
> discussed). Add a drm_ prefix to the existing CDF functions and structures,
> and there you go :-)

well, and translation for any properties that we'd want to expose to
userspace, etc, etc.. I see there being a big potential for a lot of
needless glue

BR,
-R

> The reason why I'd like to keep CDF separate from DRM (or at least not
> requiring a drm_device) is that HDMI/DP encoders can be used by pure V4L2
> drivers.
>
>> > For DSI panels (or DSI-to-whatever bridges) it's of course another
>> > story. You typically need a panel specific driver. And here I see the
>> > main point of the whole CDF: decoupling display controllers and the
>> > panel drivers, and sharing panel (and converter chip) specific drivers
>> > across display controllers. Making it easy to write new drivers, as
>> > there would be a model to follow. I'm definitely in favour of coming up
>> > with some framework that would tackle that.
>
> --
> Regards,
>
> Laurent Pinchart
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Mon, Dec 24, 2012 at 11:27 AM, Laurent Pinchart
 wrote:
> On Wednesday 19 December 2012 16:57:56 Jani Nikula wrote:
>> It just seems to me that, at least from a DRM/KMS perspective, adding
>> another layer (=CDF) for HDMI or DP (or legacy outputs) would be
>> overengineering it. They are pretty well standardized, and I don't see there
>> would be a need to write multiple display drivers for them. Each display
>> controller has one, and can easily handle any chip specific requirements
>> right there. It's my gut feeling that an additional framework would just get
>> in the way. Perhaps there could be more common HDMI/DP helper style code in
>> DRM to reduce overlap across KMS drivers, but that's another thing.
>>
>> So is the HDMI/DP drivers using CDF a more interesting idea from a non-DRM
>> perspective? Or, put another way, is it more of an alternative to using DRM?
>> Please enlighten me if there's some real benefit here that I fail to see!
>
> As Rob pointed out, you can have external HDMI/DP encoders, and even internal
> HDMI/DP encoder IPs can be shared between SoCs and SoC vendors. CDF aims at
> sharing a single driver between SoCs and boards for a given HDMI/DP encoder.

just fwiw, drm already has something a bit like this.. the i2c
encoder-slave.  With support for a couple external i2c encoders which
could in theory be shared between devices.

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Mon, Dec 24, 2012 at 11:09 AM, Laurent Pinchart
 wrote:
> On the topic of discussions, would anyone be interested in a
> BoF/brainstorming/whatever session during the FOSDEM ?

I will be at FOSDEM.. and from http://wiki.x.org/wiki/fosdem2013 it
looks like at least Daniel will be there.  If enough others are, it
could be a good idea.

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Rob Clark
On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Tuesday 18 December 2012 00:21:32 Rob Clark wrote:
>> On Mon, Dec 17, 2012 at 11:04 PM, Dave Airlie  wrote:
>> >> Many developers showed interest in the first RFC, and I've had the
>> >> opportunity to discuss it with most of them. I would like to thank (in
>> >> no particular order) Tomi Valkeinen for all the time he spend helping me
>> >> to draft v2, Marcus Lorentzon for his useful input during Linaro Connect
>> >> Q4 2012, and Linaro for inviting me to Connect and providing a venue to
>> >> discuss this topic.
>> >
>> > So this might be a bit off topic but this whole CDF triggered me
>> > looking at stuff I generally avoid:
>> >
>> > The biggest problem I'm having currently with the whole ARM graphics
>> > and output world is the proliferation of platform drivers for every
>> > little thing. The whole ordering of operations with respect to things
>> > like suspend/resume or dynamic power management is going to be a real
>> > nightmare if there are dependencies between the drivers. How do you
>> > enforce ordering of s/r operations between all the various components?
>>
>> I tend to think that sub-devices are useful just to have a way to probe hw
>> which may or may not be there, since on ARM we often don't have any
>> alternative.. but beyond that, suspend/resume, and other life-cycle aspects,
>> they should really be treated as all one device. Especially to avoid
>> undefined suspend/resume ordering.
>
> I tend to agree, except that I try to reuse the existing PM infrastructure
> when possible to avoid reinventing the wheel. So far handling suspend/resume
> ordering related to data busses in early suspend/late resume operations and
> allowing the Linux PM core to handle control busses using the Linux device
> tree worked pretty well.
>
>> CDF or some sort of mechanism to share panel drivers between drivers is
>> useful.  Keeping it within drm, is probably a good idea, if nothing else to
>> simplify re-use of helper fxns (like avi-infoframe stuff, for example) and
>> avoid dealing with merging changes across multiple trees. Treating them more
>> like shared libraries and less like sub-devices which can be dynamically
>> loaded/unloaded (ie. they should be not built as separate modules or
>> suspend/resumed or probed/removed independently of the master driver) is a
>> really good idea to avoid uncovering nasty synchronization issues later
>> (remove vs modeset or pageflip) or surprising userspace in bad ways.
>
> We've tried that in V4L2 years ago and realized that the approach led to a
> dead-end, especially when OF/DT got involved. With DT-based device probing,
> I2C camera sensors started getting probed asynchronously to the main camera
> device, as they are children of the I2C bus master. We will have similar
> issues with I2C HDMI transmitters or panels, so we should be prepared for it.

What I've done to avoid that so far is that the master device
registers the drivers for it's output sub-devices before registering
it's own device.  At least this way I can control that they are probed
first.  Not the prettiest thing, but avoids even uglier problems.

> On PC hardware the I2C devices are connected to an I2C master provided by the
> GPU, but on embedded devices they are usually connected to an independent I2C
> master. We thus can't have a single self-contained driver that controls
> everything internally, and need to interface with the rest of the SoC drivers.
>
> I agree that probing/removing devices independently of the master driver can
> lead to bad surprises, which is why I want to establish clear rules in CDF
> regarding what can and can't be done with display entities. Reference counting
> will be one way to make sure that devices don't disappear all of a sudden.

That at least helps cover some issues.. although it doesn't really
help userspace confusion.

Anyways, with enough work perhaps all problems could be solved..
otoh, there are plenty of other important problems to solve in the
world of gpus and kms, so my preference is always not to needlessly
over-complicate CDF and instead leave some time for other things

BR,
-R

>> > The other thing I'd like you guys to do is kill the idea of fbdev and
>> > v4l drivers that are "shared" with the drm codebase, really just
>> > implement fbdev and v4l on top of the drm layer, some people might
>> > think this is some sort of maintainer thing, but really nothing else
>> > makes sense, and having these shared display frameworks just to avoid
>> > having using drm/kms drivers seems totally pointless. Fix the drm
>> > fbdev emulation if an fbdev interface is needed. But creating a fourth
>> > framework because our previous 3 frameworks didn't work out doesn't
>> > seem like a situation I want to get behind too much.
>>
>> yeah, let's not have multiple frameworks to do the same thing.. For fbdev,
>> it is pretty clear that it is a dead end.  For v4l2 (subdev+mcf), it is
>> perha

Re: Display resolutions missing on external VGA display [regressing]

2012-12-27 Thread Jani Nikula
On Mon, 24 Dec 2012, John  wrote:
> On launchpad they requested to report this bug upstream. Please let me know
> if any information is missing.

Please file a bug on DRM/Intel at [1]. Please reference your mail and
the launchpad bug, no need to attach all the info there again.

What's the latest kernel where this did work for you?

It could prove helpful if you attached the dmesg all the way from boot
with drm.debug=0xe module parameter to attaching the VGA display. The
3.8-rc1 kernel you're running is fine for starters.

Thanks,
Jani.


[1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC v2 0/5] Common Display Framework

2012-12-27 Thread Tomasz Figa
Hi Laurent,

On Monday 24 of December 2012 15:12:28 Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Friday 21 December 2012 11:00:52 Tomasz Figa wrote:
> > On Tuesday 18 of December 2012 08:31:30 Vikas Sajjan wrote:
> > > On 17 December 2012 20:55, Laurent Pinchart wrote:
> > > > Hi Vikas,
> > > > 
> > > > Sorry for the late reply. I now have more time to work on CDF, so
> > > > delays should be much shorter.
> > > > 
> > > > On Thursday 06 December 2012 10:51:15 Vikas Sajjan wrote:
> > > > > Hi Laurent,
> > > > > 
> > > > > I was thinking of porting CDF to samsung EXYNOS 5250 platform,
> > > > > what
> > > > > I found is that, the exynos display controller is MIPI DSI based
> > > > > controller.
> > > > > 
> > > > > But if I look at CDF patches, it has only support for MIPI DBI
> > > > > based
> > > > > Display controller.
> > > > > 
> > > > > So my question is, do we have any generic framework for MIPI DSI
> > > > > based display controller? basically I wanted to know, how to go
> > > > > about
> > > > > porting CDF for such kind of display controller.
> > > > 
> > > > MIPI DSI support is not available yet. The only reason for that is
> > > > that I don't have any MIPI DSI hardware to write and test the code
> > > > with :-)
> > > > 
> > > > The common display framework should definitely support MIPI DSI. I
> > > > think the existing MIPI DBI code could be used as a base, so the
> > > > implementation shouldn't be too high.
> > > > 
> > > > Yeah, i was also thinking in similar lines, below is my though for
> > > > MIPI DSI support in CDF.
> > > 
> > > o   MIPI DSI support as part of CDF framework will expose
> > > §  mipi_dsi_register_device(mpi_device) (will be called
> > > mach-xxx-dt.c
> > > file )
> > > §  mipi_dsi_register_driver(mipi_driver, bus ops) (will be called
> > > from
> > > platform specific init driver call )
> > > ·bus ops will be
> > > o   read data
> > > o   write data
> > > o   write command
> > > §  MIPI DSI will be registered as bus_register()
> > > 
> > > When MIPI DSI probe is called, it (e.g., Exynos or OMAP MIPI DSI)
> > > will
> > > initialize the MIPI DSI HW IP.
> > > 
> > > This probe will also parse the DT file for MIPI DSI based panel, add
> > > the panel device (device_add() ) to kernel and register the display
> > > entity with its control and  video ops with CDF.
> > > 
> > > I can give this a try.
> > 
> > I am currently in progress of reworking Exynos MIPI DSIM code and
> > s6e8ax0 LCD driver to use the v2 RFC of Common Display Framework. I
> > have most of the work done, I have just to solve several remaining
> > problems.
> Do you already have code that you can publish ? I'm particularly
> interested (and I think Tomi Valkeinen would be as well) in looking at
> the DSI operations you expose to DSI sinks (panels, transceivers, ...).

Well, I'm afraid this might be little below your expectations, but here's 
an initial RFC of the part defining just the DSI bus. I need a bit more 
time for patches for Exynos MIPI DSI master and s6e8ax0 LCD.

The implementation is very simple and heavily based on your MIPI DBI 
support and existing Exynos MIPI DSIM framework. Provided operation set is 
based on operation set used by Exynos s6e8ax0 LCD driver. Unfortunately 
this is my only source of information about MIPI DSI.

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

>From bad07d8bdce0ff76cbc81a9da597c0d01e5244f7 Mon Sep 17 00:00:00 2001
From: Tomasz Figa 
Date: Thu, 27 Dec 2012 12:36:15 +0100
Subject: [RFC] video: display: Add generic MIPI DSI bus

Signed-off-by: Tomasz Figa 
---
 drivers/video/display/Kconfig|   4 +
 drivers/video/display/Makefile   |   1 +
 drivers/video/display/mipi-dsi-bus.c | 214 
+++
 include/video/display.h  |   1 +
 include/video/mipi-dsi-bus.h |  98 
 5 files changed, 318 insertions(+)
 create mode 100644 drivers/video/display/mipi-dsi-bus.c
 create mode 100644 include/video/mipi-dsi-bus.h

diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
index 13b6aaf..dbaff9d 100644
--- a/drivers/video/display/Kconfig
+++ b/drivers/video/display/Kconfig
@@ -9,6 +9,10 @@ config DISPLAY_MIPI_DBI
tristate
default n
 
+config DISPLAY_MIPI_DSI
+   tristate
+   default n
+
 config DISPLAY_PANEL_DPI
tristate "DPI (Parallel) Display Panels"
---help---
diff --git a/drivers/video/display/Makefile 
b/drivers/video/display/Makefile
index 482bec7..429b3ac8 100644
--- a/drivers/video/display/Makefile
+++ b/drivers/video/display/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_DISPLAY_CORE) += display-core.o
 obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o
+obj-$(CONFIG_DISPLAY_MIPI_DSI) += mipi-dsi-bus.o
 obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o
 obj-$(CONFIG_DISPLAY_PANEL_R61505) += panel-r61505.o
 obj-$(CONFIG_DISPLAY_PANEL_R61517) += panel-r61517.o
diff --git a/drivers/video/display/mipi-d

[PATCH] drm/exynos: fimd: modify condition in fimd resume

2012-12-27 Thread Prathyush K
If fimd is runtime suspended (by DPMS OFF), fimd_suspend does not
call fimd_activate(false) and just returns. Similarily the check in
fimd_resume should not resume if previously runtime_suspended.
Instead the existing check does the opposite. So if fimd was not
runtime suspended, suspend will turn off fimd but resume will not turn
it on.  This patch fixes this issue by reversing the condition.

Signed-off-by: Prathyush K 
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bf0d9ba..9accd466 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -1046,7 +1046,7 @@ static int fimd_resume(struct device *dev)
 * of pm runtime would still be 1 so in this case, fimd driver
 * should be on directly not drawing on pm runtime interface.
 */
-   if (pm_runtime_suspended(dev)) {
+   if (!pm_runtime_suspended(dev)) {
int ret;

ret = fimd_activate(ctx, true);
-- 
1.8.0



[PATCH 4/4] drm/exynos: hdmi: support extra resolutions using drm_display_mode timings

2012-12-27 Thread Rahul Sharma
From: Sean Paul 

This patch programs the core and timing generator registers using the
timing data provided in drm_display_mode and not using hard-coded
configurations.

Additional PHY configs has been added. This allows us to support more
permissible resolutions and refresh rates.

Signed-off-by: Rahul Sharma 
Signed-off-by: Sean Paul 
Signed-off-by: Shirish S 
Signed-off-by: Akshay Saraswat 
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 1022 +-
 1 file changed, 374 insertions(+), 648 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2c46b6c..8cb42ad 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -88,6 +88,73 @@ struct hdmi_resources {
int regul_count;
 };

+struct hdmi_tg_regs {
+   u8 cmd[1];
+   u8 h_fsz[2];
+   u8 hact_st[2];
+   u8 hact_sz[2];
+   u8 v_fsz[2];
+   u8 vsync[2];
+   u8 vsync2[2];
+   u8 vact_st[2];
+   u8 vact_sz[2];
+   u8 field_chg[2];
+   u8 vact_st2[2];
+   u8 vact_st3[2];
+   u8 vact_st4[2];
+   u8 vsync_top_hdmi[2];
+   u8 vsync_bot_hdmi[2];
+   u8 field_top_hdmi[2];
+   u8 field_bot_hdmi[2];
+   u8 tg_3d[1];
+};
+
+struct hdmi_core_regs {
+   u8 h_blank[2];
+   u8 v2_blank[2];
+   u8 v1_blank[2];
+   u8 v_line[2];
+   u8 h_line[2];
+   u8 hsync_pol[1];
+   u8 vsync_pol[1];
+   u8 int_pro_mode[1];
+   u8 v_blank_f0[2];
+   u8 v_blank_f1[2];
+   u8 h_sync_start[2];
+   u8 h_sync_end[2];
+   u8 v_sync_line_bef_2[2];
+   u8 v_sync_line_bef_1[2];
+   u8 v_sync_line_aft_2[2];
+   u8 v_sync_line_aft_1[2];
+   u8 v_sync_line_aft_pxl_2[2];
+   u8 v_sync_line_aft_pxl_1[2];
+   u8 v_blank_f2[2]; /* for 3D mode */
+   u8 v_blank_f3[2]; /* for 3D mode */
+   u8 v_blank_f4[2]; /* for 3D mode */
+   u8 v_blank_f5[2]; /* for 3D mode */
+   u8 v_sync_line_aft_3[2];
+   u8 v_sync_line_aft_4[2];
+   u8 v_sync_line_aft_5[2];
+   u8 v_sync_line_aft_6[2];
+   u8 v_sync_line_aft_pxl_3[2];
+   u8 v_sync_line_aft_pxl_4[2];
+   u8 v_sync_line_aft_pxl_5[2];
+   u8 v_sync_line_aft_pxl_6[2];
+   u8 vact_space_1[2];
+   u8 vact_space_2[2];
+   u8 vact_space_3[2];
+   u8 vact_space_4[2];
+   u8 vact_space_5[2];
+   u8 vact_space_6[2];
+};
+
+struct hdmi_v14_conf {
+   int pixel_clock;
+   struct hdmi_core_regs core;
+   struct hdmi_tg_regs tg;
+   int cea_video_id;
+};
+
 struct hdmi_context {
struct device   *dev;
struct drm_device   *drm_dev;
@@ -106,6 +173,7 @@ struct hdmi_context {

/* current hdmiphy conf index */
int cur_conf;
+   struct hdmi_v14_confmode_conf;

struct hdmi_resources   res;

@@ -394,586 +462,132 @@ static const struct hdmi_v13_conf hdmi_v13_confs[] = {
 };

 /* HDMI Version 1.4 */
-static const u8 hdmiphy_conf27_027[32] = {
-   0x01, 0xd1, 0x2d, 0x72, 0x40, 0x64, 0x12, 0x08,
-   0x43, 0xa0, 0x0e, 0xd9, 0x45, 0xa0, 0xac, 0x80,
-   0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86,
-   0x54, 0xe3, 0x24, 0x00, 0x00, 0x00, 0x01, 0x00,
-};
-
-static const u8 hdmiphy_conf74_176[32] = {
-   0x01, 0xd1, 0x1f, 0x10, 0x40, 0x5b, 0xef, 0x08,
-   0x81, 0xa0, 0xb9, 0xd8, 0x45, 0xa0, 0xac, 0x80,
-   0x5a, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86,
-   0x54, 0xa6, 0x24, 0x01, 0x00, 0x00, 0x01, 0x00,
-};
-
-static const u8 hdmiphy_conf74_25[32] = {
-   0x01, 0xd1, 0x1f, 0x10, 0x40, 0x40, 0xf8, 0x08,
-   0x81, 0xa0, 0xba, 0xd8, 0x45, 0xa0, 0xac, 0x80,
-   0x3c, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86,
-   0x54, 0xa5, 0x24, 0x01, 0x00, 0x00, 0x01, 0x00,
-};
-
-static const u8 hdmiphy_conf148_5[32] = {
-   0x01, 0xd1, 0x1f, 0x00, 0x40, 0x40, 0xf8, 0x08,
-   0x81, 0xa0, 0xba, 0xd8, 0x45, 0xa0, 0xac, 0x80,
-   0x3c, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86,
-   0x54, 0x4b, 0x25, 0x03, 0x00, 0x00, 0x01, 0x00,
-};
-
-struct hdmi_tg_regs {
-   u8 cmd;
-   u8 h_fsz_l;
-   u8 h_fsz_h;
-   u8 hact_st_l;
-   u8 hact_st_h;
-   u8 hact_sz_l;
-   u8 hact_sz_h;
-   u8 v_fsz_l;
-   u8 v_fsz_h;
-   u8 vsync_l;
-   u8 vsync_h;
-   u8 vsync2_l;
-   u8 vsync2_h;
-   u8 vact_st_l;
-   u8 vact_st_h;
-   u8 vact_sz_l;
-   u8 vact_sz_h;
-   u8 field_chg_l;
-   u8 field_chg_h;
-   u8 vact_st2_l;
-   u8 vact_st2_h;
-   u8 vact_st3_l;
-   u8 vact_st3_h;
-   u8 vact_st4_l;
-   u8 vact_st4_h;
-   u8 vsync_top_hdmi_l;
-   u8 vsync_top_hdmi_h;
-   u8 vsync_bot_hdmi_l;
-   u8 vsync_bot_hdmi_h;
-   u8 field_top_hdmi_l;
-   u8 field_top_hdmi_h;
-   u8 field_bot_hdmi_l;
-   u8 field_bot_hdmi_h;
-   u8 tg_3d;
-};
-
-struct hdmi_core_regs {
-   u8 h_bl

[PATCH 3/4] drm/exynos: mixer: set correct mode for range of resolutions

2012-12-27 Thread Rahul Sharma
With this patch, mixer driver find the correct resolution mode for
the range of resolutions, upto 1080 vertical lines. Resolution will
be categorized to NTSC SD, PAL SD or HD and the correct mode is
set to the mixer configuration register.

Signed-off-by: Rahul Sharma 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 093b374..8ef0e71 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -283,13 +283,13 @@ static void mixer_cfg_scan(struct mixer_context *ctx, 
unsigned int height)
MXR_CFG_SCAN_PROGRASSIVE);

/* choosing between porper HD and SD mode */
-   if (height == 480)
+   if (height <= 480)
val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD;
-   else if (height == 576)
+   else if (height <= 576)
val |= MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD;
-   else if (height == 720)
+   else if (height <= 720)
val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
-   else if (height == 1080)
+   else if (height <= 1080)
val |= MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD;
else
val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
-- 
1.8.0



[PATCH 2/4] drm/exynos: implement display-mode-check callback in mixer driver

2012-12-27 Thread Rahul Sharma
This patch adds the implementation of check_mode callback in the mixer
driver. Based on the mixer version, correct set of restrictions will be
exposed by the mixer driver. A resolution will be acceptable only if passes
the criteria set by mixer and hdmi IPs.

Signed-off-by: Rahul Sharma 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 21db895..093b374 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -810,6 +810,33 @@ static void mixer_win_disable(void *ctx, int win)
mixer_ctx->win_data[win].enabled = false;
 }

+int mixer_check_mode(void *ctx, void *mode)
+{
+   struct mixer_context *mixer_ctx = ctx;
+   struct fb_videomode *check_mode = mode;
+   unsigned int w, h;
+
+   w = check_mode->xres;
+   h = check_mode->yres;
+
+   DRM_DEBUG_KMS("%s : xres=%d, yres=%d, refresh=%d, intl=%d\n",
+   __func__, check_mode->xres, check_mode->yres,
+   check_mode->refresh, (check_mode->vmode &
+   FB_VMODE_INTERLACED) ? true : false);
+
+   if (mixer_ctx->mxr_ver == MXR_VER_16_0_33_0) {
+   if ((w >= 464 && w <= 720 && h >= 261 && h <= 576) ||
+   (w >= 1024 && w <= 1280 &&
+   h >= 576 && h <= 720) ||
+   (w >= 1664 && w <= 1920 &&
+   h >= 936 && h <= 1080))
+   return 0;
+   } else {
+   return 0;
+   }
+
+   return -EINVAL;
+}
 static void mixer_wait_for_vblank(void *ctx)
 {
struct mixer_context *mixer_ctx = ctx;
@@ -947,6 +974,9 @@ static struct exynos_mixer_ops mixer_ops = {
.win_mode_set   = mixer_win_mode_set,
.win_commit = mixer_win_commit,
.win_disable= mixer_win_disable,
+
+   /* display */
+   .check_mode = mixer_check_mode,
 };

 /* for pageflip event */
-- 
1.8.0



[PATCH 1/4] drm/exynos: add display-mode-check operation to exynos_mixer_ops struct

2012-12-27 Thread Rahul Sharma
This patch adds the display mode check operation to exynos_mixer_ops
in drm-common-hdmi. In Exynos SoCs, mixer IP can put certain restrictions
on the proposed display modes. These restriction needs to be considered
during mode negotiation, which happens immediately after edid parsing.

Both, mixer check-mode and hdmi check-timing callbacks are called one after
another and ANDed result is returned back.

Signed-off-by: Rahul Sharma 
---
 drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 12 
 drivers/gpu/drm/exynos/exynos_drm_hdmi.h |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
index 55793c4..3a8eea6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
@@ -125,9 +125,21 @@ static int drm_hdmi_get_edid(struct device *dev,
 static int drm_hdmi_check_timing(struct device *dev, void *timing)
 {
struct drm_hdmi_context *ctx = to_context(dev);
+   int ret = 0;

DRM_DEBUG_KMS("%s\n", __FILE__);

+   /*
+   * Both, mixer and hdmi should be able to handle the requested mode.
+   * If any of the two fails, return mode as BAD.
+   */
+
+   if (mixer_ops && mixer_ops->check_mode)
+   ret = mixer_ops->check_mode(ctx->mixer_ctx->ctx, timing);
+
+   if (ret)
+   return ret;
+
if (hdmi_ops && hdmi_ops->check_timing)
return hdmi_ops->check_timing(ctx->hdmi_ctx->ctx, timing);

diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h 
b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
index 784a7e9..ae4b6ae 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
@@ -58,6 +58,9 @@ struct exynos_mixer_ops {
void (*win_mode_set)(void *ctx, struct exynos_drm_overlay *overlay);
void (*win_commit)(void *ctx, int zpos);
void (*win_disable)(void *ctx, int zpos);
+
+   /* display */
+   int (*check_mode)(void *ctx, void *mode);
 };

 void exynos_hdmi_drv_attach(struct exynos_drm_hdmi_context *ctx);
-- 
1.8.0



[PATCH 0/4] drm/exynos: add support for extra resolutions to exynos5

2012-12-27 Thread Rahul Sharma
This patch set adds support for more resolutions and refresh rates to Samsung
Exynos5 SoC series which contains hdmi transmitter (hdmi v1.4a compliant).

Given resolution will be supported or not, is decided by two factors:
1) Corresponding pixel clock is supported by hdmi PHY.
2) Mixer supports the display mode.

This patch series is based on branch "exynos-drm-next" at
http://git.kernel.org/?p=linux/kernel/git/daeinki/drm-exynos.git

Rahul Sharma (3):
  drm/exynos: add display-mode-check operation to exynos_mixer_ops
struct
  drm/exynos: implement display-mode-check callback in mixer driver
  drm/exynos: mixer: set correct mode for range of resolutions

Sean Paul (1):
  drm/exynos: hdmi: support extra resolutions using drm_display_mode
timings

 drivers/gpu/drm/exynos/exynos_drm_hdmi.c |   12 +
 drivers/gpu/drm/exynos/exynos_drm_hdmi.h |3 +
 drivers/gpu/drm/exynos/exynos_hdmi.c | 1022 +++---
 drivers/gpu/drm/exynos/exynos_mixer.c|   38 +-
 4 files changed, 423 insertions(+), 652 deletions(-)

-- 
1.8.0



[PATCH] drm/exynos: Add device tree based discovery support for Exynos G2D

2012-12-27 Thread Ajay Kumar
This patch adds device tree match table for Exynos G2D controller.

Signed-off-by: Ajay Kumar 
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 6ffa076..aa3d2e4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -1240,6 +1241,14 @@ static int g2d_resume(struct device *dev)

 static SIMPLE_DEV_PM_OPS(g2d_pm_ops, g2d_suspend, g2d_resume);

+#ifdef CONFIG_OF
+static const struct of_device_id exynos_g2d_match[] = {
+   { .compatible = "samsung,exynos-g2d-41" },
+   {},
+};
+MODULE_DEVICE_TABLE(of, exynos_g2d_match);
+#endif
+
 struct platform_driver g2d_driver = {
.probe  = g2d_probe,
.remove = __devexit_p(g2d_remove),
@@ -1247,5 +1256,6 @@ struct platform_driver g2d_driver = {
.name   = "s5p-g2d",
.owner  = THIS_MODULE,
.pm = &g2d_pm_ops,
+   .of_match_table = of_match_ptr(exynos_g2d_match),
},
 };
-- 
1.8.0



Re: [PATCH] drm/exynos: fix gem buffer allocation type checking

2012-12-27 Thread Inki Dae
2012/12/27 Prathyush K 

>
>
>
> On Thu, Dec 27, 2012 at 4:27 PM, Inki Dae  wrote:
>
>> This patch fixes gem buffer allocation type checking.
>> EXYNOS_BO_CONTIG has 0 so the checking should be fixed
>> to 'if (!(flags & EXYNOS_BO_NONCONTIG))'
>>
>> Signed-off-by: Inki Dae 
>> Signed-off-by: Kyungmin Park 
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_buf.c |2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> b/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> index 74592d1..911f7fd 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> @@ -38,7 +38,7 @@ static int lowlevel_buffer_allocate(struct drm_device
>> *dev,
>>  * region will be allocated else physically contiguous
>>  * as possible.
>>  */
>> -   if (flags & EXYNOS_BO_CONTIG)
>> +   if (!(flags & EXYNOS_BO_NONCONTIG))
>> dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &buf->dma_attrs);
>>
>>
> Hi Mr. Dae,
>
> If iommu is supported, we would have called arm_iommu_attach_device.
> So dma_alloc_attrs will always call arm_iommu_alloc_attrs.
>
> If iommu is not supported, dma_alloc_attrs will call arm_dma_alloc which
> will
> anyway allocate a contiguous buffer if possible.
>
> With this code, we are forcing the root framebuffer (fbdev) to be
> contiguous since fbdev
> allocation does not pass noncontig flag.
>
>
we could use noncontig flag simply like below,

  if (iommu is supported)
  exynos_drm_gem_create(dev, EXYNOS_BO_NONCONTIG, size);
  else
  exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG, size);



> Why do we need to force the allocation of a contiguous buffer when iommu
> is supported?
>
>
Simply saying, the reason is that we want to pass boot logo drew by boot
loader without iommu to kernel side without memory copy.


> Regards,
> Prathyush
>
>
>> /*
>> --
>> 1.7.4.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/exynos: fix gem buffer allocation type checking

2012-12-27 Thread Prathyush K
On Thu, Dec 27, 2012 at 4:27 PM, Inki Dae  wrote:

> This patch fixes gem buffer allocation type checking.
> EXYNOS_BO_CONTIG has 0 so the checking should be fixed
> to 'if (!(flags & EXYNOS_BO_NONCONTIG))'
>
> Signed-off-by: Inki Dae 
> Signed-off-by: Kyungmin Park 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_buf.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
> b/drivers/gpu/drm/exynos/exynos_drm_buf.c
> index 74592d1..911f7fd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
> @@ -38,7 +38,7 @@ static int lowlevel_buffer_allocate(struct drm_device
> *dev,
>  * region will be allocated else physically contiguous
>  * as possible.
>  */
> -   if (flags & EXYNOS_BO_CONTIG)
> +   if (!(flags & EXYNOS_BO_NONCONTIG))
> dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &buf->dma_attrs);
>
>
Hi Mr. Dae,

If iommu is supported, we would have called arm_iommu_attach_device.
So dma_alloc_attrs will always call arm_iommu_alloc_attrs.

If iommu is not supported, dma_alloc_attrs will call arm_dma_alloc which
will
anyway allocate a contiguous buffer if possible.

With this code, we are forcing the root framebuffer (fbdev) to be
contiguous since fbdev
allocation does not pass noncontig flag.

Why do we need to force the allocation of a contiguous buffer when iommu is
supported?

Regards,
Prathyush


> /*
> --
> 1.7.4.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/exynos: fimd: modify condition in fimd resume

2012-12-27 Thread Prathyush K
If fimd is runtime suspended (by DPMS OFF), fimd_suspend does not
call fimd_activate(false) and just returns. Similarily the check in
fimd_resume should not resume if previously runtime_suspended.
Instead the existing check does the opposite. So if fimd was not
runtime suspended, suspend will turn off fimd but resume will not turn
it on.  This patch fixes this issue by reversing the condition.

Signed-off-by: Prathyush K 
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bf0d9ba..9accd466 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -1046,7 +1046,7 @@ static int fimd_resume(struct device *dev)
 * of pm runtime would still be 1 so in this case, fimd driver
 * should be on directly not drawing on pm runtime interface.
 */
-   if (pm_runtime_suspended(dev)) {
+   if (!pm_runtime_suspended(dev)) {
int ret;
 
ret = fimd_activate(ctx, true);
-- 
1.8.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/exynos: fix gem buffer allocation type checking

2012-12-27 Thread Inki Dae
This patch fixes gem buffer allocation type checking.
EXYNOS_BO_CONTIG has 0 so the checking should be fixed
to 'if (!(flags & EXYNOS_BO_NONCONTIG))'

Signed-off-by: Inki Dae 
Signed-off-by: Kyungmin Park 
---
 drivers/gpu/drm/exynos/exynos_drm_buf.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c 
b/drivers/gpu/drm/exynos/exynos_drm_buf.c
index 74592d1..911f7fd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
@@ -38,7 +38,7 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
 * region will be allocated else physically contiguous
 * as possible.
 */
-   if (flags & EXYNOS_BO_CONTIG)
+   if (!(flags & EXYNOS_BO_NONCONTIG))
dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &buf->dma_attrs);
 
/*
-- 
1.7.4.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 57875] Second Life viewer bad rendering with git-ec83535

2012-12-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=57875

--- Comment #18 from Piero Finizio  ---
If i try to revert the commit e866bd1adea2c3b4971ad68e69c644752f2ab7b6  (above
mentioned workaround) with git
"HEAD at 7c35521 mesa: add missing texel fetch code for sRGB DXT formats"
the compilation ends with:
 ...
CC r300_screen.o
r300_screen.c: In function ‘r300_get_param’:
r300_screen.c:130:14: error: ‘PIPE_CAP_TIMER_QUERY’ undeclared (first use in
this function)
r300_screen.c:130:14: note: each undeclared identifier is reported only once
for each function it appears in
r300_screen.c:88:5: warning: enumeration value ‘PIPE_CAP_QUERY_TIME_ELAPSED’
not handled in switch [-Wswitch]
r300_screen.c:88:5: warning: enumeration value
‘PIPE_CAP_TEXTURE_BUFFER_OBJECTS’ not handled in switch [-Wswitch]
gmake[4]: *** [r300_screen.o] Error 1
gmake[4]: Leaving directory `/root/mesa/src/gallium/drivers/r300'
gmake[3]: *** [all-recursive] Error 1

 ...

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 04/10] drm/exynos: Use devm_clk_get in exynos_drm_fimc.c

2012-12-27 Thread Inki Dae
2012/12/26 Sachin Kamat 

>
>
> On Wednesday, 26 December 2012, Inki Dae  wrote:
> >
> >
> > 2012/12/24 Sachin Kamat 
> >>
> >> This eliminates the need for explicit clk_put and makes the
> >> cleanup and exit path code simpler.
> >>
> >> Cc: Eunchul Kim 
> >> Signed-off-by: Sachin Kamat 
> >> ---
> >>  drivers/gpu/drm/exynos/exynos_drm_fimc.c |   46
> ++---
> >>  1 files changed, 10 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> >> index 972aa70..c4006b8 100644
> >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> >> @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct
> platform_device *pdev)
> >> platform_get_device_id(pdev)->driver_data;
> >>
> >> /* clock control */
> >> -   ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
> >> +   ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc");
> >> if (IS_ERR(ctx->sclk_fimc_clk)) {
> >> dev_err(dev, "failed to get src fimc clock.\n");
> >> return PTR_ERR(ctx->sclk_fimc_clk);
> >> }
> >> clk_enable(ctx->sclk_fimc_clk);
> >>
> >> -   ctx->fimc_clk = clk_get(dev, "fimc");
> >> +   ctx->fimc_clk = devm_clk_get(dev, "fimc");
> >> if (IS_ERR(ctx->fimc_clk)) {
> >> dev_err(dev, "failed to get fimc clock.\n");
> >> clk_disable(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->sclk_fimc_clk);
> >> return PTR_ERR(ctx->fimc_clk);
> >> }
> >>
> >> -   ctx->wb_clk = clk_get(dev, "pxl_async0");
> >> +   ctx->wb_clk = devm_clk_get(dev, "pxl_async0");
> >> if (IS_ERR(ctx->wb_clk)) {
> >> dev_err(dev, "failed to get writeback a clock.\n");
> >> clk_disable(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->fimc_clk);
> >> return PTR_ERR(ctx->wb_clk);
> >> }
> >>
> >> -   ctx->wb_b_clk = clk_get(dev, "pxl_async1");
> >> +   ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1");
> >> if (IS_ERR(ctx->wb_b_clk)) {
> >> dev_err(dev, "failed to get writeback b clock.\n");
> >> clk_disable(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->fimc_clk);
> >> -   clk_put(ctx->wb_clk);
> >> return PTR_ERR(ctx->wb_b_clk);
> >> }
> >>
> >> -   parent_clk = clk_get(dev, ddata->parent_clk);
> >> +   parent_clk = devm_clk_get(dev, ddata->parent_clk);
> >>
> >> if (IS_ERR(parent_clk)) {
> >> dev_err(dev, "failed to get parent clock.\n");
> >> clk_disable(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->fimc_clk);
> >> -   clk_put(ctx->wb_clk);
> >> -   clk_put(ctx->wb_b_clk);
> >> return PTR_ERR(parent_clk);
> >> }
> >>
> >> if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) {
> >> dev_err(dev, "failed to set parent.\n");
> >> -   clk_put(parent_clk);
> >> +   devm_clk_put(dev, parent_clk);
> >
> > remove the above call. is there any reason that devm_clk_put should be
> called here?
>
> Devm resources are freed/released automatically only when the device
> detaches. In the above case the clock was released explicitly (for whatever
> reasons) earlier. Hence i have used devm call to keep the code logic same
> as i do not know the behavior if this clock is 'put' when the device
> detaches instead of here.
>
>
If probe is failed, devm resources are released by devres_release_all(). So
that is unnecessary call. Remove it.


>
> >
> >>
> >> clk_disable(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->sclk_fimc_clk);
> >> -   clk_put(ctx->fimc_clk);
> >> -   clk_put(ctx->wb_clk);
> >> -   clk_put(ctx->wb_b_clk);
> >> return -EINVAL;
> >> }
> >>
> >> -   clk_put(parent_clk);
> >> +   devm_clk_put(dev, parent_clk);
> >
> > ditto.
> >
> >>
> >> clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate);
> >>
> >> /* resource memory */
> >> @@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct
> platform_device *pdev)
> >> ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res);
> >> if (!ctx->regs) {
> >> dev_err(dev, "failed to map registers.\n");
> >> -   ret = -ENXIO;
> >> -   goto err_clk;
> >> +   return -ENXIO;
> >> }
> >>
> >> /* resource irq */
> >> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> >> if (!res) {
> >> dev_err(dev, "failed to request irq resource.\n");
> >> -  

[PATCH 04/10] drm/exynos: Use devm_clk_get in exynos_drm_fimc.c

2012-12-27 Thread Sachin Kamat
On Wednesday, 26 December 2012, Inki Dae  wrote:
>
>
> 2012/12/24 Sachin Kamat 
>>
>> This eliminates the need for explicit clk_put and makes the
>> cleanup and exit path code simpler.
>>
>> Cc: Eunchul Kim 
>> Signed-off-by: Sachin Kamat 
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_fimc.c |   46
++---
>>  1 files changed, 10 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> index 972aa70..c4006b8 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
>> platform_get_device_id(pdev)->driver_data;
>>
>> /* clock control */
>> -   ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
>> +   ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc");
>> if (IS_ERR(ctx->sclk_fimc_clk)) {
>> dev_err(dev, "failed to get src fimc clock.\n");
>> return PTR_ERR(ctx->sclk_fimc_clk);
>> }
>> clk_enable(ctx->sclk_fimc_clk);
>>
>> -   ctx->fimc_clk = clk_get(dev, "fimc");
>> +   ctx->fimc_clk = devm_clk_get(dev, "fimc");
>> if (IS_ERR(ctx->fimc_clk)) {
>> dev_err(dev, "failed to get fimc clock.\n");
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> return PTR_ERR(ctx->fimc_clk);
>> }
>>
>> -   ctx->wb_clk = clk_get(dev, "pxl_async0");
>> +   ctx->wb_clk = devm_clk_get(dev, "pxl_async0");
>> if (IS_ERR(ctx->wb_clk)) {
>> dev_err(dev, "failed to get writeback a clock.\n");
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->fimc_clk);
>> return PTR_ERR(ctx->wb_clk);
>> }
>>
>> -   ctx->wb_b_clk = clk_get(dev, "pxl_async1");
>> +   ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1");
>> if (IS_ERR(ctx->wb_b_clk)) {
>> dev_err(dev, "failed to get writeback b clock.\n");
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->fimc_clk);
>> -   clk_put(ctx->wb_clk);
>> return PTR_ERR(ctx->wb_b_clk);
>> }
>>
>> -   parent_clk = clk_get(dev, ddata->parent_clk);
>> +   parent_clk = devm_clk_get(dev, ddata->parent_clk);
>>
>> if (IS_ERR(parent_clk)) {
>> dev_err(dev, "failed to get parent clock.\n");
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->fimc_clk);
>> -   clk_put(ctx->wb_clk);
>> -   clk_put(ctx->wb_b_clk);
>> return PTR_ERR(parent_clk);
>> }
>>
>> if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) {
>> dev_err(dev, "failed to set parent.\n");
>> -   clk_put(parent_clk);
>> +   devm_clk_put(dev, parent_clk);
>
> remove the above call. is there any reason that devm_clk_put should be
called here?

Devm resources are freed/released automatically only when the device
detaches. In the above case the clock was released explicitly (for whatever
reasons) earlier. Hence i have used devm call to keep the code logic same
as i do not know the behavior if this clock is 'put' when the device
detaches instead of here.

>
>>
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->fimc_clk);
>> -   clk_put(ctx->wb_clk);
>> -   clk_put(ctx->wb_b_clk);
>> return -EINVAL;
>> }
>>
>> -   clk_put(parent_clk);
>> +   devm_clk_put(dev, parent_clk);
>
> ditto.
>
>>
>> clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate);
>>
>> /* resource memory */
>> @@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
>> ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res);
>> if (!ctx->regs) {
>> dev_err(dev, "failed to map registers.\n");
>> -   ret = -ENXIO;
>> -   goto err_clk;
>> +   return -ENXIO;
>> }
>>
>> /* resource irq */
>> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> if (!res) {
>> dev_err(dev, "failed to request irq resource.\n");
>> -   ret = -ENOENT;
>> -   goto err_clk;
>> +   return -ENOENT;
>> }
>>
>> ctx->irq = res->start;
>> @@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
>> IRQF_ONESHOT, "drm_fimc", ctx);
>> if (ret < 0) {
>> dev_err(dev, "failed to request irq.\n");
>> -   goto err_clk;

Re: [PATCH 04/10] drm/exynos: Use devm_clk_get in exynos_drm_fimc.c

2012-12-27 Thread Sachin Kamat
On Wednesday, 26 December 2012, Inki Dae  wrote:
>
>
> 2012/12/24 Sachin Kamat 
>>
>> This eliminates the need for explicit clk_put and makes the
>> cleanup and exit path code simpler.
>>
>> Cc: Eunchul Kim 
>> Signed-off-by: Sachin Kamat 
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_fimc.c |   46
++---
>>  1 files changed, 10 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> index 972aa70..c4006b8 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
>> platform_get_device_id(pdev)->driver_data;
>>
>> /* clock control */
>> -   ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
>> +   ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc");
>> if (IS_ERR(ctx->sclk_fimc_clk)) {
>> dev_err(dev, "failed to get src fimc clock.\n");
>> return PTR_ERR(ctx->sclk_fimc_clk);
>> }
>> clk_enable(ctx->sclk_fimc_clk);
>>
>> -   ctx->fimc_clk = clk_get(dev, "fimc");
>> +   ctx->fimc_clk = devm_clk_get(dev, "fimc");
>> if (IS_ERR(ctx->fimc_clk)) {
>> dev_err(dev, "failed to get fimc clock.\n");
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> return PTR_ERR(ctx->fimc_clk);
>> }
>>
>> -   ctx->wb_clk = clk_get(dev, "pxl_async0");
>> +   ctx->wb_clk = devm_clk_get(dev, "pxl_async0");
>> if (IS_ERR(ctx->wb_clk)) {
>> dev_err(dev, "failed to get writeback a clock.\n");
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->fimc_clk);
>> return PTR_ERR(ctx->wb_clk);
>> }
>>
>> -   ctx->wb_b_clk = clk_get(dev, "pxl_async1");
>> +   ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1");
>> if (IS_ERR(ctx->wb_b_clk)) {
>> dev_err(dev, "failed to get writeback b clock.\n");
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->fimc_clk);
>> -   clk_put(ctx->wb_clk);
>> return PTR_ERR(ctx->wb_b_clk);
>> }
>>
>> -   parent_clk = clk_get(dev, ddata->parent_clk);
>> +   parent_clk = devm_clk_get(dev, ddata->parent_clk);
>>
>> if (IS_ERR(parent_clk)) {
>> dev_err(dev, "failed to get parent clock.\n");
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->fimc_clk);
>> -   clk_put(ctx->wb_clk);
>> -   clk_put(ctx->wb_b_clk);
>> return PTR_ERR(parent_clk);
>> }
>>
>> if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) {
>> dev_err(dev, "failed to set parent.\n");
>> -   clk_put(parent_clk);
>> +   devm_clk_put(dev, parent_clk);
>
> remove the above call. is there any reason that devm_clk_put should be
called here?

Devm resources are freed/released automatically only when the device
detaches. In the above case the clock was released explicitly (for whatever
reasons) earlier. Hence i have used devm call to keep the code logic same
as i do not know the behavior if this clock is 'put' when the device
detaches instead of here.

>
>>
>> clk_disable(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->sclk_fimc_clk);
>> -   clk_put(ctx->fimc_clk);
>> -   clk_put(ctx->wb_clk);
>> -   clk_put(ctx->wb_b_clk);
>> return -EINVAL;
>> }
>>
>> -   clk_put(parent_clk);
>> +   devm_clk_put(dev, parent_clk);
>
> ditto.
>
>>
>> clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate);
>>
>> /* resource memory */
>> @@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
>> ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res);
>> if (!ctx->regs) {
>> dev_err(dev, "failed to map registers.\n");
>> -   ret = -ENXIO;
>> -   goto err_clk;
>> +   return -ENXIO;
>> }
>>
>> /* resource irq */
>> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> if (!res) {
>> dev_err(dev, "failed to request irq resource.\n");
>> -   ret = -ENOENT;
>> -   goto err_clk;
>> +   return -ENOENT;
>> }
>>
>> ctx->irq = res->start;
>> @@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
>> IRQF_ONESHOT, "drm_fimc", ctx);
>> if (ret < 0) {
>> dev_err(dev, "failed to request irq.\n");
>> -   goto err_clk;