linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C
@ 2018-04-13  9:40 Thomas Huth
  2018-04-13  9:40 ` [PATCH 1/2] drm: Move CONFIG_HDMI-dependent code to a separate file Thomas Huth
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Thomas Huth @ 2018-04-13  9:40 UTC (permalink / raw)
  To: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie, dri-devel
  Cc: linux-kernel, Gerd Hoffmann, Farhan Ali, borntraeger

By enabling the DRM code for virtio-gpu on S390, you currently also get
all the code that is enabled by CONFIG_HDMI and CONFIG_I2C automatically.
This is quite ugly, since on S390, there is no HDMI and no I2C. Thus it
would be great if the DRM code could also be compiled without CONFIG_HDMI
and CONFIG_I2C. These two patches now refactor the DRM code a little bit
so that we can compile it also without CONFIG_HDMI and CONFIG_I2C.

Thomas Huth (2):
  drivers/gpu/drm: Move CONFIG_HDMI-dependent code to a separate file
  drivers/gpu/drm: Make the DRM code compilable without CONFIG_I2C

 drivers/gpu/drm/Kconfig             |   6 +-
 drivers/gpu/drm/Makefile            |  17 ++--
 drivers/gpu/drm/drm_crtc_internal.h |   2 +
 drivers/gpu/drm/drm_edid.c          | 173 ++--------------------------------
 drivers/gpu/drm/drm_hdmi.c          | 182 ++++++++++++++++++++++++++++++++++++
 5 files changed, 206 insertions(+), 174 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_hdmi.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] drm: Move CONFIG_HDMI-dependent code to a separate file
  2018-04-13  9:40 [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C Thomas Huth
@ 2018-04-13  9:40 ` Thomas Huth
  2018-04-13  9:40 ` [PATCH 2/2] drm: Make the DRM code compilable without CONFIG_I2C Thomas Huth
  2018-04-13 14:32 ` [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C Daniel Vetter
  2 siblings, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2018-04-13  9:40 UTC (permalink / raw)
  To: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie, dri-devel
  Cc: linux-kernel, Gerd Hoffmann, Farhan Ali, borntraeger

Selecting CONFIG_HDMI for S390 is inappropriate - there is no real
graphic hardware on this architecture. The drm subsystem is only
enabled here for using the virtual graphics card "virtio-gpu". So
it should be possible to compile the drm subsystem also without
CONFIG_DRM. Let's move the related code to a separate file for this.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/gpu/drm/Kconfig             |   2 +-
 drivers/gpu/drm/Makefile            |   1 +
 drivers/gpu/drm/drm_crtc_internal.h |   2 +
 drivers/gpu/drm/drm_edid.c          | 163 +-------------------------------
 drivers/gpu/drm/drm_hdmi.c          | 182 ++++++++++++++++++++++++++++++++++++
 5 files changed, 188 insertions(+), 162 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_hdmi.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index deeefa7..298a518 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -8,7 +8,7 @@ menuconfig DRM
 	tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)"
 	depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA
 	select DRM_PANEL_ORIENTATION_QUIRKS
-	select HDMI
+	select HDMI if !S390
 	select FB_CMDLINE
 	select I2C
 	select I2C_ALGOBIT
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 50093ff..16fd8e3 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -20,6 +20,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
 		drm_syncobj.o drm_lease.o
 
+drm-$(CONFIG_HDMI) += drm_hdmi.o
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 3c2b828..0804348 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -220,3 +220,5 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 
 /* drm_edid.c */
 void drm_mode_fixup_1366x768(struct drm_display_mode *mode);
+u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match);
+bool drm_valid_hdmi_vic(u8 vic);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f..3820763 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -29,7 +29,6 @@
  */
 #include <linux/kernel.h>
 #include <linux/slab.h>
-#include <linux/hdmi.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/vga_switcheroo.h>
@@ -3062,7 +3061,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_
  *
  * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
  */
-static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
+u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
 {
 	u8 vic;
 
@@ -3085,7 +3084,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
 	return 0;
 }
 
-static bool drm_valid_hdmi_vic(u8 vic)
+bool drm_valid_hdmi_vic(u8 vic)
 {
 	return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes);
 }
@@ -4817,76 +4816,6 @@ void drm_set_preferred_mode(struct drm_connector *connector,
 EXPORT_SYMBOL(drm_set_preferred_mode);
 
 /**
- * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
- *                                              data from a DRM display mode
- * @frame: HDMI AVI infoframe
- * @mode: DRM display mode
- * @is_hdmi2_sink: Sink is HDMI 2.0 compliant
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int
-drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-					 const struct drm_display_mode *mode,
-					 bool is_hdmi2_sink)
-{
-	int err;
-
-	if (!frame || !mode)
-		return -EINVAL;
-
-	err = hdmi_avi_infoframe_init(frame);
-	if (err < 0)
-		return err;
-
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
-		frame->pixel_repeat = 1;
-
-	frame->video_code = drm_match_cea_mode(mode);
-
-	/*
-	 * HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
-	 * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
-	 * have to make sure we dont break HDMI 1.4 sinks.
-	 */
-	if (!is_hdmi2_sink && frame->video_code > 64)
-		frame->video_code = 0;
-
-	/*
-	 * HDMI spec says if a mode is found in HDMI 1.4b 4K modes
-	 * we should send its VIC in vendor infoframes, else send the
-	 * VIC in AVI infoframes. Lets check if this mode is present in
-	 * HDMI 1.4b 4K modes
-	 */
-	if (frame->video_code) {
-		u8 vendor_if_vic = drm_match_hdmi_mode(mode);
-		bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
-
-		if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
-			frame->video_code = 0;
-	}
-
-	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
-
-	/*
-	 * Populate picture aspect ratio from either
-	 * user input (if specified) or from the CEA mode list.
-	 */
-	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
-		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
-		frame->picture_aspect = mode->picture_aspect_ratio;
-	else if (frame->video_code > 0)
-		frame->picture_aspect = drm_get_cea_aspect_ratio(
-						frame->video_code);
-
-	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
-	frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
-
-	return 0;
-}
-EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
-
-/**
  * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
  *                                        quantization range information
  * @frame: HDMI AVI infoframe
@@ -4945,94 +4874,6 @@ void drm_set_preferred_mode(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_hdmi_avi_infoframe_quant_range);
 
-static enum hdmi_3d_structure
-s3d_structure_from_display_mode(const struct drm_display_mode *mode)
-{
-	u32 layout = mode->flags & DRM_MODE_FLAG_3D_MASK;
-
-	switch (layout) {
-	case DRM_MODE_FLAG_3D_FRAME_PACKING:
-		return HDMI_3D_STRUCTURE_FRAME_PACKING;
-	case DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE:
-		return HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE;
-	case DRM_MODE_FLAG_3D_LINE_ALTERNATIVE:
-		return HDMI_3D_STRUCTURE_LINE_ALTERNATIVE;
-	case DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL:
-		return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL;
-	case DRM_MODE_FLAG_3D_L_DEPTH:
-		return HDMI_3D_STRUCTURE_L_DEPTH;
-	case DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH:
-		return HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH;
-	case DRM_MODE_FLAG_3D_TOP_AND_BOTTOM:
-		return HDMI_3D_STRUCTURE_TOP_AND_BOTTOM;
-	case DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF:
-		return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF;
-	default:
-		return HDMI_3D_STRUCTURE_INVALID;
-	}
-}
-
-/**
- * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe with
- * data from a DRM display mode
- * @frame: HDMI vendor infoframe
- * @connector: the connector
- * @mode: DRM display mode
- *
- * Note that there's is a need to send HDMI vendor infoframes only when using a
- * 4k or stereoscopic 3D mode. So when giving any other mode as input this
- * function will return -EINVAL, error that can be safely ignored.
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int
-drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
-					    struct drm_connector *connector,
-					    const struct drm_display_mode *mode)
-{
-	/*
-	 * FIXME: sil-sii8620 doesn't have a connector around when
-	 * we need one, so we have to be prepared for a NULL connector.
-	 */
-	bool has_hdmi_infoframe = connector ?
-		connector->display_info.has_hdmi_infoframe : false;
-	int err;
-	u32 s3d_flags;
-	u8 vic;
-
-	if (!frame || !mode)
-		return -EINVAL;
-
-	if (!has_hdmi_infoframe)
-		return -EINVAL;
-
-	vic = drm_match_hdmi_mode(mode);
-	s3d_flags = mode->flags & DRM_MODE_FLAG_3D_MASK;
-
-	/*
-	 * Even if it's not absolutely necessary to send the infoframe
-	 * (ie.vic==0 and s3d_struct==0) we will still send it if we
-	 * know that the sink can handle it. This is based on a
-	 * suggestion in HDMI 2.0 Appendix F. Apparently some sinks
-	 * have trouble realizing that they shuld switch from 3D to 2D
-	 * mode if the source simply stops sending the infoframe when
-	 * it wants to switch from 3D to 2D.
-	 */
-
-	if (vic && s3d_flags)
-		return -EINVAL;
-
-	err = hdmi_vendor_infoframe_init(frame);
-	if (err < 0)
-		return err;
-
-	frame->vic = vic;
-	frame->s3d_struct = s3d_structure_from_display_mode(mode);
-
-	return 0;
-}
-EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
-
 static int drm_parse_tiled_block(struct drm_connector *connector,
 				 struct displayid_block *block)
 {
diff --git a/drivers/gpu/drm/drm_hdmi.c b/drivers/gpu/drm/drm_hdmi.c
new file mode 100644
index 0000000..9ddf538
--- /dev/null
+++ b/drivers/gpu/drm/drm_hdmi.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright (c) 2006 Luc Verhaegen (quirks list)
+ * Copyright (c) 2007-2008 Intel Corporation
+ *   Jesse Barnes <jesse.barnes@intel.com>
+ * Copyright 2010 Red Hat, Inc.
+ *
+ * DDC probing routines (drm_ddc_read & drm_do_probe_ddc_edid) originally from
+ * FB layer.
+ *   Copyright (C) 2006 Dennis Munsie <dmunsie@cecropia.com>
+ */
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/hdmi.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/vga_switcheroo.h>
+#include <drm/drmP.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_displayid.h>
+#include <drm/drm_scdc_helper.h>
+
+#include "drm_crtc_internal.h"
+
+/**
+ * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
+ *                                              data from a DRM display mode
+ * @frame: HDMI AVI infoframe
+ * @mode: DRM display mode
+ * @is_hdmi2_sink: Sink is HDMI 2.0 compliant
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
+					 const struct drm_display_mode *mode,
+					 bool is_hdmi2_sink)
+{
+	int err;
+
+	if (!frame || !mode)
+		return -EINVAL;
+
+	err = hdmi_avi_infoframe_init(frame);
+	if (err < 0)
+		return err;
+
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		frame->pixel_repeat = 1;
+
+	frame->video_code = drm_match_cea_mode(mode);
+
+	/*
+	 * HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
+	 * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
+	 * have to make sure we dont break HDMI 1.4 sinks.
+	 */
+	if (!is_hdmi2_sink && frame->video_code > 64)
+		frame->video_code = 0;
+
+	/*
+	 * HDMI spec says if a mode is found in HDMI 1.4b 4K modes
+	 * we should send its VIC in vendor infoframes, else send the
+	 * VIC in AVI infoframes. Lets check if this mode is present in
+	 * HDMI 1.4b 4K modes
+	 */
+	if (frame->video_code) {
+		u8 vendor_if_vic = drm_match_hdmi_mode(mode);
+		bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
+
+		if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
+			frame->video_code = 0;
+	}
+
+	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
+
+	/*
+	 * Populate picture aspect ratio from either
+	 * user input (if specified) or from the CEA mode list.
+	 */
+	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
+		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
+		frame->picture_aspect = mode->picture_aspect_ratio;
+	else if (frame->video_code > 0)
+		frame->picture_aspect = drm_get_cea_aspect_ratio(
+						frame->video_code);
+
+	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
+	frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
+
+static enum hdmi_3d_structure
+s3d_structure_from_display_mode(const struct drm_display_mode *mode)
+{
+	u32 layout = mode->flags & DRM_MODE_FLAG_3D_MASK;
+
+	switch (layout) {
+	case DRM_MODE_FLAG_3D_FRAME_PACKING:
+		return HDMI_3D_STRUCTURE_FRAME_PACKING;
+	case DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE:
+		return HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE;
+	case DRM_MODE_FLAG_3D_LINE_ALTERNATIVE:
+		return HDMI_3D_STRUCTURE_LINE_ALTERNATIVE;
+	case DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL:
+		return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL;
+	case DRM_MODE_FLAG_3D_L_DEPTH:
+		return HDMI_3D_STRUCTURE_L_DEPTH;
+	case DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH:
+		return HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH;
+	case DRM_MODE_FLAG_3D_TOP_AND_BOTTOM:
+		return HDMI_3D_STRUCTURE_TOP_AND_BOTTOM;
+	case DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF:
+		return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF;
+	default:
+		return HDMI_3D_STRUCTURE_INVALID;
+	}
+}
+
+/**
+ * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe with
+ * data from a DRM display mode
+ * @frame: HDMI vendor infoframe
+ * @connector: the connector
+ * @mode: DRM display mode
+ *
+ * Note that there's is a need to send HDMI vendor infoframes only when using a
+ * 4k or stereoscopic 3D mode. So when giving any other mode as input this
+ * function will return -EINVAL, error that can be safely ignored.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
+					    struct drm_connector *connector,
+					    const struct drm_display_mode *mode)
+{
+	/*
+	 * FIXME: sil-sii8620 doesn't have a connector around when
+	 * we need one, so we have to be prepared for a NULL connector.
+	 */
+	bool has_hdmi_infoframe = connector ?
+		connector->display_info.has_hdmi_infoframe : false;
+	int err;
+	u32 s3d_flags;
+	u8 vic;
+
+	if (!frame || !mode)
+		return -EINVAL;
+
+	if (!has_hdmi_infoframe)
+		return -EINVAL;
+
+	vic = drm_match_hdmi_mode(mode);
+	s3d_flags = mode->flags & DRM_MODE_FLAG_3D_MASK;
+
+	/*
+	 * Even if it's not absolutely necessary to send the infoframe
+	 * (ie.vic==0 and s3d_struct==0) we will still send it if we
+	 * know that the sink can handle it. This is based on a
+	 * suggestion in HDMI 2.0 Appendix F. Apparently some sinks
+	 * have trouble realizing that they shuld switch from 3D to 2D
+	 * mode if the source simply stops sending the infoframe when
+	 * it wants to switch from 3D to 2D.
+	 */
+
+	if (vic && s3d_flags)
+		return -EINVAL;
+
+	err = hdmi_vendor_infoframe_init(frame);
+	if (err < 0)
+		return err;
+
+	frame->vic = vic;
+	frame->s3d_struct = s3d_structure_from_display_mode(mode);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] drm: Make the DRM code compilable without CONFIG_I2C
  2018-04-13  9:40 [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C Thomas Huth
  2018-04-13  9:40 ` [PATCH 1/2] drm: Move CONFIG_HDMI-dependent code to a separate file Thomas Huth
@ 2018-04-13  9:40 ` Thomas Huth
  2018-04-13 13:04   ` Jani Nikula
  2018-04-13 14:32 ` [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C Daniel Vetter
  2 siblings, 1 reply; 7+ messages in thread
From: Thomas Huth @ 2018-04-13  9:40 UTC (permalink / raw)
  To: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie, dri-devel
  Cc: linux-kernel, Gerd Hoffmann, Farhan Ali, borntraeger

Selecting CONFIG_HDMI for S390 is inappropriate - there is no real
graphic hardware on this architecture. The drm subsystem is only
enabled here for using the virtual graphics card "virtio-gpu". So
it should be possible to compile the drm subsystem also without
CONFIG_I2C. Tweak the Makefile to only compile the affected files
with CONFIG_I2C=y and disable some I2C-related code in drm_edid.c.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/gpu/drm/Kconfig    |  4 ++--
 drivers/gpu/drm/Makefile   | 16 +++++++++-------
 drivers/gpu/drm/drm_edid.c | 10 +++++++---
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 298a518..c9df67f 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -10,8 +10,8 @@ menuconfig DRM
 	select DRM_PANEL_ORIENTATION_QUIRKS
 	select HDMI if !S390
 	select FB_CMDLINE
-	select I2C
-	select I2C_ALGOBIT
+	select I2C if !S390
+	select I2C_ALGOBIT if !S390
 	select DMA_SHARED_BUFFER
 	select SYNC_FILE
 	help
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 16fd8e3..4cf53ba 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -7,10 +7,9 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_context.o drm_dma.o \
 		drm_file.o drm_gem.o drm_ioctl.o drm_irq.o \
 		drm_lock.o drm_memory.o drm_drv.o \
-		drm_scatter.o drm_pci.o \
+		drm_scatter.o drm_pci.o drm_info.o \
 		drm_sysfs.o drm_hashtab.o drm_mm.o \
 		drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
-		drm_info.o drm_encoder_slave.o \
 		drm_trace_points.o drm_global.o drm_prime.o \
 		drm_rect.o drm_vma_manager.o drm_flip_work.o \
 		drm_modeset_lock.o drm_atomic.o drm_bridge.o \
@@ -20,6 +19,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
 		drm_syncobj.o drm_lease.o
 
+drm-$(CONFIG_I2C) += drm_encoder_slave.o
 drm-$(CONFIG_HDMI) += drm_hdmi.o
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
@@ -32,11 +32,13 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
 drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
 drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
 
-drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
-		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
-		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
-		drm_simple_kms_helper.o drm_modeset_helper.o \
-		drm_scdc_helper.o drm_gem_framebuffer_helper.o
+drm_kms_helper-y := drm_crtc_helper.o drm_probe_helper.o \
+		drm_plane_helper.o drm_atomic_helper.o \
+		drm_gem_framebuffer_helper.o drm_kms_helper_common.o \
+		drm_simple_kms_helper.o drm_modeset_helper.o
+
+drm_kms_helper-$(CONFIG_I2C) += drm_dp_helper.o drm_dp_mst_topology.o \
+		drm_dp_dual_mode_helper.o drm_scdc_helper.o
 
 drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
 drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3820763..0d0d5af 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1467,11 +1467,14 @@ bool drm_edid_is_valid(struct edid *edid)
 static int
 drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len)
 {
-	struct i2c_adapter *adapter = data;
-	unsigned char start = block * EDID_LENGTH;
 	unsigned char segment = block >> 1;
 	unsigned char xfers = segment ? 3 : 2;
-	int ret, retries = 5;
+	int ret = -1;
+
+#if IS_ENABLED(CONFIG_I2C)
+	struct i2c_adapter *adapter = data;
+	unsigned char start = block * EDID_LENGTH;
+	int retries = 5;
 
 	/*
 	 * The core I2C driver will automatically retry the transfer if the
@@ -1512,6 +1515,7 @@ bool drm_edid_is_valid(struct edid *edid)
 			break;
 		}
 	} while (ret != xfers && --retries);
+#endif	/* CONFIG_I2C */
 
 	return ret == xfers ? 0 : -1;
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm: Make the DRM code compilable without CONFIG_I2C
  2018-04-13  9:40 ` [PATCH 2/2] drm: Make the DRM code compilable without CONFIG_I2C Thomas Huth
@ 2018-04-13 13:04   ` Jani Nikula
  0 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2018-04-13 13:04 UTC (permalink / raw)
  To: Thomas Huth, Gustavo Padovan, Maarten Lankhorst, Sean Paul,
	David Airlie, dri-devel
  Cc: borntraeger, Farhan Ali, linux-kernel, Gerd Hoffmann

On Fri, 13 Apr 2018, Thomas Huth <thuth@redhat.com> wrote:
> Selecting CONFIG_HDMI for S390 is inappropriate - there is no real
> graphic hardware on this architecture. The drm subsystem is only
> enabled here for using the virtual graphics card "virtio-gpu". So
> it should be possible to compile the drm subsystem also without
> CONFIG_I2C. Tweak the Makefile to only compile the affected files
> with CONFIG_I2C=y and disable some I2C-related code in drm_edid.c.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  drivers/gpu/drm/Kconfig    |  4 ++--
>  drivers/gpu/drm/Makefile   | 16 +++++++++-------
>  drivers/gpu/drm/drm_edid.c | 10 +++++++---
>  3 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 298a518..c9df67f 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -10,8 +10,8 @@ menuconfig DRM
>  	select DRM_PANEL_ORIENTATION_QUIRKS
>  	select HDMI if !S390
>  	select FB_CMDLINE
> -	select I2C
> -	select I2C_ALGOBIT
> +	select I2C if !S390
> +	select I2C_ALGOBIT if !S390
>  	select DMA_SHARED_BUFFER
>  	select SYNC_FILE
>  	help
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 16fd8e3..4cf53ba 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -7,10 +7,9 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
>  		drm_context.o drm_dma.o \
>  		drm_file.o drm_gem.o drm_ioctl.o drm_irq.o \
>  		drm_lock.o drm_memory.o drm_drv.o \
> -		drm_scatter.o drm_pci.o \
> +		drm_scatter.o drm_pci.o drm_info.o \
>  		drm_sysfs.o drm_hashtab.o drm_mm.o \
>  		drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
> -		drm_info.o drm_encoder_slave.o \
>  		drm_trace_points.o drm_global.o drm_prime.o \
>  		drm_rect.o drm_vma_manager.o drm_flip_work.o \
>  		drm_modeset_lock.o drm_atomic.o drm_bridge.o \
> @@ -20,6 +19,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
>  		drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
>  		drm_syncobj.o drm_lease.o
>  
> +drm-$(CONFIG_I2C) += drm_encoder_slave.o
>  drm-$(CONFIG_HDMI) += drm_hdmi.o
>  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>  drm-$(CONFIG_DRM_VM) += drm_vm.o
> @@ -32,11 +32,13 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
>  drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
>  drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>  
> -drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
> -		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
> -		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
> -		drm_simple_kms_helper.o drm_modeset_helper.o \
> -		drm_scdc_helper.o drm_gem_framebuffer_helper.o
> +drm_kms_helper-y := drm_crtc_helper.o drm_probe_helper.o \
> +		drm_plane_helper.o drm_atomic_helper.o \
> +		drm_gem_framebuffer_helper.o drm_kms_helper_common.o \
> +		drm_simple_kms_helper.o drm_modeset_helper.o
> +
> +drm_kms_helper-$(CONFIG_I2C) += drm_dp_helper.o drm_dp_mst_topology.o \
> +		drm_dp_dual_mode_helper.o drm_scdc_helper.o
>  
>  drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3820763..0d0d5af 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1467,11 +1467,14 @@ bool drm_edid_is_valid(struct edid *edid)
>  static int
>  drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len)
>  {
> -	struct i2c_adapter *adapter = data;
> -	unsigned char start = block * EDID_LENGTH;
>  	unsigned char segment = block >> 1;
>  	unsigned char xfers = segment ? 3 : 2;
> -	int ret, retries = 5;
> +	int ret = -1;
> +
> +#if IS_ENABLED(CONFIG_I2C)

So I don't know if the patches are worth it, I'll let others decide, but
no matter what the conditional build needs to be *outside* of function
bodies. The #else branch would then contain just:

#else
static inline int
drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len)
{
        return -1;
}
#endif /* CONFIG_I2C */


BR,
Jani.

> +	struct i2c_adapter *adapter = data;
> +	unsigned char start = block * EDID_LENGTH;
> +	int retries = 5;
>  
>  	/*
>  	 * The core I2C driver will automatically retry the transfer if the
> @@ -1512,6 +1515,7 @@ bool drm_edid_is_valid(struct edid *edid)
>  			break;
>  		}
>  	} while (ret != xfers && --retries);
> +#endif	/* CONFIG_I2C */
>  
>  	return ret == xfers ? 0 : -1;
>  }

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C
  2018-04-13  9:40 [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C Thomas Huth
  2018-04-13  9:40 ` [PATCH 1/2] drm: Move CONFIG_HDMI-dependent code to a separate file Thomas Huth
  2018-04-13  9:40 ` [PATCH 2/2] drm: Make the DRM code compilable without CONFIG_I2C Thomas Huth
@ 2018-04-13 14:32 ` Daniel Vetter
  2018-04-13 14:46   ` Thomas Huth
  2 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2018-04-13 14:32 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie,
	dri-devel, borntraeger, Farhan Ali, Linux Kernel Mailing List,
	Gerd Hoffmann

On Fri, Apr 13, 2018 at 11:40 AM, Thomas Huth <thuth@redhat.com> wrote:
> By enabling the DRM code for virtio-gpu on S390, you currently also get
> all the code that is enabled by CONFIG_HDMI and CONFIG_I2C automatically.
> This is quite ugly, since on S390, there is no HDMI and no I2C. Thus it
> would be great if the DRM code could also be compiled without CONFIG_HDMI
> and CONFIG_I2C. These two patches now refactor the DRM code a little bit
> so that we can compile it also without CONFIG_HDMI and CONFIG_I2C.
>
> Thomas Huth (2):
>   drivers/gpu/drm: Move CONFIG_HDMI-dependent code to a separate file
>   drivers/gpu/drm: Make the DRM code compilable without CONFIG_I2C

What's the benefit? Why does I2C/HDMI hurt you?

Note that you still can't compile out DP code, and the DRM legacy
code, and that's much bigger ...
-Daniel

>
>  drivers/gpu/drm/Kconfig             |   6 +-
>  drivers/gpu/drm/Makefile            |  17 ++--
>  drivers/gpu/drm/drm_crtc_internal.h |   2 +
>  drivers/gpu/drm/drm_edid.c          | 173 ++--------------------------------
>  drivers/gpu/drm/drm_hdmi.c          | 182 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 206 insertions(+), 174 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_hdmi.c
>
> --
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C
  2018-04-13 14:32 ` [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C Daniel Vetter
@ 2018-04-13 14:46   ` Thomas Huth
  2018-04-13 14:53     ` Daniel Vetter
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Huth @ 2018-04-13 14:46 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie,
	dri-devel, borntraeger, Farhan Ali, Linux Kernel Mailing List,
	Gerd Hoffmann

On 13.04.2018 16:32, Daniel Vetter wrote:
> On Fri, Apr 13, 2018 at 11:40 AM, Thomas Huth <thuth@redhat.com> wrote:
>> By enabling the DRM code for virtio-gpu on S390, you currently also get
>> all the code that is enabled by CONFIG_HDMI and CONFIG_I2C automatically.
>> This is quite ugly, since on S390, there is no HDMI and no I2C. Thus it
>> would be great if the DRM code could also be compiled without CONFIG_HDMI
>> and CONFIG_I2C. These two patches now refactor the DRM code a little bit
>> so that we can compile it also without CONFIG_HDMI and CONFIG_I2C.
>>
>> Thomas Huth (2):
>>   drivers/gpu/drm: Move CONFIG_HDMI-dependent code to a separate file
>>   drivers/gpu/drm: Make the DRM code compilable without CONFIG_I2C
> 
> What's the benefit? Why does I2C/HDMI hurt you?

Why should I be forced to compile-in subsystems that do not make any
sense on this architecture? It's just completely weird to see CONFIG_I2C
enabled on s390x.

 Thomas

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C
  2018-04-13 14:46   ` Thomas Huth
@ 2018-04-13 14:53     ` Daniel Vetter
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2018-04-13 14:53 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie,
	dri-devel, borntraeger, Farhan Ali, Linux Kernel Mailing List,
	Gerd Hoffmann

On Fri, Apr 13, 2018 at 4:46 PM, Thomas Huth <thuth@redhat.com> wrote:
> On 13.04.2018 16:32, Daniel Vetter wrote:
>> On Fri, Apr 13, 2018 at 11:40 AM, Thomas Huth <thuth@redhat.com> wrote:
>>> By enabling the DRM code for virtio-gpu on S390, you currently also get
>>> all the code that is enabled by CONFIG_HDMI and CONFIG_I2C automatically.
>>> This is quite ugly, since on S390, there is no HDMI and no I2C. Thus it
>>> would be great if the DRM code could also be compiled without CONFIG_HDMI
>>> and CONFIG_I2C. These two patches now refactor the DRM code a little bit
>>> so that we can compile it also without CONFIG_HDMI and CONFIG_I2C.
>>>
>>> Thomas Huth (2):
>>>   drivers/gpu/drm: Move CONFIG_HDMI-dependent code to a separate file
>>>   drivers/gpu/drm: Make the DRM code compilable without CONFIG_I2C
>>
>> What's the benefit? Why does I2C/HDMI hurt you?
>
> Why should I be forced to compile-in subsystems that do not make any
> sense on this architecture? It's just completely weird to see CONFIG_I2C
> enabled on s390x.

"Looks wierd" is not really a good engineering criteria, especially in
graphics :-)

For context: In DRM almost nothing is optional, and it greatly
simplifies life and coding. We don't have epic amounts of #ifdef
battles to make trivial code changes compile, except in all the places
where external stuff is optional (like backlight).

So making something optional will have a pretty clear cost on the drm
subsystem, and it doesn't make sense to pay that cost to "look less
wierd". To get this merged we need some clear benefits, which will
balance out the inevitable cost of having to maintain this forever
(and most likely getting yelled at by Linus for making some rando
compile config no longer work).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-04-13 14:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-13  9:40 [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C Thomas Huth
2018-04-13  9:40 ` [PATCH 1/2] drm: Move CONFIG_HDMI-dependent code to a separate file Thomas Huth
2018-04-13  9:40 ` [PATCH 2/2] drm: Make the DRM code compilable without CONFIG_I2C Thomas Huth
2018-04-13 13:04   ` Jani Nikula
2018-04-13 14:32 ` [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C Daniel Vetter
2018-04-13 14:46   ` Thomas Huth
2018-04-13 14:53     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).