linux-kernel - Re: [PATCH 1/1] drm/tegra: Add guard to avoid double disable/enable of RGB outputs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52FA7EA0.6050509@gmail.com>
Date:	Tue, 11 Feb 2014 23:48:48 +0400
From:	Dmitry Osipenko <digetx@...il.com>
To:	kusmabite@...il.com
CC:	thierry.reding@...il.com, tbergstrom@...dia.com, airlied@...ux.ie,
	swarren@...dotorg.org, dri-devel@...ts.freedesktop.org,
	linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] drm/tegra: Add guard to avoid double disable/enable
 of RGB outputs

11.02.2014 23:13, Erik Faye-Lund пишет:
> On Tue, Feb 11, 2014 at 6:12 PM, Dmitry Osipenko <digetx@...il.com> wrote:
>> Add guard to check whether RGB output is already enabled in the way it's
>> done for HDMI output. Fixes possible hang on trying to disable output twice
>> (first time during driver probe and second on fb registering).
>>
>> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
>> ---
>>  drivers/gpu/drm/tegra/rgb.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
>> index 338f7f6..0266fb4 100644
>> --- a/drivers/gpu/drm/tegra/rgb.c
>> +++ b/drivers/gpu/drm/tegra/rgb.c
>> @@ -15,6 +15,7 @@
>>  struct tegra_rgb {
>>         struct tegra_output output;
>>         struct tegra_dc *dc;
>> +       bool enabled;
>>
>>         struct clk *clk_parent;
>>         struct clk *clk;
>> @@ -89,6 +90,9 @@ static int tegra_output_rgb_enable(struct tegra_output *output)
>>         struct tegra_rgb *rgb = to_rgb(output);
>>         unsigned long value;
>>
>> +       if (rgb->enabled)
>> +               return 0;
>> +
>>         tegra_dc_write_regs(rgb->dc, rgb_enable, ARRAY_SIZE(rgb_enable));
>>
>>         value = DE_SELECT_ACTIVE | DE_CONTROL_NORMAL;
>> @@ -122,6 +126,8 @@ static int tegra_output_rgb_enable(struct tegra_output *output)
>>         tegra_dc_writel(rgb->dc, GENERAL_ACT_REQ << 8, DC_CMD_STATE_CONTROL);
>>         tegra_dc_writel(rgb->dc, GENERAL_ACT_REQ, DC_CMD_STATE_CONTROL);
>>
>> +       rgb->enabled = true;
>> +
>>         return 0;
>>  }
>>
>> @@ -130,6 +136,9 @@ static int tegra_output_rgb_disable(struct tegra_output *output)
>>         struct tegra_rgb *rgb = to_rgb(output);
>>         unsigned long value;
>>
>> +       if (!rgb->enabled)
>> +               return 0;
>> +
>>         value = tegra_dc_readl(rgb->dc, DC_CMD_DISPLAY_POWER_CONTROL);
>>         value &= ~(PW0_ENABLE | PW1_ENABLE | PW2_ENABLE | PW3_ENABLE |
>>                    PW4_ENABLE | PM0_ENABLE | PM1_ENABLE);
>> @@ -144,6 +153,8 @@ static int tegra_output_rgb_disable(struct tegra_output *output)
>>
>>         tegra_dc_write_regs(rgb->dc, rgb_disable, ARRAY_SIZE(rgb_disable));
>>
>> +       rgb->enabled = false;
>> +
>>         return 0;
>>  }
>>
> 
> Wouldn't it make more sense to make "enabled" and int that counts how
> many times tegra_output_rgb_enable has been called? That way you can
> have tegra_output_rgb_disable only really disable the display once the
> same amount of disables have been performed...
> 
RGB is a part of DC, i.e. one per DC. tegra_output_rgb_enable() would be called
for each individual DC, so it doesn't make any sense. Besides, I doubt tegra
device with two panels ever existed or will be :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/