2017년 01월 05일 19:35에 Andrzej Hajda 이(가) 쓴 글:
> On 05.01.2017 11:19, Inki Dae wrote:
>>
>> 2017년 01월 05일 19:15에 Andrzej Hajda 이(가) 쓴 글:
>>> On 05.01.2017 11:05, Inki Dae wrote:
>>>> 2017년 01월 02일 17:39에 Andrzej Hajda 이(가) 쓴 글:
>>>>> In case of HW trigger mode, sysreg register should be configured to
>>>>> enable TE functionality. The patch refactors also trigger setup function.
>>>>>
>>>>> Signed-off-by: Andrzej Hajda <a.hajda at samsung.com>
>>>>> ---
>>>>> v2: fixed bitop operator (thanks Ilia)
>>>>> ---
>>>>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 40 
>>>>> +++++++++++++++++++++------
>>>>>  1 file changed, 32 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c 
>>>>> b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>>>> index 6ca1f31..b63b8e6 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>>>> @@ -13,9 +13,11 @@
>>>>>  #include <linux/platform_device.h>
>>>>>  #include <linux/clk.h>
>>>>>  #include <linux/component.h>
>>>>> +#include <linux/mfd/syscon.h>
>>>>>  #include <linux/of_device.h>
>>>>>  #include <linux/of_gpio.h>
>>>>>  #include <linux/pm_runtime.h>
>>>>> +#include <linux/regmap.h>
>>>>>  
>>>>>  #include <video/exynos5433_decon.h>
>>>>>  
>>>>> @@ -25,6 +27,9 @@
>>>>>  #include "exynos_drm_plane.h"
>>>>>  #include "exynos_drm_iommu.h"
>>>>>  
>>>>> +#define DSD_CFG_MUX 0x1004
>>>>> +#define DSD_CFG_MUX_TE_UNMASK_GLOBAL BIT(13)
>>>>> +
>>>>>  #define WINDOWS_NR       3
>>>>>  #define MIN_FB_WIDTH_FOR_16WORD_BURST    128
>>>>>  
>>>>> @@ -56,6 +61,7 @@ struct decon_context {
>>>>>   struct exynos_drm_plane         planes[WINDOWS_NR];
>>>>>   struct exynos_drm_plane_config  configs[WINDOWS_NR];
>>>>>   void __iomem                    *addr;
>>>>> + struct regmap                   *sysreg;
>>>>>   struct clk                      *clks[ARRAY_SIZE(decon_clks_name)];
>>>>>   int                             pipe;
>>>>>   unsigned long                   flags;
>>>>> @@ -117,12 +123,22 @@ static void decon_disable_vblank(struct 
>>>>> exynos_drm_crtc *crtc)
>>>>>  
>>>>>  static void decon_setup_trigger(struct decon_context *ctx)
>>>>>  {
>>>>> - u32 val = !(ctx->out_type & I80_HW_TRG)
>>>>> -         ? TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
>>>>> -           TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
>>>>> -         : TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
>>>>> -           TRIGCON_HWTRIGMASK | TRIGCON_HWTRIGEN;
>>>>> - writel(val, ctx->addr + DECON_TRIGCON);
>>>>> + if (!(ctx->out_type & (IFTYPE_I80 | I80_HW_TRG)))
>>>>> +         return;
>>>>> +
>>>>> + if (!(ctx->out_type & I80_HW_TRG)) {
>>>>> +         writel(TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
>>>>> +                | TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN,
>>>>> +                ctx->addr + DECON_TRIGCON);
>>>>> +         return;
>>>>> + }
>>>>> +
>>>>> + writel(TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F | TRIGCON_HWTRIGMASK
>>>>> +        | TRIGCON_HWTRIGEN, ctx->addr + DECON_TRIGCON);
>>>>> +
>>>>> + if (regmap_update_bits(ctx->sysreg, DSD_CFG_MUX,
>>>>> +                        DSD_CFG_MUX_TE_UNMASK_GLOBAL, ~0))
>>>>> +         DRM_ERROR("Cannot update sysreg.\n");
>>>>>  }
>>>>>  
>>>>>  static void decon_commit(struct exynos_drm_crtc *crtc)
>>>>> @@ -147,8 +163,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>>>>>   val = CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F;
>>>>>   writel(val, ctx->addr + DECON_CMU);
>>>>>  
>>>>> - if (ctx->out_type & (IFTYPE_I80 | I80_HW_TRG))
>>>>> -         decon_setup_trigger(ctx);
>>>>> + decon_setup_trigger(ctx);
>>>> Calling this function in video mode is unnecessary. Why did you remove 
>>>> above condition?
>>> I have moved the condition to decon_setup_trigger.
>> In video mode, it doesn't need to even call decon_setup_trigger function 
>> even though this function is just returned.
>> This is really trivial but it'd better not to call this function so seems no 
>> reason to modify.
> 
> The function is inlined anyway so it does not matter from efficiency
> point of view. The good point in this change is that it puts trigger
> related code into one function making decon_commit little bit less messy.

I don't see how much this change mitigates making decon_commit to be messy 
because already same codes are here and there.
Anyway really trivial thing. Will merge it. Stubborn. :)

> 
> Regards
> Andrzej
> 
> 
>>
>>> Regards
>>> Andrzej
>>>
>>>>>  
>>>>>   /* lcd on and use command if */
>>>>>   val = VIDOUT_LCD_ON;
>>>>> @@ -640,6 +655,15 @@ static int exynos5433_decon_probe(struct 
>>>>> platform_device *pdev)
>>>>>           ctx->out_type |= IFTYPE_I80;
>>>>>   }
>>>>>  
>>>>> + if (ctx->out_type & I80_HW_TRG) {
>>>>> +         ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
>>>>> +                                                 "samsung,disp-sysreg");
>>>>> +         if (IS_ERR(ctx->sysreg)) {
>>>>> +                 dev_err(dev, "failed to get system register\n");
>>>>> +                 return PTR_ERR(ctx->sysreg);
>>>>> +         }
>>>>> + }
>>>>> +
>>>>>   for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
>>>>>           struct clk *clk;
>>>>>  
>>>>>
>>>
>>>
>>
> 
> 
> 

Reply via email to