New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 906726 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Bad cxsr_latency_table in i915/intel_pm.c

Project Member Reported by groeck@chromium.org, Nov 19

Issue description

Commit 7ba382e29c4f ("UPSTREAM: drm/i915: Shrink cxsr_latency_table") in chromeos-4.4 rearranged struct cxsr_latency as follows.

 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;
+       u16 fsb_freq;
+       u16 mem_freq;
+       u16 display_sr;
+       u16 display_hpll_disable;
+       u16 cursor_sr;
+       u16 cursor_hpll_disable;
+       bool is_desktop : 1;
+       bool is_ddr3 : 1;
 };

However, the commit did not update the actual table, which is still:

static const struct cxsr_latency cxsr_latency_table[] = {
        {1, 0, 800, 400, 3382, 33382, 3983, 33983},    /* DDR2-400 SC */
        {1, 0, 800, 667, 3354, 33354, 3807, 33807},    /* DDR2-667 SC */
        {1, 0, 800, 800, 3347, 33347, 3763, 33763},    /* DDR2-800 SC */
        {1, 1, 800, 667, 6420, 36420, 6873, 36873},    /* DDR3-667 SC */
        {1, 1, 800, 800, 5902, 35902, 6318, 36318},    /* DDR3-800 SC */
...

0day reports:

drivers/gpu/drm/i915/intel_pm.c:228:39: warning: cast truncates bits from constant value (f8f becomes 1) 
drivers/gpu/drm/i915/intel_pm.c:228:45: warning: cast truncates bits from constant value (84bf becomes 1) 
drivers/gpu/drm/i915/intel_pm.c:229:39: warning: cast truncates bits from constant value (edf becomes 1) 
drivers/gpu/drm/i915/intel_pm.c:229:45: warning: cast truncates bits from constant value (840f becomes 1) 
drivers/gpu/drm/i915/intel_pm.c:230:39: warning: cast truncates bits from constant value (eb3 becomes 1) 

and so on, This is not surprising since the boolean variables are now at the end of the structure but still initialized at the beginning.
Also see upstream commit c13fb77890965 ("drm/i915: Fix cxsr_latency_table reorg") which should fix the problem.

I would submit a CL to fix the problem, but it appears that no one complained so maybe I am just confused.

 
Yeah we probably didn't notice because that code doesn't run (the cxsr is a memory timing thing on pineview, which we have deprecated, and which didn't even run on 4.4).
Owner: groeck@chromium.org
Status: Assigned (was: Untriaged)
Since the code isn't currently used, I'll apply the upstream patch to fix it and to silence 0day, at the very least making sure that other problems don't disappear in the noise.

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 22

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/12ac457e34df43d377851f564b2a719557e4d77f

commit 12ac457e34df43d377851f564b2a719557e4d77f
Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Date: Thu Nov 22 02:13:19 2018

UPSTREAM: drm/i915: Fix cxsr_latency_table reorg

I have re-ordered some struct members in patch:

  commit 44a655cae3043453f9dd8076538712d52e2e0ce4
  Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
  Date:   Thu Oct 13 11:09:23 2016 +0100

      drm/i915: Shrink cxsr_latency_table

but that particular one is not initialized with named
initializers which broke it.

Move the bitfields back at the beginning. Space saving
is still there.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: 44a655cae304 ("drm/i915: Shrink cxsr_latency_table")
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476453302-7580-1-git-send-email-tvrtko.ursulin@linux.intel.com
(cherry picked from commit c13fb7789096596f7cd5a64b24dbb66116bfc519)

BUG= chromium:906726 
TEST=Run static analyzers

Change-Id: Ifde6b8d11ff39e04ac6ab6e289f5dddd39671a72
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1345489
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/12ac457e34df43d377851f564b2a719557e4d77f/drivers/gpu/drm/i915/intel_drv.h

Status: Fixed (was: Started)

Sign in to add a comment