On Mon, Nov 14, 2011 at 1:03 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> On Fri, 2011-11-11 at 18:39 +0530, mythripk@xxxxxx wrote:
>> From: Mythri P K <mythripk@xxxxxx>
>>
>> Change the timing match logic, Instead of the statically mapped method
>> to get the corresponding timings for a given code and mode, move to a
>> simpler array indexed method. It will help to scale up to add more
>> timings when needed.
>>
>> Signed-off-by: Mythri P K <mythripk@xxxxxx>
>> ---
>> drivers/video/omap2/dss/hdmi.c | 162 ++++++++++++++++-----------------------
>> 1 files changed, 67 insertions(+), 95 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
>> index f76ae47..b859350 100644
>> --- a/drivers/video/omap2/dss/hdmi.c
>> +++ b/drivers/video/omap2/dss/hdmi.c
>> @@ -58,8 +58,6 @@
>> #define EDID_SIZE_BLOCK0_TIMING_DESCRIPTOR 4
>> #define EDID_SIZE_BLOCK1_TIMING_DESCRIPTOR 4
>>
>> -#define OMAP_HDMI_TIMINGS_NB 34
>> -
>> #define HDMI_DEFAULT_REGN 16
>> #define HDMI_DEFAULT_REGM2 1
>>
>> @@ -88,7 +86,7 @@ static struct {
>> * map it to corresponding CEA or VESA index.
>> */
>>
>> -static const struct hdmi_config cea_vesa_timings[OMAP_HDMI_TIMINGS_NB] = {
>> +static const struct hdmi_config cea_timings[] = {
>> { {640, 480, 25200, 96, 16, 48, 2, 10, 33, 0, 0, 0}, {1, HDMI_HDMI} },
>> { {720, 480, 27027, 62, 16, 60, 6, 9, 30, 0, 0, 0}, {2, HDMI_HDMI} },
>> { {1280, 720, 74250, 40, 110, 220, 5, 5, 20, 1, 1, 0}, {4, HDMI_HDMI} },
>> @@ -104,6 +102,8 @@ static const struct hdmi_config cea_vesa_timings[OMAP_HDMI_TIMINGS_NB] = {
>> { {1920, 1080, 74250, 44, 638, 148, 5, 4, 36, 1, 1, 0}, {32, HDMI_HDMI} },
>> { {2880, 480, 108108, 248, 64, 240, 6, 9, 30, 0, 0, 0}, {35, HDMI_HDMI} },
>> { {2880, 576, 108000, 256, 48, 272, 5, 5, 39, 0, 0, 0}, {37, HDMI_HDMI} },
>> +};
>> +static const struct hdmi_config vesa_timings[] = {
>> /* VESA From Here */
>> { {640, 480, 25175, 96, 16, 48, 2 , 11, 31, 0, 0, 0}, {4, HDMI_DVI} },
>> { {800, 600, 40000, 128, 40, 88, 4 , 1, 23, 1, 1, 0}, {9, HDMI_DVI} },
>> @@ -126,39 +126,6 @@ static const struct hdmi_config cea_vesa_timings[OMAP_HDMI_TIMINGS_NB] = {
>> { {1280, 720, 74250, 40, 110, 220, 5, 5, 20, 1, 1, 0}, {0x55, HDMI_DVI} }
>> };
>>
>> -/*
>> - * This is a static mapping array which maps the timing values
>> - * with corresponding CEA / VESA code
>> - */
>> -static const int code_index[OMAP_HDMI_TIMINGS_NB] = {
>> - 1, 19, 4, 2, 37, 6, 21, 20, 5, 16, 17, 29, 31, 35, 32,
>> - /* <--15 CEA 17--> vesa*/
>> - 4, 9, 0xE, 0x17, 0x1C, 0x27, 0x20, 0x23, 0x10, 0x2A,
>> - 0X2F, 0x3A, 0X51, 0X52, 0x16, 0x29, 0x39, 0x1B
>> -};
>> -
>> -/*
>> - * This is reverse static mapping which maps the CEA / VESA code
>> - * to the corresponding timing values
>> - */
>> -static const int code_cea[39] = {
>> - -1, 0, 3, 3, 2, 8, 5, 5, -1, -1,
>> - -1, -1, -1, -1, -1, -1, 9, 10, 10, 1,
>> - 7, 6, 6, -1, -1, -1, -1, -1, -1, 11,
>> - 11, 12, 14, -1, -1, 13, 13, 4, 4
>> -};
>> -
>> -static const int code_vesa[85] = {
>> - -1, -1, -1, -1, 15, -1, -1, -1, -1, 16,
>> - -1, -1, -1, -1, 17, -1, 23, -1, -1, -1,
>> - -1, -1, 29, 18, -1, -1, -1, 32, 19, -1,
>> - -1, -1, 21, -1, -1, 22, -1, -1, -1, 20,
>> - -1, 30, 24, -1, -1, -1, -1, 25, -1, -1,
>> - -1, -1, -1, -1, -1, -1, -1, 31, 26, -1,
>> - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
>> - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
>> - -1, 27, 28, -1, 33};
>> -
>> static int hdmi_runtime_get(void)
>> {
>> int r;
>> @@ -188,82 +155,88 @@ int hdmi_init_display(struct omap_dss_device *dssdev)
>> return 0;
>> }
>>
>> -static int get_timings_index(void)
>> +static bool hdmi_find_code(const struct hdmi_config *timings_arr,
>> + int len, struct hdmi_config *timing)
>> {
>> - int code;
>> + int i;
>>
>> - if (hdmi.mode == 0)
>> - code = code_vesa[hdmi.code];
>> - else
>> - code = code_cea[hdmi.code];
>> + for (i = 0; i < len; i++) {
>> + if (timings_arr[i].cm.code == hdmi.code) {
>> + *timing = timings_arr[i];
>> + return true;
>> + }
>> + }
>> +
>> + return false;
>> +}
>
> You could return the hdmi_config pointer instead of making a copy and
> returning a bool.
In this function i'm passing the timing value and finally there needs
to be one copy whether it is in this function or after the return,
because the timings array is a const and dssdev->paneltimings and
config timings are not, so do you see any benefit of doing that later
or suggest any other method?
>
> You shouldn't use hdmi.code in this function, but get the code as a
> parameter.
>
>>
>> - if (code == -1) {
>> +static void hdmi_get_timings(struct hdmi_config *timing)
>> +{
>> + int r;
>> +
>> + if (hdmi.mode == 0) {
>> + r = hdmi_find_code(vesa_timings,
>> + ARRAY_SIZE(vesa_timings), timing);
>> + } else {
>> + r = hdmi_find_code(cea_timings,
>> + ARRAY_SIZE(cea_timings), timing);
>> + }
>> + if (!r) {
>> /* HDMI code 4 corresponds to 640 * 480 VGA */
>> hdmi.code = 4;
>> /* DVI mode 1 corresponds to HDMI 0 to DVI */
>> hdmi.mode = HDMI_DVI;
>> + *timing = vesa_timings[0];
>> + }
>> +}
>
> Same thing here, you could just return the pointer to hdmi_config.
>
> And also for this function you shouldn't use hdmi.mode and hdmi.code,
> but they should be parameters to this function.
>
In the next patch i remove the hdmi.code and mode and use the config instead.
> And the code in the error path shouldn't modify hdmi.code or hdmi.mode,
> it should just return NULL, and the caller would then do what it wants,
> in this case use VGA as a fallback.
ok.
>
>> +
>> +static bool hdmi_timings_compare(struct omap_video_timings *timing,
>> + const struct hdmi_video_timings *temp)
>
> Don't call the second argument "temp". It's not a temporary variable.
> Naming the parameters timing1 and timing2 would make much more sense.
>
ok.
Thanks and regards,
Mythri.
> Tomi
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
