Current implementation of mode_valid() and mode_fixup() callbacks
handle packed pixel modes improperly.
Fix it by using proper maximum clock values from the documentation.
Signed-off-by: Maciej Purski <[email protected]>
---
drivers/gpu/drm/bridge/sil-sii8620.c | 90 ++++++++++++++++++++----------------
1 file changed, 51 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 5168783..1718c2e 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -34,8 +34,11 @@
#define SII8620_BURST_BUF_LEN 288
#define VAL_RX_HDMI_CTRL2_DEFVAL VAL_RX_HDMI_CTRL2_IDLE_CNT(3)
-#define MHL1_MAX_LCLK 225000
-#define MHL3_MAX_LCLK 600000
+
+#define MHL1_MAX_PCLK 75000
+#define MHL1_MAX_PCLK_PP_MODE 150000
+#define MHL3_MAX_PCLK 200000
+#define MHL3_MAX_PCLK_PP_MODE 300000
enum sii8620_mode {
CM_DISCONNECTED,
@@ -2123,61 +2126,70 @@ static void sii8620_detach(struct drm_bridge *bridge)
rc_unregister_device(ctx->rc_dev);
}
+static int sii8620_is_packing_required(struct sii8620 *ctx,
+ const struct drm_display_mode *mode)
+{
+ int ret;
+
+ if (sii8620_is_mhl3(ctx)) {
+ if (mode->clock < MHL3_MAX_PCLK)
+ ret = 0;
+ else if (mode->clock < MHL3_MAX_PCLK_PP_MODE)
+ ret = 1;
+ else
+ ret = -1;
+ } else {
+ if (mode->clock < MHL1_MAX_PCLK)
+ ret = 0;
+ else if (mode->clock < MHL1_MAX_PCLK_PP_MODE)
+ ret = 1;
+ else
+ ret = -1;
+ }
+
+ return ret;
+}
+
static enum drm_mode_status sii8620_mode_valid(struct drm_bridge *bridge,
const struct drm_display_mode *mode)
{
+ enum drm_mode_status ret;
struct sii8620 *ctx = bridge_to_sii8620(bridge);
+ int pack_required = sii8620_is_packing_required(ctx, mode);
bool can_pack = ctx->devcap[MHL_DCAP_VID_LINK_MODE] &
MHL_DCAP_VID_LINK_PPIXEL;
- unsigned int max_pclk = sii8620_is_mhl3(ctx) ? MHL3_MAX_LCLK :
- MHL1_MAX_LCLK;
- max_pclk /= can_pack ? 2 : 3;
- return (mode->clock > max_pclk) ? MODE_CLOCK_HIGH : MODE_OK;
+ switch (pack_required) {
+ case -1:
+ ret = MODE_CLOCK_HIGH;
+ break;
+ case 1:
+ ret = (can_pack) ? MODE_OK : MODE_CLOCK_HIGH;
+ break;
+ case 0:
+ ret = MODE_OK;
+ break;
+ }
+
+ return ret;
}
+
static bool sii8620_mode_fixup(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
{
struct sii8620 *ctx = bridge_to_sii8620(bridge);
- int max_lclk;
- bool ret = true;
mutex_lock(&ctx->lock);
- max_lclk = sii8620_is_mhl3(ctx) ? MHL3_MAX_LCLK : MHL1_MAX_LCLK;
- if (max_lclk > 3 * adjusted_mode->clock) {
- ctx->use_packed_pixel = 0;
- goto end;
- }
- if ((ctx->devcap[MHL_DCAP_VID_LINK_MODE] & MHL_DCAP_VID_LINK_PPIXEL) &&
- max_lclk > 2 * adjusted_mode->clock) {
- ctx->use_packed_pixel = 1;
- goto end;
- }
- ret = false;
-end:
- if (ret) {
- u8 vic = drm_match_cea_mode(adjusted_mode);
-
- if (!vic) {
- union hdmi_infoframe frm;
- u8 mhl_vic[] = { 0, 95, 94, 93, 98 };
-
- /* FIXME: We need the connector here */
- drm_hdmi_vendor_infoframe_from_display_mode(
- &frm.vendor.hdmi, NULL, adjusted_mode);
- vic = frm.vendor.hdmi.vic;
- if (vic >= ARRAY_SIZE(mhl_vic))
- vic = 0;
- vic = mhl_vic[vic];
- }
- ctx->video_code = vic;
- ctx->pixel_clock = adjusted_mode->clock;
- }
+ ctx->use_packed_pixel = sii8620_is_packing_required(ctx, adjusted_mode);
+ ctx->video_code = drm_match_cea_mode(adjusted_mode);
+ ctx->pixel_clock = adjusted_mode->clock;
+
mutex_unlock(&ctx->lock);
- return ret;
+
+ return true;
}
static const struct drm_bridge_funcs sii8620_bridge_funcs = {
--
2.7.4
Hi Maciej,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next]
[also build test WARNING on next-20180119]
[cannot apply to v4.15-rc9]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Maciej-Purski/drm-bridge-sii8620-fix-display-modes-validation/20180125-174703
base: git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-randconfig-sb0-01260635 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/gpu/drm/bridge/sil-sii8620.c: In function 'sii8620_mode_valid':
>> drivers/gpu/drm/bridge/sil-sii8620.c:2242:2: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
return ret;
^
vim +/ret +2242 drivers/gpu/drm/bridge/sil-sii8620.c
2220
2221 static enum drm_mode_status sii8620_mode_valid(struct drm_bridge *bridge,
2222 const struct drm_display_mode *mode)
2223 {
2224 enum drm_mode_status ret;
2225 struct sii8620 *ctx = bridge_to_sii8620(bridge);
2226 int pack_required = sii8620_is_packing_required(ctx, mode);
2227 bool can_pack = ctx->devcap[MHL_DCAP_VID_LINK_MODE] &
2228 MHL_DCAP_VID_LINK_PPIXEL;
2229
2230 switch (pack_required) {
2231 case -1:
2232 ret = MODE_CLOCK_HIGH;
2233 break;
2234 case 1:
2235 ret = (can_pack) ? MODE_OK : MODE_CLOCK_HIGH;
2236 break;
2237 case 0:
2238 ret = MODE_OK;
2239 break;
2240 }
2241
> 2242 return ret;
2243 }
2244
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation