* [PATCH 1/9] drm/i915: Shrink cxsr_latency_table
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-10 7:15 ` Joonas Lahtinen
2016-10-07 13:34 ` [PATCH 2/9] drm/i915: Shrink sdvo_cmd_names Tvrtko Ursulin
` (9 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
unsigned long is too wide - use smaller types in
struct cxsr_latency to save 800-something bytes of .rodata.
v2: All data even fits in u16 for even more saving. (Ville Syrjala)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 16 ++++++++--------
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f48e79ae2ac6..f618a1c4b9a3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -807,14 +807,14 @@ struct intel_watermark_params {
};
struct cxsr_latency {
- int is_desktop;
- int is_ddr3;
- unsigned long fsb_freq;
- unsigned long mem_freq;
- unsigned long display_sr;
- unsigned long display_hpll_disable;
- unsigned long cursor_sr;
- unsigned long cursor_hpll_disable;
+ bool is_desktop : 1;
+ bool is_ddr3 : 1;
+ u16 fsb_freq;
+ u16 mem_freq;
+ u16 display_sr;
+ u16 display_hpll_disable;
+ u16 cursor_sr;
+ u16 cursor_hpll_disable;
};
#define to_intel_atomic_state(x) container_of(x, struct intel_atomic_state, base)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7f1748a1e614..3b1f0b40ccb9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -252,8 +252,8 @@ static const struct cxsr_latency cxsr_latency_table[] = {
{0, 1, 400, 800, 6042, 36042, 6584, 36584}, /* DDR3-800 SC */
};
-static const struct cxsr_latency *intel_get_cxsr_latency(int is_desktop,
- int is_ddr3,
+static const struct cxsr_latency *intel_get_cxsr_latency(bool is_desktop,
+ bool is_ddr3,
int fsb,
int mem)
{
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 1/9] drm/i915: Shrink cxsr_latency_table
2016-10-07 13:34 ` [PATCH 1/9] drm/i915: Shrink cxsr_latency_table Tvrtko Ursulin
@ 2016-10-10 7:15 ` Joonas Lahtinen
0 siblings, 0 replies; 28+ messages in thread
From: Joonas Lahtinen @ 2016-10-10 7:15 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote:
> @@ -807,14 +807,14 @@ struct intel_watermark_params {
>
> };
>
> struct cxsr_latency {
> - int is_desktop;
> - int is_ddr3;
> - unsigned long fsb_freq;
> - unsigned long mem_freq;
> - unsigned long display_sr;
> - unsigned long display_hpll_disable;
> - unsigned long cursor_sr;
> - unsigned long cursor_hpll_disable;
> + bool is_desktop : 1;
> + bool is_ddr3 : 1;
bitfields to the end of struct, with that;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/9] drm/i915: Shrink sdvo_cmd_names
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
2016-10-07 13:34 ` [PATCH 1/9] drm/i915: Shrink cxsr_latency_table Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-10 7:16 ` Joonas Lahtinen
2016-10-07 13:34 ` [PATCH 3/9] drm/i915: Shrink per-platform watermark configuration Tvrtko Ursulin
` (8 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Pack the struct _sdvo_cmd_name to save 736 bytes of .rodata.
This is fine since the name pointers are used only for debug.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_sdvo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index a061b0029797..9f352aac9526 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -307,7 +307,7 @@ static bool intel_sdvo_read_byte(struct intel_sdvo *intel_sdvo, u8 addr, u8 *ch)
static const struct _sdvo_cmd_name {
u8 cmd;
const char *name;
-} sdvo_cmd_names[] = {
+} __attribute__ ((packed)) sdvo_cmd_names[] = {
SDVO_CMD_NAME_ENTRY(SDVO_CMD_RESET),
SDVO_CMD_NAME_ENTRY(SDVO_CMD_GET_DEVICE_CAPS),
SDVO_CMD_NAME_ENTRY(SDVO_CMD_GET_FIRMWARE_REV),
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 3/9] drm/i915: Shrink per-platform watermark configuration
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
2016-10-07 13:34 ` [PATCH 1/9] drm/i915: Shrink cxsr_latency_table Tvrtko Ursulin
2016-10-07 13:34 ` [PATCH 2/9] drm/i915: Shrink sdvo_cmd_names Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-10 7:17 ` Joonas Lahtinen
2016-10-07 13:34 ` [PATCH 4/9] drm/i915: Shrink TV modes const data Tvrtko Ursulin
` (7 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Use types of more appropriate size in struct
intel_watermark_params to save 512 bytes of .rodata.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 10 +++++-----
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f618a1c4b9a3..2ab417c73ad8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -799,11 +799,11 @@ struct intel_plane {
};
struct intel_watermark_params {
- unsigned long fifo_size;
- unsigned long max_wm;
- unsigned long default_wm;
- unsigned long guard_size;
- unsigned long cacheline_size;
+ u16 fifo_size;
+ u16 max_wm;
+ u8 default_wm;
+ u8 guard_size;
+ u8 cacheline_size;
};
struct cxsr_latency {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3b1f0b40ccb9..96d0c57c816c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -775,13 +775,13 @@ static bool g4x_check_srwm(struct drm_device *dev,
display_wm, cursor_wm);
if (display_wm > display->max_wm) {
- DRM_DEBUG_KMS("display watermark is too large(%d/%ld), disabling\n",
+ DRM_DEBUG_KMS("display watermark is too large(%d/%u), disabling\n",
display_wm, display->max_wm);
return false;
}
if (cursor_wm > cursor->max_wm) {
- DRM_DEBUG_KMS("cursor watermark is too large(%d/%ld), disabling\n",
+ DRM_DEBUG_KMS("cursor watermark is too large(%d/%u), disabling\n",
cursor_wm, cursor->max_wm);
return false;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 4/9] drm/i915: Shrink TV modes const data
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
` (2 preceding siblings ...)
2016-10-07 13:34 ` [PATCH 3/9] drm/i915: Shrink per-platform watermark configuration Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-10 6:49 ` Jani Nikula
2016-10-10 7:22 ` Joonas Lahtinen
2016-10-07 13:34 ` [PATCH 5/9] drm/i915: Use unsigned int for latencies Tvrtko Ursulin
` (6 subsequent siblings)
10 siblings, 2 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Make struct video_levels and struct tv_mode use data types
of sufficient width to save approximately one kilobyte in
the .rodata section.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_tv.c | 50 ++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 3988c45f9e5f..fd4d59341897 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -86,7 +86,8 @@ struct intel_tv {
};
struct video_levels {
- int blank, black, burst;
+ u16 blank, black;
+ u8 burst;
};
struct color_conversion {
@@ -339,34 +340,43 @@ static const struct video_levels component_levels = {
struct tv_mode {
const char *name;
- int clock;
- int refresh; /* in millihertz (for precision) */
- u32 oversample;
- int hsync_end, hblank_start, hblank_end, htotal;
- bool progressive, trilevel_sync, component_only;
- int vsync_start_f1, vsync_start_f2, vsync_len;
- bool veq_ena;
- int veq_start_f1, veq_start_f2, veq_len;
- int vi_end_f1, vi_end_f2, nbr_end;
- bool burst_ena;
- int hburst_start, hburst_len;
- int vburst_start_f1, vburst_end_f1;
- int vburst_start_f2, vburst_end_f2;
- int vburst_start_f3, vburst_end_f3;
- int vburst_start_f4, vburst_end_f4;
+
+ u32 clock;
+ u16 refresh; /* in millihertz (for precision) */
+ u32 oversample;
+ u8 hsync_end;
+ u16 hblank_start, hblank_end, htotal;
+ bool progressive : 1, trilevel_sync : 1, component_only : 1;
+ u8 vsync_start_f1, vsync_start_f2, vsync_len;
+ bool veq_ena : 1;
+ u8 veq_start_f1, veq_start_f2, veq_len;
+ u8 vi_end_f1, vi_end_f2;
+ u16 nbr_end;
+ bool burst_ena : 1;
+ u8 hburst_start, hburst_len;
+ u8 vburst_start_f1;
+ u16 vburst_end_f1;
+ u8 vburst_start_f2;
+ u16 vburst_end_f2;
+ u8 vburst_start_f3;
+ u16 vburst_end_f3;
+ u8 vburst_start_f4;
+ u16 vburst_end_f4;
/*
* subcarrier programming
*/
- int dda2_size, dda3_size, dda1_inc, dda2_inc, dda3_inc;
- u32 sc_reset;
- bool pal_burst;
+ u16 dda2_size, dda3_size;
+ u8 dda1_inc;
+ u16 dda2_inc, dda3_inc;
+ u32 sc_reset;
+ bool pal_burst : 1;
/*
* blank/black levels
*/
const struct video_levels *composite_levels, *svideo_levels;
const struct color_conversion *composite_color, *svideo_color;
const u32 *filter_table;
- int max_srcw;
+ u16 max_srcw;
};
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 4/9] drm/i915: Shrink TV modes const data
2016-10-07 13:34 ` [PATCH 4/9] drm/i915: Shrink TV modes const data Tvrtko Ursulin
@ 2016-10-10 6:49 ` Jani Nikula
2016-10-10 8:38 ` Tvrtko Ursulin
2016-10-10 7:22 ` Joonas Lahtinen
1 sibling, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2016-10-10 6:49 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On Fri, 07 Oct 2016, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Make struct video_levels and struct tv_mode use data types
> of sufficient width to save approximately one kilobyte in
> the .rodata section.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/intel_tv.c | 50 ++++++++++++++++++++++++-----------------
> 1 file changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 3988c45f9e5f..fd4d59341897 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -86,7 +86,8 @@ struct intel_tv {
> };
>
> struct video_levels {
> - int blank, black, burst;
> + u16 blank, black;
> + u8 burst;
> };
>
> struct color_conversion {
> @@ -339,34 +340,43 @@ static const struct video_levels component_levels = {
>
> struct tv_mode {
> const char *name;
> - int clock;
> - int refresh; /* in millihertz (for precision) */
> - u32 oversample;
> - int hsync_end, hblank_start, hblank_end, htotal;
> - bool progressive, trilevel_sync, component_only;
> - int vsync_start_f1, vsync_start_f2, vsync_len;
> - bool veq_ena;
> - int veq_start_f1, veq_start_f2, veq_len;
> - int vi_end_f1, vi_end_f2, nbr_end;
> - bool burst_ena;
> - int hburst_start, hburst_len;
> - int vburst_start_f1, vburst_end_f1;
> - int vburst_start_f2, vburst_end_f2;
> - int vburst_start_f3, vburst_end_f3;
> - int vburst_start_f4, vburst_end_f4;
> +
> + u32 clock;
> + u16 refresh; /* in millihertz (for precision) */
> + u32 oversample;
> + u8 hsync_end;
> + u16 hblank_start, hblank_end, htotal;
> + bool progressive : 1, trilevel_sync : 1, component_only : 1;
> + u8 vsync_start_f1, vsync_start_f2, vsync_len;
> + bool veq_ena : 1;
> + u8 veq_start_f1, veq_start_f2, veq_len;
> + u8 vi_end_f1, vi_end_f2;
> + u16 nbr_end;
> + bool burst_ena : 1;
> + u8 hburst_start, hburst_len;
> + u8 vburst_start_f1;
> + u16 vburst_end_f1;
> + u8 vburst_start_f2;
> + u16 vburst_end_f2;
> + u8 vburst_start_f3;
> + u16 vburst_end_f3;
> + u8 vburst_start_f4;
> + u16 vburst_end_f4;
Not convinced about the indentation change.
BR,
Jani.
> /*
> * subcarrier programming
> */
> - int dda2_size, dda3_size, dda1_inc, dda2_inc, dda3_inc;
> - u32 sc_reset;
> - bool pal_burst;
> + u16 dda2_size, dda3_size;
> + u8 dda1_inc;
> + u16 dda2_inc, dda3_inc;
> + u32 sc_reset;
> + bool pal_burst : 1;
> /*
> * blank/black levels
> */
> const struct video_levels *composite_levels, *svideo_levels;
> const struct color_conversion *composite_color, *svideo_color;
> const u32 *filter_table;
> - int max_srcw;
> + u16 max_srcw;
> };
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 4/9] drm/i915: Shrink TV modes const data
2016-10-10 6:49 ` Jani Nikula
@ 2016-10-10 8:38 ` Tvrtko Ursulin
0 siblings, 0 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-10 8:38 UTC (permalink / raw)
To: Jani Nikula, Tvrtko Ursulin, Intel-gfx
On 10/10/2016 07:49, Jani Nikula wrote:
> On Fri, 07 Oct 2016, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Make struct video_levels and struct tv_mode use data types
>> of sufficient width to save approximately one kilobyte in
>> the .rodata section.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_tv.c | 50 ++++++++++++++++++++++++-----------------
>> 1 file changed, 30 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
>> index 3988c45f9e5f..fd4d59341897 100644
>> --- a/drivers/gpu/drm/i915/intel_tv.c
>> +++ b/drivers/gpu/drm/i915/intel_tv.c
>> @@ -86,7 +86,8 @@ struct intel_tv {
>> };
>>
>> struct video_levels {
>> - int blank, black, burst;
>> + u16 blank, black;
>> + u8 burst;
>> };
>>
>> struct color_conversion {
>> @@ -339,34 +340,43 @@ static const struct video_levels component_levels = {
>>
>> struct tv_mode {
>> const char *name;
>> - int clock;
>> - int refresh; /* in millihertz (for precision) */
>> - u32 oversample;
>> - int hsync_end, hblank_start, hblank_end, htotal;
>> - bool progressive, trilevel_sync, component_only;
>> - int vsync_start_f1, vsync_start_f2, vsync_len;
>> - bool veq_ena;
>> - int veq_start_f1, veq_start_f2, veq_len;
>> - int vi_end_f1, vi_end_f2, nbr_end;
>> - bool burst_ena;
>> - int hburst_start, hburst_len;
>> - int vburst_start_f1, vburst_end_f1;
>> - int vburst_start_f2, vburst_end_f2;
>> - int vburst_start_f3, vburst_end_f3;
>> - int vburst_start_f4, vburst_end_f4;
>> +
>> + u32 clock;
>> + u16 refresh; /* in millihertz (for precision) */
>> + u32 oversample;
>> + u8 hsync_end;
>> + u16 hblank_start, hblank_end, htotal;
>> + bool progressive : 1, trilevel_sync : 1, component_only : 1;
>> + u8 vsync_start_f1, vsync_start_f2, vsync_len;
>> + bool veq_ena : 1;
>> + u8 veq_start_f1, veq_start_f2, veq_len;
>> + u8 vi_end_f1, vi_end_f2;
>> + u16 nbr_end;
>> + bool burst_ena : 1;
>> + u8 hburst_start, hburst_len;
>> + u8 vburst_start_f1;
>> + u16 vburst_end_f1;
>> + u8 vburst_start_f2;
>> + u16 vburst_end_f2;
>> + u8 vburst_start_f3;
>> + u16 vburst_end_f3;
>> + u8 vburst_start_f4;
>> + u16 vburst_end_f4;
> Not convinced about the indentation change.
I found it much more readable like that in this case. I still do, but
thinking about it more, maybe it is just because my editor does no
syntax highlighting for u32 types. I need to fix that.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/9] drm/i915: Shrink TV modes const data
2016-10-07 13:34 ` [PATCH 4/9] drm/i915: Shrink TV modes const data Tvrtko Ursulin
2016-10-10 6:49 ` Jani Nikula
@ 2016-10-10 7:22 ` Joonas Lahtinen
1 sibling, 0 replies; 28+ messages in thread
From: Joonas Lahtinen @ 2016-10-10 7:22 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Make struct video_levels and struct tv_mode use data types
> of sufficient width to save approximately one kilobyte in
> the .rodata section.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Would it hurt us to make the struct packed?
I can see why not to reorder the struct (although, it uses designated
initializers, so it should not matter in that sense).
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/9] drm/i915: Use unsigned int for latencies
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
` (3 preceding siblings ...)
2016-10-07 13:34 ` [PATCH 4/9] drm/i915: Shrink TV modes const data Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-10 7:24 ` Joonas Lahtinen
2016-10-07 13:34 ` [PATCH 6/9] drm/i915: unsigned int is enough for crtc clock Tvrtko Ursulin
` (5 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 96d0c57c816c..68b3614c6a0b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -372,7 +372,7 @@ void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
* A value of 5us seems to be a good balance; safe for very low end
* platforms but not overly aggressive on lower latency configs.
*/
-static const int pessimal_latency_ns = 5000;
+static const unsigned int pessimal_latency_ns = 5000;
#define VLV_FIFO_START(dsparb, dsparb2, lo_shift, hi_shift) \
((((dsparb) >> (lo_shift)) & 0xff) | ((((dsparb2) >> (hi_shift)) & 0x1) << 8))
@@ -585,7 +585,7 @@ static const struct intel_watermark_params i845_wm_info = {
static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
const struct intel_watermark_params *wm,
int fifo_size, int cpp,
- unsigned long latency_ns)
+ unsigned int latency_ns)
{
long entries_required, wm_size;
@@ -709,9 +709,9 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
static bool g4x_compute_wm0(struct drm_device *dev,
int plane,
const struct intel_watermark_params *display,
- int display_latency_ns,
+ unsigned int display_latency_ns,
const struct intel_watermark_params *cursor,
- int cursor_latency_ns,
+ unsigned int cursor_latency_ns,
int *plane_wm,
int *cursor_wm)
{
@@ -796,7 +796,7 @@ static bool g4x_check_srwm(struct drm_device *dev,
static bool g4x_compute_srwm(struct drm_device *dev,
int plane,
- int latency_ns,
+ unsigned int latency_ns,
const struct intel_watermark_params *display,
const struct intel_watermark_params *cursor,
int *display_wm, int *cursor_wm)
@@ -1385,7 +1385,7 @@ static void vlv_update_wm(struct drm_crtc *crtc)
static void g4x_update_wm(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
- static const int sr_latency_ns = 12000;
+ static const unsigned int sr_latency_ns = 12000;
struct drm_i915_private *dev_priv = to_i915(dev);
int planea_wm, planeb_wm, cursora_wm, cursorb_wm;
int plane_sr, cursor_sr;
@@ -1453,7 +1453,7 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
crtc = single_enabled_crtc(dev);
if (crtc) {
/* self-refresh has much higher latency */
- static const int sr_latency_ns = 12000;
+ static const unsigned int sr_latency_ns = 12000;
const struct drm_display_mode *adjusted_mode = &to_intel_crtc(crtc)->config->base.adjusted_mode;
int clock = adjusted_mode->crtc_clock;
int htotal = adjusted_mode->crtc_htotal;
@@ -1600,7 +1600,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
/* Calc sr entries for one plane configs */
if (HAS_FW_BLC(dev) && enabled) {
/* self-refresh has much higher latency */
- static const int sr_latency_ns = 6000;
+ static const unsigned int sr_latency_ns = 6000;
const struct drm_display_mode *adjusted_mode = &to_intel_crtc(enabled)->config->base.adjusted_mode;
int clock = adjusted_mode->crtc_clock;
int htotal = adjusted_mode->crtc_htotal;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 5/9] drm/i915: Use unsigned int for latencies
2016-10-07 13:34 ` [PATCH 5/9] drm/i915: Use unsigned int for latencies Tvrtko Ursulin
@ 2016-10-10 7:24 ` Joonas Lahtinen
2016-10-10 8:34 ` Tvrtko Ursulin
0 siblings, 1 reply; 28+ messages in thread
From: Joonas Lahtinen @ 2016-10-10 7:24 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
And maybe Suggested-by? :)
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/9] drm/i915: Use unsigned int for latencies
2016-10-10 7:24 ` Joonas Lahtinen
@ 2016-10-10 8:34 ` Tvrtko Ursulin
0 siblings, 0 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-10 8:34 UTC (permalink / raw)
To: Joonas Lahtinen, Tvrtko Ursulin, Intel-gfx
On 10/10/2016 08:24, Joonas Lahtinen wrote:
> On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> And maybe Suggested-by? :)
Definitely, yes, should have put it on all of the related ones. Will
correct.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 6/9] drm/i915: unsigned int is enough for crtc clock
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
` (4 preceding siblings ...)
2016-10-07 13:34 ` [PATCH 5/9] drm/i915: Use unsigned int for latencies Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-10 7:25 ` Joonas Lahtinen
2016-10-07 13:34 ` [PATCH 7/9] drm/i915: Convert get_fifo_size return from int to unsigned int Tvrtko Ursulin
` (4 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 68b3614c6a0b..56726777b4d3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -582,7 +582,7 @@ static const struct intel_watermark_params i845_wm_info = {
* past the watermark point. If the FIFO drains completely, a FIFO underrun
* will occur, and a display engine hang could result.
*/
-static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
+static unsigned long intel_calculate_wm(unsigned int clock_in_khz,
const struct intel_watermark_params *wm,
int fifo_size, int cpp,
unsigned int latency_ns)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 7/9] drm/i915: Convert get_fifo_size return from int to unsigned int
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
` (5 preceding siblings ...)
2016-10-07 13:34 ` [PATCH 6/9] drm/i915: unsigned int is enough for crtc clock Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-10 7:46 ` Joonas Lahtinen
2016-10-07 13:34 ` [PATCH 8/9] drm/i915: Make intel_calculate_wm return " Tvrtko Ursulin
` (3 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Unsigned is a more appropriate data type in this case.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 36 ++++++++++++++++++------------------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a219a3534750..59a2a9598c4b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -492,7 +492,7 @@ struct dpll;
struct drm_i915_display_funcs {
int (*get_display_clock_speed)(struct drm_device *dev);
- int (*get_fifo_size)(struct drm_device *dev, int plane);
+ unsigned int (*get_fifo_size)(struct drm_device *dev, int plane);
int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
int (*compute_intermediate_wm)(struct drm_device *dev,
struct intel_crtc *intel_crtc,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 56726777b4d3..faa379e54322 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -377,11 +377,11 @@ static const unsigned int pessimal_latency_ns = 5000;
#define VLV_FIFO_START(dsparb, dsparb2, lo_shift, hi_shift) \
((((dsparb) >> (lo_shift)) & 0xff) | ((((dsparb2) >> (hi_shift)) & 0x1) << 8))
-static int vlv_get_fifo_size(struct drm_device *dev,
- enum pipe pipe, int plane)
+static unsigned int vlv_get_fifo_size(struct drm_device *dev,
+ enum pipe pipe, int plane)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- int sprite0_start, sprite1_start, size;
+ unsigned int sprite0_start, sprite1_start, size;
switch (pipe) {
uint32_t dsparb, dsparb2, dsparb3;
@@ -421,7 +421,7 @@ static int vlv_get_fifo_size(struct drm_device *dev,
return 0;
}
- DRM_DEBUG_KMS("Pipe %c %s %c FIFO size: %d\n",
+ DRM_DEBUG_KMS("Pipe %c %s %c FIFO size: %u\n",
pipe_name(pipe), plane == 0 ? "primary" : "sprite",
plane == 0 ? plane_name(pipe) : sprite_name(pipe, plane - 1),
size);
@@ -429,49 +429,49 @@ static int vlv_get_fifo_size(struct drm_device *dev,
return size;
}
-static int i9xx_get_fifo_size(struct drm_device *dev, int plane)
+static unsigned int i9xx_get_fifo_size(struct drm_device *dev, int plane)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- uint32_t dsparb = I915_READ(DSPARB);
- int size;
+ u32 dsparb = I915_READ(DSPARB);
+ unsigned int size;
size = dsparb & 0x7f;
if (plane)
size = ((dsparb >> DSPARB_CSTART_SHIFT) & 0x7f) - size;
- DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %d\n", dsparb,
+ DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %u\n", dsparb,
plane ? "B" : "A", size);
return size;
}
-static int i830_get_fifo_size(struct drm_device *dev, int plane)
+static unsigned int i830_get_fifo_size(struct drm_device *dev, int plane)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- uint32_t dsparb = I915_READ(DSPARB);
- int size;
+ u32 dsparb = I915_READ(DSPARB);
+ unsigned int size;
size = dsparb & 0x1ff;
if (plane)
size = ((dsparb >> DSPARB_BEND_SHIFT) & 0x1ff) - size;
size >>= 1; /* Convert to cachelines */
- DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %d\n", dsparb,
+ DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %u\n", dsparb,
plane ? "B" : "A", size);
return size;
}
-static int i845_get_fifo_size(struct drm_device *dev, int plane)
+static unsigned int i845_get_fifo_size(struct drm_device *dev, int plane)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- uint32_t dsparb = I915_READ(DSPARB);
- int size;
+ u32 dsparb = I915_READ(DSPARB);
+ unsigned int size;
size = dsparb & 0x7f;
size >>= 2; /* Convert to cachelines */
- DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %d\n", dsparb,
+ DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %u\n", dsparb,
plane ? "B" : "A",
size);
@@ -584,7 +584,7 @@ static const struct intel_watermark_params i845_wm_info = {
*/
static unsigned long intel_calculate_wm(unsigned int clock_in_khz,
const struct intel_watermark_params *wm,
- int fifo_size, int cpp,
+ unsigned int fifo_size, int cpp,
unsigned int latency_ns)
{
long entries_required, wm_size;
@@ -1522,7 +1522,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
uint32_t fwater_lo;
uint32_t fwater_hi;
int cwm, srwm = 1;
- int fifo_size;
+ unsigned int fifo_size;
int planea_wm, planeb_wm;
struct drm_crtc *crtc, *enabled = NULL;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 8/9] drm/i915: Make intel_calculate_wm return unsigned int
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
` (6 preceding siblings ...)
2016-10-07 13:34 ` [PATCH 7/9] drm/i915: Convert get_fifo_size return from int to unsigned int Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-10 8:02 ` Joonas Lahtinen
2016-10-07 13:34 ` [PATCH 9/9] drm/i915: Tidy watermark computation local types Tvrtko Ursulin
` (2 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Either unsigned int is enough or it isn't. There is no point in
using an unsigned long here.
Also replace local long variables with integers. No need to have
a difference between 32- and 64-bit build in any case.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index faa379e54322..308edc4378fa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -582,12 +582,12 @@ static const struct intel_watermark_params i845_wm_info = {
* past the watermark point. If the FIFO drains completely, a FIFO underrun
* will occur, and a display engine hang could result.
*/
-static unsigned long intel_calculate_wm(unsigned int clock_in_khz,
- const struct intel_watermark_params *wm,
- unsigned int fifo_size, int cpp,
- unsigned int latency_ns)
+static unsigned int intel_calculate_wm(unsigned int clock_in_khz,
+ const struct intel_watermark_params *wm,
+ unsigned int fifo_size, int cpp,
+ unsigned int latency_ns)
{
- long entries_required, wm_size;
+ int entries_required, wm_size;
/*
* Note: we need to make sure we don't overflow for various clock &
@@ -595,18 +595,17 @@ static unsigned long intel_calculate_wm(unsigned int clock_in_khz,
* clocks go from a few thousand to several hundred thousand.
* latency is usually a few thousand
*/
- entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) /
- 1000;
+ entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) / 1000;
entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size);
- DRM_DEBUG_KMS("FIFO entries required for mode: %ld\n", entries_required);
+ DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required);
wm_size = fifo_size - (entries_required + wm->guard_size);
- DRM_DEBUG_KMS("FIFO watermark level: %ld\n", wm_size);
+ DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size);
/* Don't promote wm_size to unsigned... */
- if (wm_size > (long)wm->max_wm)
+ if (wm_size > (int)wm->max_wm)
wm_size = wm->max_wm;
if (wm_size <= 0)
wm_size = wm->default_wm;
@@ -646,7 +645,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
struct drm_crtc *crtc;
const struct cxsr_latency *latency;
u32 reg;
- unsigned long wm;
+ unsigned int wm;
latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev), dev_priv->is_ddr3,
dev_priv->fsb_freq, dev_priv->mem_freq);
@@ -1522,8 +1521,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
uint32_t fwater_lo;
uint32_t fwater_hi;
int cwm, srwm = 1;
- unsigned int fifo_size;
- int planea_wm, planeb_wm;
+ unsigned int fifo_size, planea_wm, planeb_wm;
struct drm_crtc *crtc, *enabled = NULL;
if (IS_I945GM(dev))
@@ -1548,7 +1546,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
enabled = crtc;
} else {
planea_wm = fifo_size - wm_info->guard_size;
- if (planea_wm > (long)wm_info->max_wm)
+ if (planea_wm > wm_info->max_wm)
planea_wm = wm_info->max_wm;
}
@@ -1573,11 +1571,11 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
enabled = NULL;
} else {
planeb_wm = fifo_size - wm_info->guard_size;
- if (planeb_wm > (long)wm_info->max_wm)
+ if (planeb_wm > wm_info->max_wm)
planeb_wm = wm_info->max_wm;
}
- DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
+ DRM_DEBUG_KMS("FIFO watermarks - A: %u, B: %u\n", planea_wm, planeb_wm);
if (IS_I915GM(dev) && enabled) {
struct drm_i915_gem_object *obj;
@@ -1653,8 +1651,8 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_crtc *crtc;
const struct drm_display_mode *adjusted_mode;
- uint32_t fwater_lo;
- int planea_wm;
+ u32 fwater_lo;
+ unsigned int planea_wm;
crtc = single_enabled_crtc(dev);
if (crtc == NULL)
@@ -1668,7 +1666,7 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
fwater_lo = I915_READ(FW_BLC) & ~0xfff;
fwater_lo |= (3<<8) | planea_wm;
- DRM_DEBUG_KMS("Setting FIFO watermarks - A: %d\n", planea_wm);
+ DRM_DEBUG_KMS("Setting FIFO watermarks - A: %u\n", planea_wm);
I915_WRITE(FW_BLC, fwater_lo);
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 8/9] drm/i915: Make intel_calculate_wm return unsigned int
2016-10-07 13:34 ` [PATCH 8/9] drm/i915: Make intel_calculate_wm return " Tvrtko Ursulin
@ 2016-10-10 8:02 ` Joonas Lahtinen
2016-10-10 8:33 ` Tvrtko Ursulin
0 siblings, 1 reply; 28+ messages in thread
From: Joonas Lahtinen @ 2016-10-10 8:02 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote:
> @@ -595,18 +595,17 @@ static unsigned long intel_calculate_wm(unsigned int clock_in_khz,
> * clocks go from a few thousand to several hundred thousand.
> * latency is usually a few thousand
> */
> - entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) /
> - 1000;
> + entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) / 1000;
Is the intermediary result always within int?
> entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size);
>
> - DRM_DEBUG_KMS("FIFO entries required for mode: %ld\n", entries_required);
> + DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required);
>
> wm_size = fifo_size - (entries_required + wm->guard_size);
>
> - DRM_DEBUG_KMS("FIFO watermark level: %ld\n", wm_size);
> + DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size);
>
> /* Don't promote wm_size to unsigned... */
> - if (wm_size > (long)wm->max_wm)
> + if (wm_size > (int)wm->max_wm)
> wm_size = wm->max_wm;
> if (wm_size <= 0)
> wm_size = wm->default_wm;
This could be
if (wm_size <= 0)
wm_size = wm->default_wm;
else if (wm_size > U16_MAX || (u16)wm_size > wm->max_wm)
wm_size = wm->max_wm;
or something?
Other than that, types look better that they used to.
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
The whole type landscape in watermark code seems bit sloppy to me.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 8/9] drm/i915: Make intel_calculate_wm return unsigned int
2016-10-10 8:02 ` Joonas Lahtinen
@ 2016-10-10 8:33 ` Tvrtko Ursulin
0 siblings, 0 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-10 8:33 UTC (permalink / raw)
To: Joonas Lahtinen, Tvrtko Ursulin, Intel-gfx
On 10/10/2016 09:02, Joonas Lahtinen wrote:
> On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote:
>> @@ -595,18 +595,17 @@ static unsigned long intel_calculate_wm(unsigned int clock_in_khz,
>> * clocks go from a few thousand to several hundred thousand.
>> * latency is usually a few thousand
>> */
>> - entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) /
>> - 1000;
>> + entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) / 1000;
> Is the intermediary result always within int?
I certainly hope so! Display clock in MHz * cpp * latency which is u16.
So 2^31 / 2^16 / cpp=8 leaves 4GHz for the clock before it would overflow.
>> entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size);
>>
>> - DRM_DEBUG_KMS("FIFO entries required for mode: %ld\n", entries_required);
>> + DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required);
>>
>> wm_size = fifo_size - (entries_required + wm->guard_size);
>>
>> - DRM_DEBUG_KMS("FIFO watermark level: %ld\n", wm_size);
>> + DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size);
>>
>> /* Don't promote wm_size to unsigned... */
>> - if (wm_size > (long)wm->max_wm)
>> + if (wm_size > (int)wm->max_wm)
>> wm_size = wm->max_wm;
>> if (wm_size <= 0)
>> wm_size = wm->default_wm;
> This could be
>
> if (wm_size <= 0)
> wm_size = wm->default_wm;
> else if (wm_size > U16_MAX || (u16)wm_size > wm->max_wm)
> wm_size = wm->max_wm;
>
> or something?
>
> Other than that, types look better that they used to.
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> The whole type landscape in watermark code seems bit sloppy to me.
Yes, but I would also like not to introduce regressions. So I am a bit
reluctant to push forward with the second half of this series. :I
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 9/9] drm/i915: Tidy watermark computation local types
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
` (7 preceding siblings ...)
2016-10-07 13:34 ` [PATCH 8/9] drm/i915: Make intel_calculate_wm return " Tvrtko Ursulin
@ 2016-10-07 13:34 ` Tvrtko Ursulin
2016-10-07 13:48 ` Ville Syrjälä
2016-10-10 8:19 ` ✗ Fi.CI.BAT: warning for .rodata diet 2 (non-disruptive version) Patchwork
2016-10-10 8:27 ` ✗ Fi.CI.BAT: failure " Patchwork
10 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 13:34 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Convert to from signed to unsigned and from longs
to integers where appropriate.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 60 +++++++++++++++++++----------------------
1 file changed, 28 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 308edc4378fa..1203a7a80e9e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -711,14 +711,14 @@ static bool g4x_compute_wm0(struct drm_device *dev,
unsigned int display_latency_ns,
const struct intel_watermark_params *cursor,
unsigned int cursor_latency_ns,
- int *plane_wm,
- int *cursor_wm)
+ unsigned int *plane_wm,
+ unsigned int *cursor_wm)
{
struct drm_crtc *crtc;
const struct drm_display_mode *adjusted_mode;
int htotal, hdisplay, clock, cpp;
- int line_time_us, line_count;
- int entries, tlb_miss;
+ unsigned int line_time_us, line_count, entries;
+ int tlb_miss;
crtc = intel_get_crtc_for_plane(dev, plane);
if (!intel_crtc_active(crtc)) {
@@ -740,7 +740,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
entries += tlb_miss;
entries = DIV_ROUND_UP(entries, display->cacheline_size);
*plane_wm = entries + display->guard_size;
- if (*plane_wm > (int)display->max_wm)
+ if (*plane_wm > display->max_wm)
*plane_wm = display->max_wm;
/* Use the large buffer method to calculate cursor watermark */
@@ -752,8 +752,8 @@ static bool g4x_compute_wm0(struct drm_device *dev,
entries += tlb_miss;
entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
*cursor_wm = entries + cursor->guard_size;
- if (*cursor_wm > (int)cursor->max_wm)
- *cursor_wm = (int)cursor->max_wm;
+ if (*cursor_wm > cursor->max_wm)
+ *cursor_wm = cursor->max_wm;
return true;
}
@@ -766,21 +766,21 @@ static bool g4x_compute_wm0(struct drm_device *dev,
* must be disabled.
*/
static bool g4x_check_srwm(struct drm_device *dev,
- int display_wm, int cursor_wm,
+ unsigned int display_wm, unsigned int cursor_wm,
const struct intel_watermark_params *display,
const struct intel_watermark_params *cursor)
{
- DRM_DEBUG_KMS("SR watermark: display plane %d, cursor %d\n",
+ DRM_DEBUG_KMS("SR watermark: display plane %u, cursor %u\n",
display_wm, cursor_wm);
if (display_wm > display->max_wm) {
- DRM_DEBUG_KMS("display watermark is too large(%d/%u), disabling\n",
+ DRM_DEBUG_KMS("display watermark is too large(%u/%u), disabling\n",
display_wm, display->max_wm);
return false;
}
if (cursor_wm > cursor->max_wm) {
- DRM_DEBUG_KMS("cursor watermark is too large(%d/%u), disabling\n",
+ DRM_DEBUG_KMS("cursor watermark is too large(%u/%u), disabling\n",
cursor_wm, cursor->max_wm);
return false;
}
@@ -798,15 +798,13 @@ static bool g4x_compute_srwm(struct drm_device *dev,
unsigned int latency_ns,
const struct intel_watermark_params *display,
const struct intel_watermark_params *cursor,
- int *display_wm, int *cursor_wm)
+ unsigned int *display_wm, unsigned int *cursor_wm)
{
struct drm_crtc *crtc;
const struct drm_display_mode *adjusted_mode;
int hdisplay, htotal, cpp, clock;
- unsigned long line_time_us;
- int line_count, line_size;
- int small, large;
- int entries;
+ unsigned int line_time_us, line_count, line_size;
+ unsigned int small, large, entries;
if (!latency_ns) {
*display_wm = *cursor_wm = 0;
@@ -836,9 +834,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
*cursor_wm = entries + cursor->guard_size;
- return g4x_check_srwm(dev,
- *display_wm, *cursor_wm,
- display, cursor);
+ return g4x_check_srwm(dev, *display_wm, *cursor_wm, display, cursor);
}
#define FW_WM_VLV(value, plane) \
@@ -1386,8 +1382,9 @@ static void g4x_update_wm(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
static const unsigned int sr_latency_ns = 12000;
struct drm_i915_private *dev_priv = to_i915(dev);
- int planea_wm, planeb_wm, cursora_wm, cursorb_wm;
- int plane_sr, cursor_sr;
+ unsigned int planea_wm, planeb_wm;
+ unsigned int cursora_wm, cursorb_wm;
+ unsigned int plane_sr, cursor_sr;
unsigned int enabled = 0;
bool cxsr_enabled;
@@ -1416,8 +1413,8 @@ static void g4x_update_wm(struct drm_crtc *crtc)
plane_sr = cursor_sr = 0;
}
- DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%d, cursor=%d, "
- "B: plane=%d, cursor=%d, SR: plane=%d, cursor=%d\n",
+ DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%u, cursor=%u, "
+ "B: plane=%u, cursor=%u, SR: plane=%u, cursor=%u\n",
planea_wm, cursora_wm,
planeb_wm, cursorb_wm,
plane_sr, cursor_sr);
@@ -1458,8 +1455,7 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
int htotal = adjusted_mode->crtc_htotal;
int hdisplay = to_intel_crtc(crtc)->config->pipe_src_w;
int cpp = drm_format_plane_cpp(crtc->primary->state->fb->pixel_format, 0);
- unsigned long line_time_us;
- int entries;
+ unsigned int line_time_us, entries;
line_time_us = max(htotal * 1000 / clock, 1);
@@ -1518,9 +1514,10 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
struct drm_device *dev = unused_crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
const struct intel_watermark_params *wm_info;
- uint32_t fwater_lo;
- uint32_t fwater_hi;
- int cwm, srwm = 1;
+ u32 fwater_lo;
+ u32 fwater_hi;
+ unsigned int cwm;
+ int srwm = 1;
unsigned int fifo_size, planea_wm, planeb_wm;
struct drm_crtc *crtc, *enabled = NULL;
@@ -1604,8 +1601,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
int htotal = adjusted_mode->crtc_htotal;
int hdisplay = to_intel_crtc(enabled)->config->pipe_src_w;
int cpp = drm_format_plane_cpp(enabled->primary->state->fb->pixel_format, 0);
- unsigned long line_time_us;
- int entries;
+ unsigned int line_time_us, entries;
if (IS_I915GM(dev) || IS_I945GM(dev))
cpp = 4;
@@ -1616,7 +1612,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
entries = (((sr_latency_ns / line_time_us) + 1000) / 1000) *
cpp * hdisplay;
entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
- DRM_DEBUG_KMS("self-refresh entries: %d\n", entries);
+ DRM_DEBUG_KMS("self-refresh entries: %u\n", entries);
srwm = wm_info->fifo_size - entries;
if (srwm < 0)
srwm = 1;
@@ -1628,7 +1624,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
I915_WRITE(FW_BLC_SELF, srwm & 0x3f);
}
- DRM_DEBUG_KMS("Setting FIFO watermarks - A: %d, B: %d, C: %d, SR %d\n",
+ DRM_DEBUG_KMS("Setting FIFO watermarks - A: %u, B: %u, C: %u, SR %d\n",
planea_wm, planeb_wm, cwm, srwm);
fwater_lo = ((planeb_wm & 0x3f) << 16) | (planea_wm & 0x3f);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 9/9] drm/i915: Tidy watermark computation local types
2016-10-07 13:34 ` [PATCH 9/9] drm/i915: Tidy watermark computation local types Tvrtko Ursulin
@ 2016-10-07 13:48 ` Ville Syrjälä
2016-10-07 14:51 ` Tvrtko Ursulin
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2016-10-07 13:48 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Fri, Oct 07, 2016 at 02:34:12PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Convert to from signed to unsigned and from longs
Ddi you check that we can't get negative values? Depending on how you do
things that can be possible on gmch platforms on account of the
watermarks being inverted. IIRC we might have places were negative
values might be present in temporary results.
> to integers where appropriate.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 60 +++++++++++++++++++----------------------
> 1 file changed, 28 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 308edc4378fa..1203a7a80e9e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -711,14 +711,14 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> unsigned int display_latency_ns,
> const struct intel_watermark_params *cursor,
> unsigned int cursor_latency_ns,
> - int *plane_wm,
> - int *cursor_wm)
> + unsigned int *plane_wm,
> + unsigned int *cursor_wm)
> {
> struct drm_crtc *crtc;
> const struct drm_display_mode *adjusted_mode;
> int htotal, hdisplay, clock, cpp;
> - int line_time_us, line_count;
> - int entries, tlb_miss;
> + unsigned int line_time_us, line_count, entries;
> + int tlb_miss;
>
> crtc = intel_get_crtc_for_plane(dev, plane);
> if (!intel_crtc_active(crtc)) {
> @@ -740,7 +740,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> entries += tlb_miss;
> entries = DIV_ROUND_UP(entries, display->cacheline_size);
> *plane_wm = entries + display->guard_size;
> - if (*plane_wm > (int)display->max_wm)
> + if (*plane_wm > display->max_wm)
> *plane_wm = display->max_wm;
>
> /* Use the large buffer method to calculate cursor watermark */
> @@ -752,8 +752,8 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> entries += tlb_miss;
> entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
> *cursor_wm = entries + cursor->guard_size;
> - if (*cursor_wm > (int)cursor->max_wm)
> - *cursor_wm = (int)cursor->max_wm;
> + if (*cursor_wm > cursor->max_wm)
> + *cursor_wm = cursor->max_wm;
>
> return true;
> }
> @@ -766,21 +766,21 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> * must be disabled.
> */
> static bool g4x_check_srwm(struct drm_device *dev,
> - int display_wm, int cursor_wm,
> + unsigned int display_wm, unsigned int cursor_wm,
> const struct intel_watermark_params *display,
> const struct intel_watermark_params *cursor)
> {
> - DRM_DEBUG_KMS("SR watermark: display plane %d, cursor %d\n",
> + DRM_DEBUG_KMS("SR watermark: display plane %u, cursor %u\n",
> display_wm, cursor_wm);
>
> if (display_wm > display->max_wm) {
> - DRM_DEBUG_KMS("display watermark is too large(%d/%u), disabling\n",
> + DRM_DEBUG_KMS("display watermark is too large(%u/%u), disabling\n",
> display_wm, display->max_wm);
> return false;
> }
>
> if (cursor_wm > cursor->max_wm) {
> - DRM_DEBUG_KMS("cursor watermark is too large(%d/%u), disabling\n",
> + DRM_DEBUG_KMS("cursor watermark is too large(%u/%u), disabling\n",
> cursor_wm, cursor->max_wm);
> return false;
> }
> @@ -798,15 +798,13 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> unsigned int latency_ns,
> const struct intel_watermark_params *display,
> const struct intel_watermark_params *cursor,
> - int *display_wm, int *cursor_wm)
> + unsigned int *display_wm, unsigned int *cursor_wm)
> {
> struct drm_crtc *crtc;
> const struct drm_display_mode *adjusted_mode;
> int hdisplay, htotal, cpp, clock;
> - unsigned long line_time_us;
> - int line_count, line_size;
> - int small, large;
> - int entries;
> + unsigned int line_time_us, line_count, line_size;
> + unsigned int small, large, entries;
>
> if (!latency_ns) {
> *display_wm = *cursor_wm = 0;
> @@ -836,9 +834,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
> *cursor_wm = entries + cursor->guard_size;
>
> - return g4x_check_srwm(dev,
> - *display_wm, *cursor_wm,
> - display, cursor);
> + return g4x_check_srwm(dev, *display_wm, *cursor_wm, display, cursor);
> }
>
> #define FW_WM_VLV(value, plane) \
> @@ -1386,8 +1382,9 @@ static void g4x_update_wm(struct drm_crtc *crtc)
> struct drm_device *dev = crtc->dev;
> static const unsigned int sr_latency_ns = 12000;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - int planea_wm, planeb_wm, cursora_wm, cursorb_wm;
> - int plane_sr, cursor_sr;
> + unsigned int planea_wm, planeb_wm;
> + unsigned int cursora_wm, cursorb_wm;
> + unsigned int plane_sr, cursor_sr;
> unsigned int enabled = 0;
> bool cxsr_enabled;
>
> @@ -1416,8 +1413,8 @@ static void g4x_update_wm(struct drm_crtc *crtc)
> plane_sr = cursor_sr = 0;
> }
>
> - DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%d, cursor=%d, "
> - "B: plane=%d, cursor=%d, SR: plane=%d, cursor=%d\n",
> + DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%u, cursor=%u, "
> + "B: plane=%u, cursor=%u, SR: plane=%u, cursor=%u\n",
> planea_wm, cursora_wm,
> planeb_wm, cursorb_wm,
> plane_sr, cursor_sr);
> @@ -1458,8 +1455,7 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
> int htotal = adjusted_mode->crtc_htotal;
> int hdisplay = to_intel_crtc(crtc)->config->pipe_src_w;
> int cpp = drm_format_plane_cpp(crtc->primary->state->fb->pixel_format, 0);
> - unsigned long line_time_us;
> - int entries;
> + unsigned int line_time_us, entries;
>
> line_time_us = max(htotal * 1000 / clock, 1);
>
> @@ -1518,9 +1514,10 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> struct drm_device *dev = unused_crtc->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> const struct intel_watermark_params *wm_info;
> - uint32_t fwater_lo;
> - uint32_t fwater_hi;
> - int cwm, srwm = 1;
> + u32 fwater_lo;
> + u32 fwater_hi;
> + unsigned int cwm;
> + int srwm = 1;
> unsigned int fifo_size, planea_wm, planeb_wm;
> struct drm_crtc *crtc, *enabled = NULL;
>
> @@ -1604,8 +1601,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> int htotal = adjusted_mode->crtc_htotal;
> int hdisplay = to_intel_crtc(enabled)->config->pipe_src_w;
> int cpp = drm_format_plane_cpp(enabled->primary->state->fb->pixel_format, 0);
> - unsigned long line_time_us;
> - int entries;
> + unsigned int line_time_us, entries;
>
> if (IS_I915GM(dev) || IS_I945GM(dev))
> cpp = 4;
> @@ -1616,7 +1612,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> entries = (((sr_latency_ns / line_time_us) + 1000) / 1000) *
> cpp * hdisplay;
> entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
> - DRM_DEBUG_KMS("self-refresh entries: %d\n", entries);
> + DRM_DEBUG_KMS("self-refresh entries: %u\n", entries);
> srwm = wm_info->fifo_size - entries;
> if (srwm < 0)
> srwm = 1;
> @@ -1628,7 +1624,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> I915_WRITE(FW_BLC_SELF, srwm & 0x3f);
> }
>
> - DRM_DEBUG_KMS("Setting FIFO watermarks - A: %d, B: %d, C: %d, SR %d\n",
> + DRM_DEBUG_KMS("Setting FIFO watermarks - A: %u, B: %u, C: %u, SR %d\n",
> planea_wm, planeb_wm, cwm, srwm);
>
> fwater_lo = ((planeb_wm & 0x3f) << 16) | (planea_wm & 0x3f);
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 9/9] drm/i915: Tidy watermark computation local types
2016-10-07 13:48 ` Ville Syrjälä
@ 2016-10-07 14:51 ` Tvrtko Ursulin
2016-10-07 15:16 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2016-10-07 14:51 UTC (permalink / raw)
To: Ville Syrjälä, Tvrtko Ursulin; +Cc: Intel-gfx
On 07/10/2016 14:48, Ville Syrjälä wrote:
> On Fri, Oct 07, 2016 at 02:34:12PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Convert to from signed to unsigned and from longs
> Ddi you check that we can't get negative values? Depending on how you do
> things that can be possible on gmch platforms on account of the
> watermarks being inverted. IIRC we might have places were negative
> values might be present in temporary results.
I tried to be careful, but it is of course possible I've missed
something. Could you give me a more precise pointer on where exactly to
look? In this particular patch?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 9/9] drm/i915: Tidy watermark computation local types
2016-10-07 14:51 ` Tvrtko Ursulin
@ 2016-10-07 15:16 ` Ville Syrjälä
2016-10-10 8:06 ` Joonas Lahtinen
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2016-10-07 15:16 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Fri, Oct 07, 2016 at 03:51:50PM +0100, Tvrtko Ursulin wrote:
>
> On 07/10/2016 14:48, Ville Syrjälä wrote:
> > On Fri, Oct 07, 2016 at 02:34:12PM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Convert to from signed to unsigned and from longs
> > Ddi you check that we can't get negative values? Depending on how you do
> > things that can be possible on gmch platforms on account of the
> > watermarks being inverted. IIRC we might have places were negative
> > values might be present in temporary results.
>
> I tried to be careful, but it is of course possible I've missed
> something. Could you give me a more precise pointer on where exactly to
> look? In this particular patch?
I don't have specifics in mind right now. But in general I've become a
bit wary of unsigned in my older days on account of integer promotions
and arithmetic conversions. It's far too easy to end up with an unsigned
value where signed was needed.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 9/9] drm/i915: Tidy watermark computation local types
2016-10-07 15:16 ` Ville Syrjälä
@ 2016-10-10 8:06 ` Joonas Lahtinen
0 siblings, 0 replies; 28+ messages in thread
From: Joonas Lahtinen @ 2016-10-10 8:06 UTC (permalink / raw)
To: Ville Syrjälä, Tvrtko Ursulin; +Cc: Intel-gfx
On pe, 2016-10-07 at 18:16 +0300, Ville Syrjälä wrote:
> On Fri, Oct 07, 2016 at 03:51:50PM +0100, Tvrtko Ursulin wrote:
> >
> > I tried to be careful, but it is of course possible I've missed
> > something. Could you give me a more precise pointer on where exactly to
> > look? In this particular patch?
>
> I don't have specifics in mind right now. But in general I've become a
> bit wary of unsigned in my older days on account of integer promotions
> and arithmetic conversions. It's far too easy to end up with an unsigned
> value where signed was needed.
Yes, if one is not paying attention to the maximum expected values. And
looking back a few months, paying attention is what our watermark code
needs badly. By quick look, some types are random and there're quite a
few casts. Try "git grep max_wm" for example.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✗ Fi.CI.BAT: warning for .rodata diet 2 (non-disruptive version)
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
` (8 preceding siblings ...)
2016-10-07 13:34 ` [PATCH 9/9] drm/i915: Tidy watermark computation local types Tvrtko Ursulin
@ 2016-10-10 8:19 ` Patchwork
2016-10-10 8:27 ` ✗ Fi.CI.BAT: failure " Patchwork
10 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2016-10-10 8:19 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: .rodata diet 2 (non-disruptive version)
URL : https://patchwork.freedesktop.org/series/13445/
State : warning
== Summary ==
Series 13445v1 .rodata diet 2 (non-disruptive version)
https://patchwork.freedesktop.org/api/1.0/series/13445/revisions/1/mbox/
Results at /archive/results/CI_IGT_test/Patchwork_2648/
0f2a32f6bdc7590370d79a8ec5be27a8762f1d91 drm-intel-nightly: 2016y-10m-10d-05h-29m-53s UTC integration manifest
cdb5478 drm/i915: Tidy watermark computation local types
4e9225d drm/i915: Make intel_calculate_wm return unsigned int
08ef44d drm/i915: Convert get_fifo_size return from int to unsigned int
359176c drm/i915: unsigned int is enough for crtc clock
2442f60 drm/i915: Use unsigned int for latencies
cbb8eab drm/i915: Shrink TV modes const data
40148c9 drm/i915: Shrink per-platform watermark configuration
b0fbaa3 drm/i915: Shrink sdvo_cmd_names
34dbd0b drm/i915: Shrink cxsr_latency_table
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* ✗ Fi.CI.BAT: failure for .rodata diet 2 (non-disruptive version)
2016-10-07 13:34 [PATCH 0/9] .rodata diet 2 (non-disruptive version) Tvrtko Ursulin
` (9 preceding siblings ...)
2016-10-10 8:19 ` ✗ Fi.CI.BAT: warning for .rodata diet 2 (non-disruptive version) Patchwork
@ 2016-10-10 8:27 ` Patchwork
10 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2016-10-10 8:27 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: .rodata diet 2 (non-disruptive version)
URL : https://patchwork.freedesktop.org/series/13445/
State : failure
== Summary ==
Series 13445v1 .rodata diet 2 (non-disruptive version)
https://patchwork.freedesktop.org/api/1.0/series/13445/revisions/1/mbox/
Test gem_busy:
Subgroup basic-hang-default:
pass -> FAIL (fi-hsw-4770r)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (fi-byt-j1900)
Test vgem_basic:
Subgroup unload:
pass -> SKIP (fi-ilk-650)
skip -> PASS (fi-ivb-3770)
skip -> PASS (fi-bsw-n3050)
pass -> SKIP (fi-hsw-4770)
skip -> PASS (fi-skl-6700k)
pass -> SKIP (fi-ivb-3520m)
fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17
fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43
fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31
fi-byt-j1900 total:248 pass:213 dwarn:2 dfail:0 fail:1 skip:32
fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36
fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24
fi-hsw-4770r total:248 pass:223 dwarn:0 dfail:0 fail:1 skip:24
fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62
fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27
fi-ivb-3770 total:248 pass:208 dwarn:0 dfail:0 fail:0 skip:40
fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26
fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16
fi-skl-6700hq total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24
fi-skl-6700k total:248 pass:222 dwarn:1 dfail:0 fail:0 skip:25
fi-skl-6770hq total:248 pass:230 dwarn:1 dfail:0 fail:1 skip:16
fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37
fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39
Results at /archive/results/CI_IGT_test/Patchwork_2648/
0f2a32f6bdc7590370d79a8ec5be27a8762f1d91 drm-intel-nightly: 2016y-10m-10d-05h-29m-53s UTC integration manifest
cdb5478 drm/i915: Tidy watermark computation local types
4e9225d drm/i915: Make intel_calculate_wm return unsigned int
08ef44d drm/i915: Convert get_fifo_size return from int to unsigned int
359176c drm/i915: unsigned int is enough for crtc clock
2442f60 drm/i915: Use unsigned int for latencies
cbb8eab drm/i915: Shrink TV modes const data
40148c9 drm/i915: Shrink per-platform watermark configuration
b0fbaa3 drm/i915: Shrink sdvo_cmd_names
34dbd0b drm/i915: Shrink cxsr_latency_table
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread