Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
On Thu, May 19, 2022 at 09:26:59AM -0700, Guillaume Ranquet wrote:
> On Thu, 12 May 2022 09:44, Maxime Ripard wrote:
> >Hi,
> >
> >On Wed, May 11, 2022 at 05:59:13AM -0700, Guillaume Ranquet wrote:
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +#include
> >> >> +
> >> >> +#include "mtk_dp_reg.h"
> >> >> +
> >> >> +#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
> >> >> +#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
> >> >> +
> >> >> +//TODO: platform/device data or dts?
> >> >
> >> >DTS :)
> >>
> >> It's probably going to be a platform_data struct for v10...
> >> If I have time, I'll change it to a dts property for v10.
> >
> >I can't really imagine a case where we would need platform_data
> >nowadays. If you have a device tree, then it should be part of the
> >binding.
> >
> >What issue would you like to address by using a platform_data?
> >
>
> Ok, I'll migrate to dt then. I didn't realize platform_data were depreciated.
>
> Angelo wants the MAX_LINRATE and MAX_LANES defines to be configurable.
> I imagined platform_data would be more appropriate as (per my understanding)
> the
> limitation is associated with a specific SoC.
The entire device tree is nothing but a collection of data associated to
a specific SoC though :)
> >> >> +static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
> >> >> + struct drm_connector *connector)
> >> >> +{
> >> >> + struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> >> >> + bool enabled = mtk_dp->enabled;
> >> >> + struct edid *new_edid = NULL;
> >> >> +
> >> >> + if (!enabled)
> >> >> + drm_bridge_chain_pre_enable(bridge);
> >> >> +
> >> >> + drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
> >> >> + usleep_range(2000, 5000);
> >> >> +
> >> >> + if (mtk_dp_plug_state(mtk_dp))
> >> >> + new_edid = drm_get_edid(connector, _dp->aux.ddc);
> >> >> +
> >> >> + if (!enabled)
> >> >> + drm_bridge_chain_post_disable(bridge);
> >> >
> >> >Are you sure we can't get a mode set while get_edid is called?
> >> >
> >> >If we can, then you could end up disabling the device while it's being
> >> >powered on.
> >>
> >> I'm a bit unsure, I need to spend more time in the drm stack to make sure.
> >> I'll get back to you when I have a definitive answer.
> >
> >So, it looks like it's ok.
> >
> >get_edid is your implementation of get_modes, which is called by
> >drm_helper_probe_single_connector_modes
> >
> >https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_probe_helper.c#L416
> >
> >This is the standard implemantion of fill_modes, which is called
> >whenever the get_connector ioctl is called (or similar paths, like
> >drm_client_modeset_probe)
> >
> >drm_helper_probe_single_connector_modes is under the assumption that the
> >mode_config.mutex is held though, and that the big lock. So it should be
> >serialized there.
> >
> >Just for future proofing though, it would be better to use refcounting
> >there. Would runtime_pm work for you there?
> >
>
> Thx for looking into this for me.
> Not sure runtime_pm works here as it would only refcount if compiled
> with CONFIG_PM?
It should be enabled in most configurations these days, and you can
always depend on it in your Kconfig option.
> I'd rather use the enabled field as a refcounter instead of a boolean.
It's a bit more ad-hoc, but that would work too. Make sure to use a lock
or atomic operations though
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
On Thu, 12 May 2022 09:44, Maxime Ripard wrote:
>Hi,
>
>On Wed, May 11, 2022 at 05:59:13AM -0700, Guillaume Ranquet wrote:
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +#include
>> >> +
>> >> +#include "mtk_dp_reg.h"
>> >> +
>> >> +#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
>> >> +#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
>> >> +
>> >> +//TODO: platform/device data or dts?
>> >
>> >DTS :)
>>
>> It's probably going to be a platform_data struct for v10...
>> If I have time, I'll change it to a dts property for v10.
>
>I can't really imagine a case where we would need platform_data
>nowadays. If you have a device tree, then it should be part of the
>binding.
>
>What issue would you like to address by using a platform_data?
>
Ok, I'll migrate to dt then. I didn't realize platform_data were depreciated.
Angelo wants the MAX_LINRATE and MAX_LANES defines to be configurable.
I imagined platform_data would be more appropriate as (per my understanding) the
limitation is associated with a specific SoC.
>> >> +static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge
>> >> *bridge)
>> >> +{
>> >> + return connector_status_connected;
>> >> +}
>> >
>> >I'm not quite sure what's going on there. You seem to have some support
>> >for HPD interrupts above, but you always report the display as
>> >connected?
>> >
>> >I'd assume that either you don't have HPD support and then always report
>> >it as connected, or you have HPD support and report the current status
>> >in detect, but that combination seems weird.
>>
>> The HPD logic needs more work, some things have been broken when I split
>> the driver into three patches eDP - DP - Audio
>> The assumption at first was that eDP didn't need any HPD handling... but it
>> seems I was wrong and the eDP driver needs to be reworked.
>
>That can be made into a patch of its own if you prefer.
>
>You first introduce the driver without status reporting (always
>returning connected or unknown), and then add the needed bits for HPD.
>
>However, that first patch shouldn't contain the interrupt plumbing and
>so on, it's just confusing.
>
After discussing a while with Mediatek, it appears that hot plug detection
needs to be handled even for eDP... (which is always connected).
So I'll revert to the split I made earlier in v8 where hot plug detection was
part of the eDP patch the addition of the External display port was a
trivial patch [1].
>> >> +static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
>> >> + struct drm_connector *connector)
>> >> +{
>> >> + struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
>> >> + bool enabled = mtk_dp->enabled;
>> >> + struct edid *new_edid = NULL;
>> >> +
>> >> + if (!enabled)
>> >> + drm_bridge_chain_pre_enable(bridge);
>> >> +
>> >> + drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
>> >> + usleep_range(2000, 5000);
>> >> +
>> >> + if (mtk_dp_plug_state(mtk_dp))
>> >> + new_edid = drm_get_edid(connector, _dp->aux.ddc);
>> >> +
>> >> + if (!enabled)
>> >> + drm_bridge_chain_post_disable(bridge);
>> >
>> >Are you sure we can't get a mode set while get_edid is called?
>> >
>> >If we can, then you could end up disabling the device while it's being
>> >powered on.
>>
>> I'm a bit unsure, I need to spend more time in the drm stack to make sure.
>> I'll get back to you when I have a definitive answer.
>
>So, it looks like it's ok.
>
>get_edid is your implementation of get_modes, which is called by
>drm_helper_probe_single_connector_modes
>
>https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_probe_helper.c#L416
>
>This is the standard implemantion of fill_modes, which is called
>whenever the get_connector ioctl is called (or similar paths, like
>drm_client_modeset_probe)
>
>drm_helper_probe_single_connector_modes is under the assumption that the
>mode_config.mutex is held though, and that the big lock. So it should be
>serialized there.
>
>Just for future proofing though, it would be better to use refcounting
>there. Would runtime_pm work for you there?
>
Thx for looking into this for me.
Not sure runtime_pm works here as it would only refcount if compiled
with CONFIG_PM?
I'd rather use the enabled field as a refcounter instead of a boolean.
Unless I'm totally missing your point?
Thx,
Guillaume.
>> >> +static void mtk_dp_parse_drm_mode_timings(struct mtk_dp *mtk_dp,
>> >> + struct drm_display_mode *mode)
>> >> +{
>> >> + struct mtk_dp_timings *timings = _dp->info.timings;
>> >> +
>> >> + drm_display_mode_to_videomode(mode, >vm);
>> >> +
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
Hi,
On Wed, May 11, 2022 at 05:59:13AM -0700, Guillaume Ranquet wrote:
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#include "mtk_dp_reg.h"
> >> +
> >> +#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
> >> +#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
> >> +
> >> +//TODO: platform/device data or dts?
> >
> >DTS :)
>
> It's probably going to be a platform_data struct for v10...
> If I have time, I'll change it to a dts property for v10.
I can't really imagine a case where we would need platform_data
nowadays. If you have a device tree, then it should be part of the
binding.
What issue would you like to address by using a platform_data?
> >> +static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge
> >> *bridge)
> >> +{
> >> + return connector_status_connected;
> >> +}
> >
> >I'm not quite sure what's going on there. You seem to have some support
> >for HPD interrupts above, but you always report the display as
> >connected?
> >
> >I'd assume that either you don't have HPD support and then always report
> >it as connected, or you have HPD support and report the current status
> >in detect, but that combination seems weird.
>
> The HPD logic needs more work, some things have been broken when I split
> the driver into three patches eDP - DP - Audio
> The assumption at first was that eDP didn't need any HPD handling... but it
> seems I was wrong and the eDP driver needs to be reworked.
That can be made into a patch of its own if you prefer.
You first introduce the driver without status reporting (always
returning connected or unknown), and then add the needed bits for HPD.
However, that first patch shouldn't contain the interrupt plumbing and
so on, it's just confusing.
> >> +static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
> >> + struct drm_connector *connector)
> >> +{
> >> + struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> >> + bool enabled = mtk_dp->enabled;
> >> + struct edid *new_edid = NULL;
> >> +
> >> + if (!enabled)
> >> + drm_bridge_chain_pre_enable(bridge);
> >> +
> >> + drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
> >> + usleep_range(2000, 5000);
> >> +
> >> + if (mtk_dp_plug_state(mtk_dp))
> >> + new_edid = drm_get_edid(connector, _dp->aux.ddc);
> >> +
> >> + if (!enabled)
> >> + drm_bridge_chain_post_disable(bridge);
> >
> >Are you sure we can't get a mode set while get_edid is called?
> >
> >If we can, then you could end up disabling the device while it's being
> >powered on.
>
> I'm a bit unsure, I need to spend more time in the drm stack to make sure.
> I'll get back to you when I have a definitive answer.
So, it looks like it's ok.
get_edid is your implementation of get_modes, which is called by
drm_helper_probe_single_connector_modes
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_probe_helper.c#L416
This is the standard implemantion of fill_modes, which is called
whenever the get_connector ioctl is called (or similar paths, like
drm_client_modeset_probe)
drm_helper_probe_single_connector_modes is under the assumption that the
mode_config.mutex is held though, and that the big lock. So it should be
serialized there.
Just for future proofing though, it would be better to use refcounting
there. Would runtime_pm work for you there?
> >> +static void mtk_dp_parse_drm_mode_timings(struct mtk_dp *mtk_dp,
> >> +struct drm_display_mode *mode)
> >> +{
> >> + struct mtk_dp_timings *timings = _dp->info.timings;
> >> +
> >> + drm_display_mode_to_videomode(mode, >vm);
> >> + timings->frame_rate = mode->clock * 1000 / mode->htotal / mode->vtotal;
> >
> >drm_mode_vrefresh()
> >
> >> + timings->htotal = mode->htotal;
> >> + timings->vtotal = mode->vtotal;
> >> +}
> >
> >It's not really clear to me why you need to duplicate drm_display_mode
> >here?
> >
> It's saved to be re-used in mtk_dp_set_msa().
> It's not ideal, I'll check if I can get the mode directly from
> mtk_dp_set_msa()
Yeah, it looks like mtk_dp_set_msa() uses fairly straightforward values,
this will be just as easy with drm_display_mode.
Maxime
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
On Fri, 29 Apr 2022 10:39, Maxime Ripard wrote:
>Hi Guillaume,
>
Hi Maxime, Thx for your review.
>On Mon, Mar 28, 2022 at 12:39:23AM +0200, Guillaume Ranquet wrote:
>> From: Markus Schneider-Pargmann
>>
>> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
>>
>> It supports the mt8195, the embedded DisplayPort units. It offers
>> DisplayPort 1.4 with up to 4 lanes.
>>
>> The driver shares its iomap range with the mtk-dp-phy driver using
>> the regmap/syscon facility.
>>
>> This driver is based on an initial version by
>> Jason-JH.Lin .
>>
>> Signed-off-by: Markus Schneider-Pargmann
>> Signed-off-by: Guillaume Ranquet
>> Reported-by: kernel test robot
>
>You don't need to set Reported-by on a patch introducing a new driver.
>That would be typically done for a fix.
>
Ok.
>> ---
>> drivers/gpu/drm/mediatek/Kconfig |8 +
>> drivers/gpu/drm/mediatek/Makefile |2 +
>> drivers/gpu/drm/mediatek/mtk_dp.c | 2221
>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
>> drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
>> 6 files changed, 2801 insertions(+)
>> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
>> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
>>
>> diff --git a/drivers/gpu/drm/mediatek/Kconfig
>> b/drivers/gpu/drm/mediatek/Kconfig
>> index 2976d21e9a34..03ffa9b896c3 100644
>> --- a/drivers/gpu/drm/mediatek/Kconfig
>> +++ b/drivers/gpu/drm/mediatek/Kconfig
>> @@ -28,3 +28,11 @@ config DRM_MEDIATEK_HDMI
>> select PHY_MTK_HDMI
>> help
>>DRM/KMS HDMI driver for Mediatek SoCs
>> +
>> +config MTK_DPTX_SUPPORT
>> +tristate "DRM DPTX Support for Mediatek SoCs"
>> +depends on DRM_MEDIATEK
>> +select PHY_MTK_DP
>> +select DRM_DP_HELPER
>> +help
>> + DRM/KMS Display Port driver for Mediatek SoCs.
>> diff --git a/drivers/gpu/drm/mediatek/Makefile
>> b/drivers/gpu/drm/mediatek/Makefile
>> index 29098d7c8307..d86a6406055e 100644
>> --- a/drivers/gpu/drm/mediatek/Makefile
>> +++ b/drivers/gpu/drm/mediatek/Makefile
>> @@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
>>mtk_hdmi_ddc.o
>>
>> obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
>> +
>> +obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
>> b/drivers/gpu/drm/mediatek/mtk_dp.c
>> new file mode 100644
>> index ..7cd8459cf719
>> --- /dev/null
>> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
>> @@ -0,0 +1,2221 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019 MediaTek Inc.
>> + * Copyright (c) 2021 BayLibre
>
>2022?
right.
>
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "mtk_dp_reg.h"
>> +
>> +#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
>> +#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
>> +
>> +//TODO: platform/device data or dts?
>
>DTS :)
It's probably going to be a platform_data struct for v10...
If I have time, I'll change it to a dts property for v10.
>
>> +#define MTK_DP_MAX_LANES 4
>> +#define MTK_DP_MAX_LINK_RATE MTK_DP_LINKRATE_HBR3
>> +
>> +#define MTK_DP_TBC_BUF_READ_START_ADDR 0x08
>> +
>> +#define MTK_DP_TRAIN_RETRY_LIMIT 8
>> +#define MTK_DP_TRAIN_MAX_ITERATIONS 5
>> +
>> +#define MTK_DP_AUX_WRITE_READ_WAIT_TIME_US 20
>> +
>> +#define MTK_DP_DP_VERSION_11 0x11
>> +
>> +enum mtk_dp_state {
>> +MTK_DP_STATE_INITIAL,
>> +MTK_DP_STATE_IDLE,
>> +MTK_DP_STATE_PREPARE,
>> +MTK_DP_STATE_NORMAL,
>> +};
>> +
>> +enum mtk_dp_train_state {
>> +MTK_DP_TRAIN_STATE_STARTUP = 0,
>> +MTK_DP_TRAIN_STATE_CHECKCAP,
>> +MTK_DP_TRAIN_STATE_CHECKEDID,
>> +MTK_DP_TRAIN_STATE_TRAINING_PRE,
>> +MTK_DP_TRAIN_STATE_TRAINING,
>> +MTK_DP_TRAIN_STATE_NORMAL,
>> +MTK_DP_TRAIN_STATE_DPIDLE,
>> +};
>> +
>> +struct mtk_dp_timings {
>> +struct videomode vm;
>> +
>> +u16 htotal;
>> +u16 vtotal;
>> +u8 frame_rate;
>> +u32 pix_rate_khz;
>> +};
>> +
>> +struct mtk_dp_train_info {
>> +bool tps3;
>> +bool tps4;
>> +bool sink_ssc;
>> +bool cable_plugged_in;
>> +bool cable_state_change;
>> +bool cr_done;
>> +bool eq_done;
>> +
>> +/* link_rate is in multiple of 0.27Gbps */
>> +int link_rate;
>> +int lane_count;
>> +
>> +u8 irq_status;
>> +int check_cap_count;
>> +};
>> +
>> +/* Same values as used by the DP Spec for MISC0 bits 1 and 2 */
>> +enum mtk_dp_color_format {
>> +MTK_DP_COLOR_FORMAT_RGB_444 = 0,
>> +MTK_DP_COLOR_FORMAT_YUV_422 = 1,
>> +MTK_DP_COLOR_FORMAT_YUV_444 = 2,
>> +MTK_DP_COLOR_FORMAT_YUV_420 = 3,
>> +
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
Hi Guillaume,
On Mon, Mar 28, 2022 at 12:39:23AM +0200, Guillaume Ranquet wrote:
> From: Markus Schneider-Pargmann
>
> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
>
> It supports the mt8195, the embedded DisplayPort units. It offers
> DisplayPort 1.4 with up to 4 lanes.
>
> The driver shares its iomap range with the mtk-dp-phy driver using
> the regmap/syscon facility.
>
> This driver is based on an initial version by
> Jason-JH.Lin .
>
> Signed-off-by: Markus Schneider-Pargmann
> Signed-off-by: Guillaume Ranquet
> Reported-by: kernel test robot
You don't need to set Reported-by on a patch introducing a new driver.
That would be typically done for a fix.
> ---
> drivers/gpu/drm/mediatek/Kconfig |8 +
> drivers/gpu/drm/mediatek/Makefile |2 +
> drivers/gpu/drm/mediatek/mtk_dp.c | 2221
> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++
> drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
> 6 files changed, 2801 insertions(+)
> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
>
> diff --git a/drivers/gpu/drm/mediatek/Kconfig
> b/drivers/gpu/drm/mediatek/Kconfig
> index 2976d21e9a34..03ffa9b896c3 100644
> --- a/drivers/gpu/drm/mediatek/Kconfig
> +++ b/drivers/gpu/drm/mediatek/Kconfig
> @@ -28,3 +28,11 @@ config DRM_MEDIATEK_HDMI
> select PHY_MTK_HDMI
> help
> DRM/KMS HDMI driver for Mediatek SoCs
> +
> +config MTK_DPTX_SUPPORT
> + tristate "DRM DPTX Support for Mediatek SoCs"
> + depends on DRM_MEDIATEK
> + select PHY_MTK_DP
> + select DRM_DP_HELPER
> + help
> + DRM/KMS Display Port driver for Mediatek SoCs.
> diff --git a/drivers/gpu/drm/mediatek/Makefile
> b/drivers/gpu/drm/mediatek/Makefile
> index 29098d7c8307..d86a6406055e 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
> mtk_hdmi_ddc.o
>
> obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
> +
> +obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> b/drivers/gpu/drm/mediatek/mtk_dp.c
> new file mode 100644
> index ..7cd8459cf719
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -0,0 +1,2221 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + * Copyright (c) 2021 BayLibre
2022?
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "mtk_dp_reg.h"
> +
> +#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
> +#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
> +
> +//TODO: platform/device data or dts?
DTS :)
> +#define MTK_DP_MAX_LANES 4
> +#define MTK_DP_MAX_LINK_RATE MTK_DP_LINKRATE_HBR3
> +
> +#define MTK_DP_TBC_BUF_READ_START_ADDR 0x08
> +
> +#define MTK_DP_TRAIN_RETRY_LIMIT 8
> +#define MTK_DP_TRAIN_MAX_ITERATIONS 5
> +
> +#define MTK_DP_AUX_WRITE_READ_WAIT_TIME_US 20
> +
> +#define MTK_DP_DP_VERSION_11 0x11
> +
> +enum mtk_dp_state {
> + MTK_DP_STATE_INITIAL,
> + MTK_DP_STATE_IDLE,
> + MTK_DP_STATE_PREPARE,
> + MTK_DP_STATE_NORMAL,
> +};
> +
> +enum mtk_dp_train_state {
> + MTK_DP_TRAIN_STATE_STARTUP = 0,
> + MTK_DP_TRAIN_STATE_CHECKCAP,
> + MTK_DP_TRAIN_STATE_CHECKEDID,
> + MTK_DP_TRAIN_STATE_TRAINING_PRE,
> + MTK_DP_TRAIN_STATE_TRAINING,
> + MTK_DP_TRAIN_STATE_NORMAL,
> + MTK_DP_TRAIN_STATE_DPIDLE,
> +};
> +
> +struct mtk_dp_timings {
> + struct videomode vm;
> +
> + u16 htotal;
> + u16 vtotal;
> + u8 frame_rate;
> + u32 pix_rate_khz;
> +};
> +
> +struct mtk_dp_train_info {
> + bool tps3;
> + bool tps4;
> + bool sink_ssc;
> + bool cable_plugged_in;
> + bool cable_state_change;
> + bool cr_done;
> + bool eq_done;
> +
> + /* link_rate is in multiple of 0.27Gbps */
> + int link_rate;
> + int lane_count;
> +
> + u8 irq_status;
> + int check_cap_count;
> +};
> +
> +/* Same values as used by the DP Spec for MISC0 bits 1 and 2 */
> +enum mtk_dp_color_format {
> + MTK_DP_COLOR_FORMAT_RGB_444 = 0,
> + MTK_DP_COLOR_FORMAT_YUV_422 = 1,
> + MTK_DP_COLOR_FORMAT_YUV_444 = 2,
> + MTK_DP_COLOR_FORMAT_YUV_420 = 3,
> + MTK_DP_COLOR_FORMAT_YONLY = 4,
> + MTK_DP_COLOR_FORMAT_RAW = 5,
> + MTK_DP_COLOR_FORMAT_RESERVED = 6,
> + MTK_DP_COLOR_FORMAT_DEFAULT = MTK_DP_COLOR_FORMAT_RGB_444,
> + MTK_DP_COLOR_FORMAT_UNKNOWN = 15,
> +};
Isn't that redundant with DP_MSA_MISC_COLOR_* ?
> +/* Multiple of 0.27Gbps */
> +enum mtk_dp_linkrate {
> + MTK_DP_LINKRATE_RBR =
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
Il 12/04/22 11:46, Guillaume Ranquet ha scritto:
On Mon, 28 Mar 2022 11:14, AngeloGioacchino Del Regno
wrote:
Il 28/03/22 00:39, Guillaume Ranquet ha scritto:
From: Markus Schneider-Pargmann
This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
It supports the mt8195, the embedded DisplayPort units. It offers
DisplayPort 1.4 with up to 4 lanes.
The driver shares its iomap range with the mtk-dp-phy driver using
the regmap/syscon facility.
This driver is based on an initial version by
Jason-JH.Lin .
Signed-off-by: Markus Schneider-Pargmann
Signed-off-by: Guillaume Ranquet
Reported-by: kernel test robot
Hello Guillaume,
as you know, there's some more work to do on this driver.
I will also mention here, not on the code, that at this point, your
mtk_dp_aux_transfer() function has something VERY similar to function
drm_dp_dpcd_access(), so I really believe that you can instead use
functions drm_dp_dpcd_read() and drm_dp_dpcd_write(), avoiding code
duplication around.
Please check drivers/gpu/drm/dp/drm_dp.c.
This is already in my TODO list as this has been suggested by Rex earlier.
---
drivers/gpu/drm/mediatek/Kconfig |8 +
drivers/gpu/drm/mediatek/Makefile |2 +
drivers/gpu/drm/mediatek/mtk_dp.c | 2221
drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++
drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
6 files changed, 2801 insertions(+)
create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 2976d21e9a34..03ffa9b896c3 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -28,3 +28,11 @@ config DRM_MEDIATEK_HDMI
select PHY_MTK_HDMI
help
DRM/KMS HDMI driver for Mediatek SoCs
+
+config MTK_DPTX_SUPPORT
Actually, I think that the best would be DRM_MEDIATEK_DP_TX or
DRM_MEDIATEK_DP...
...also, ordering is important, please!
I will update the name.
What do you mean by ordering? do you expect the configs to be ordered
alphabetically?
Yes, correct. Configuration options shall be alphabetically ordered.
Obviously, the same applies for Makefile :)
+ tristate "DRM DPTX Support for Mediatek SoCs"
+ depends on DRM_MEDIATEK
+ select PHY_MTK_DP
+ select DRM_DP_HELPER
+ help
+ DRM/KMS Display Port driver for Mediatek SoCs.
diff --git a/drivers/gpu/drm/mediatek/Makefile
b/drivers/gpu/drm/mediatek/Makefile
index 29098d7c8307..d86a6406055e 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
mtk_hdmi_ddc.o
obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
+
+obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
b/drivers/gpu/drm/mediatek/mtk_dp.c
new file mode 100644
index ..7cd8459cf719
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
..snip..
+
+#define MTK_UPD_BITS_OR_OUT(mtk_dp, offset, val, mask, ret, label) \
+ do {\
+ ret = mtk_dp_update_bits(mtk_dp, offset, val, mask); \
+ if (ret) \
+ goto label; \
+ } while (0)
I'm sorry, no offense - but this macro is a bit ugly...
I think I understand why you have introduced it, but in my opinion this
decreases
human readability a lot, I was even about to point out multiple functions that
you had unused labels before checking this macro, as that was totally
unexpected...
In my opinion, this should be open-coded everywhere... yes it makes the file a
bit fatter in terms of amount of text, but eh... it's life :)))
No offense taken... I find the macro ugly too... but I couldn't think
of anything less
ugly... I'll bite the bullet and write all of the code then.
I can't think of anything shorter, either.. so yep, sometimes writing 'em all
is simply the best option.
+
snip
+
+static int mtk_dp_set_color_format(struct mtk_dp *mtk_dp,
+ enum mtk_dp_color_format color_format)
+{
..snip..
+
+static int mtk_dp_dt_parse(struct mtk_dp *mtk_dp,
+ struct platform_device *pdev)
+{
+ struct device *dev = >dev;
+ int ret = 0;
+ void __iomem *base;
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ mtk_dp->regs = syscon_node_to_regmap(dev->of_node);
+ if (IS_ERR(mtk_dp->regs))
+ return PTR_ERR(mtk_dp->regs);
+
+ //TODO: optional clock?
Well, if it's optional, you should use devm_clk_get_optional(), meaning
that..
+ mtk_dp->dp_tx_clk = devm_clk_get(dev, "faxi");
+ if (IS_ERR(mtk_dp->dp_tx_clk)) {
+ ret =
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
On Tue, 29 Mar 2022 07:41, Rex-BC Chen wrote:
>Hello Guillaume,
>
>Thanks for your patch.
>I add some comments below:
>
>On Mon, 2022-03-28 at 00:39 +0200, Guillaume Ranquet wrote:
>> From: Markus Schneider-Pargmann
>>
>> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
>>
>> It supports the mt8195, the embedded DisplayPort units. It offers
>> DisplayPort 1.4 with up to 4 lanes.
>>
>> The driver shares its iomap range with the mtk-dp-phy driver using
>> the regmap/syscon facility.
>>
>> This driver is based on an initial version by
>> Jason-JH.Lin .
>>
>> Signed-off-by: Markus Schneider-Pargmann
>> Signed-off-by: Guillaume Ranquet
>> Reported-by: kernel test robot
>> ---
>> drivers/gpu/drm/mediatek/Kconfig |8 +
>> drivers/gpu/drm/mediatek/Makefile |2 +
>> drivers/gpu/drm/mediatek/mtk_dp.c | 2221
>>
>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
>> drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
>> 6 files changed, 2801 insertions(+)
>> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
>> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
>>
>> diff --git a/drivers/gpu/drm/mediatek/Kconfig
>> b/drivers/gpu/drm/mediatek/Kconfig
>> index 2976d21e9a34..03ffa9b896c3 100644
>> --- a/drivers/gpu/drm/mediatek/Kconfig
>> +++ b/drivers/gpu/drm/mediatek/Kconfig
>> @@ -28,3 +28,11 @@ config DRM_MEDIATEK_HDMI
>> select PHY_MTK_HDMI
>> help
>>DRM/KMS HDMI driver for Mediatek SoCs
>> +
>> +config MTK_DPTX_SUPPORT
>> +tristate "DRM DPTX Support for Mediatek SoCs"
>> +depends on DRM_MEDIATEK
>> +select PHY_MTK_DP
>> +select DRM_DP_HELPER
>> +help
>> + DRM/KMS Display Port driver for Mediatek SoCs.
>> diff --git a/drivers/gpu/drm/mediatek/Makefile
>> b/drivers/gpu/drm/mediatek/Makefile
>> index 29098d7c8307..d86a6406055e 100644
>> --- a/drivers/gpu/drm/mediatek/Makefile
>> +++ b/drivers/gpu/drm/mediatek/Makefile
>> @@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
>>mtk_hdmi_ddc.o
>>
>> obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
>> +
>> +obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
>> b/drivers/gpu/drm/mediatek/mtk_dp.c
>> new file mode 100644
>> index ..7cd8459cf719
>> --- /dev/null
>> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
>> @@ -0,0 +1,2221 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019 MediaTek Inc.
>> + * Copyright (c) 2021 BayLibre
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "mtk_dp_reg.h"
>> +
>> +#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
>> +#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
>> +
>> +//TODO: platform/device data or dts?
>> +#define MTK_DP_MAX_LANES 4
>> +#define MTK_DP_MAX_LINK_RATE MTK_DP_LINKRATE_HBR3
>> +
>> +#define MTK_DP_TBC_BUF_READ_START_ADDR 0x08
>> +
>> +#define MTK_DP_TRAIN_RETRY_LIMIT 8
>> +#define MTK_DP_TRAIN_MAX_ITERATIONS 5
>> +
>> +#define MTK_DP_AUX_WRITE_READ_WAIT_TIME_US 20
>> +
>> +#define MTK_DP_DP_VERSION_11 0x11
>> +
>> +enum mtk_dp_state {
>> +MTK_DP_STATE_INITIAL,
>> +MTK_DP_STATE_IDLE,
>> +MTK_DP_STATE_PREPARE,
>> +MTK_DP_STATE_NORMAL,
>> +};
>> +
>>
>
>snif...
>
>> +
>> +static int mtk_dp_msa_bypass_disable(struct mtk_dp *mtk_dp)
>> +{
>> +const u16 bits_to_set =
>> +BIT(HTOTAL_SEL_DP_ENC0_P0_SHIFT) |
>> +BIT(VTOTAL_SEL_DP_ENC0_P0_SHIFT) |
>> +BIT(HSTART_SEL_DP_ENC0_P0_SHIFT) |
>> +BIT(VSTART_SEL_DP_ENC0_P0_SHIFT) |
>> +BIT(HWIDTH_SEL_DP_ENC0_P0_SHIFT) |
>> +BIT(VHEIGHT_SEL_DP_ENC0_P0_SHIFT) |
>> +BIT(HSP_SEL_DP_ENC0_P0_SHIFT) |
>> BIT(HSW_SEL_DP_ENC0_P0_SHIFT) |
>> +BIT(VSP_SEL_DP_ENC0_P0_SHIFT) |
>> BIT(VSW_SEL_DP_ENC0_P0_SHIFT);
>> +return mtk_dp_update_bits(mtk_dp, MTK_DP_ENC0_P0_3030,
>> bits_to_set,
>> + bits_to_set);
>> +}
>> +
>> +#define MTK_UPD_BITS_OR_OUT(mtk_dp, offset, val, mask, ret, label) \
>> +do {\
>> +ret = mtk_dp_update_bits(mtk_dp, offset, val, mask); \
>> +if (ret) \
>> +goto label; \
>> +} while (0)
>> +
>> +static int mtk_dp_set_msa(struct mtk_dp *mtk_dp)
>> +{
>> +int ret;
>> +struct mtk_dp_timings *timings = _dp->info.timings;
>> +
>> +MTK_UPD_BITS_OR_OUT(mtk_dp, MTK_DP_ENC0_P0_3010, timings-
>> >htotal,
>> +HTOTAL_SW_DP_ENC0_P0_MASK, ret, out);
>> +MTK_UPD_BITS_OR_OUT(mtk_dp, MTK_DP_ENC0_P0_3018,
>> +timings->vm.hsync_len + timings-
>>
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
On Tue, 29 Mar 2022 05:34, Rex-BC Chen wrote:
>On Mon, 2022-03-28 at 00:39 +0200, Guillaume Ranquet wrote:
>> From: Markus Schneider-Pargmann
>>
>> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
>>
>> It supports the mt8195, the embedded DisplayPort units. It offers
>> DisplayPort 1.4 with up to 4 lanes.
>>
>> The driver shares its iomap range with the mtk-dp-phy driver using
>> the regmap/syscon facility.
>>
>> This driver is based on an initial version by
>> Jason-JH.Lin .
>>
>> Signed-off-by: Markus Schneider-Pargmann
>> Signed-off-by: Guillaume Ranquet
>> Reported-by: kernel test robot
>> ---
>> drivers/gpu/drm/mediatek/Kconfig |8 +
>> drivers/gpu/drm/mediatek/Makefile |2 +
>> drivers/gpu/drm/mediatek/mtk_dp.c | 2221
>>
>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
>> drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
>> 6 files changed, 2801 insertions(+)
>> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
>> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
>>
>> diff --git a/drivers/gpu/drm/mediatek/Kconfig
>> b/drivers/gpu/drm/mediatek/Kconfig
>> index 2976d21e9a34..03ffa9b896c3 100644
>> --- a/drivers/gpu/drm/mediatek/Kconfig
>> +++ b/drivers/gpu/drm/mediatek/Kconfig
>> @@ -28,3 +28,11 @@ config DRM_MEDIATEK_HDMI
>> select PHY_MTK_HDMI
>> help
>>DRM/KMS HDMI driver for Mediatek SoCs
>> +
>> +config MTK_DPTX_SUPPORT
>> +tristate "DRM DPTX Support for Mediatek SoCs"
>> +depends on DRM_MEDIATEK
>> +select PHY_MTK_DP
>> +select DRM_DP_HELPER
>> +help
>> + DRM/KMS Display Port driver for Mediatek SoCs.
>> diff --git a/drivers/gpu/drm/mediatek/Makefile
>> b/drivers/gpu/drm/mediatek/Makefile
>> index 29098d7c8307..d86a6406055e 100644
>> --- a/drivers/gpu/drm/mediatek/Makefile
>> +++ b/drivers/gpu/drm/mediatek/Makefile
>> @@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
>>mtk_hdmi_ddc.o
>>
>> obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
>> +
>> +obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
>> b/drivers/gpu/drm/mediatek/mtk_dp.c
>> new file mode 100644
>> index ..7cd8459cf719
>> --- /dev/null
>> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
>> @@ -0,0 +1,2221 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019 MediaTek Inc.
>> + * Copyright (c) 2021 BayLibre
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>>
>
>...snip...
>
>> +
>> +static SIMPLE_DEV_PM_OPS(mtk_dp_pm_ops, mtk_dp_suspend,
>> mtk_dp_resume);
>> +
>> +static const struct of_device_id mtk_dp_of_match[] = {
>> +{ .compatible = "mediatek,mt8195-edp-tx", },
>> +{},
>> +};
>> +MODULE_DEVICE_TABLE(of, mtk_dp_of_match);
>> +
>> +struct platform_driver mtk_dp_driver = {
>> +.probe = mtk_dp_probe,
>> +.remove = mtk_dp_remove,
>> +.driver = {
>> +.name = "mediatek-drm-dp",
>> +.of_match_table = mtk_dp_of_match,
>> +.pm = _dp_pm_ops,
>> +},
>> +};
>> +
>> +MODULE_AUTHOR("Jason-JH.Lin ");
>
>Hello Guillaume,
>
>I think the module author is not Jason-JH Lin.
>He is the owner of 8195 DRM.
>The owner should be Jitao Shi
>
>BRs,
>Rex
Hi Rex,
We kept the original author from the vendor tree.
I'll update for v10.
Thx,
Guillaume.
>> +MODULE_AUTHOR("Markus Schneider-Pargmann ");
>> +MODULE_DESCRIPTION("MediaTek DisplayPort Driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
>> b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
>> new file mode 100644
>> index ..c446eef18169
>> --- /dev/null
>> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
>> @@ -0,0 +1,568 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2019 MediaTek Inc.
>> + * Copyright (c) 2021 BayLibre
>> + */
>> +#ifndef _MTK_DP_REG_H_
>> +#define _MTK_DP_REG_H_
>> +
>> +#define MTK_DP_SIP_CONTROL_AARCH32 (BIT(0) | BIT(1) | BIT(5) |
>> BIT(8) | BIT(10) | BIT(25) | BIT(31))
>> +#define MTK_DP_SIP_ATF_VIDEO_UNMUTE (BIT(5))
>> +#define MTK_DP_SIP_ATF_EDP_VIDEO_UNMUTE (BIT(0) | BIT(5))
>> +
>> +#define DP_PHY_GLB_BIAS_GEN_00 0
>> +#define RG_XTP_GLB_BIAS_INTR_CTRL GENMASK(20, 16)
>> +
>> +#define DP_PHY_GLB_DPAUX_TX (BIT(3))
>> +#define RG_CKM_PT0_CKTX_IMPSEL GENMASK(23, 20)
>> +
>> +#define DP_PHY_LANE_TX_0 (BIT(2) | BIT(8))
>> +#define RG_XTP_LN0_TX_IMPSEL_PMOS GENMASK(15, 12)
>> +#define RG_XTP_LN0_TX_IMPSEL_NMOS GENMASK(19, 16)
>> +
>> +#define DP_PHY_LANE_TX_1 (BIT(2) | BIT(9))
>> +#define RG_XTP_LN1_TX_IMPSEL_PMOS GENMASK(15, 12)
>> +#define
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
On Mon, 28 Mar 2022 11:14, AngeloGioacchino Del Regno
wrote:
>Il 28/03/22 00:39, Guillaume Ranquet ha scritto:
>> From: Markus Schneider-Pargmann
>>
>> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
>>
>> It supports the mt8195, the embedded DisplayPort units. It offers
>> DisplayPort 1.4 with up to 4 lanes.
>>
>> The driver shares its iomap range with the mtk-dp-phy driver using
>> the regmap/syscon facility.
>>
>> This driver is based on an initial version by
>> Jason-JH.Lin .
>>
>> Signed-off-by: Markus Schneider-Pargmann
>> Signed-off-by: Guillaume Ranquet
>> Reported-by: kernel test robot
>
>Hello Guillaume,
>as you know, there's some more work to do on this driver.
>
>I will also mention here, not on the code, that at this point, your
>mtk_dp_aux_transfer() function has something VERY similar to function
>drm_dp_dpcd_access(), so I really believe that you can instead use
>functions drm_dp_dpcd_read() and drm_dp_dpcd_write(), avoiding code
>duplication around.
>
>Please check drivers/gpu/drm/dp/drm_dp.c.
>
This is already in my TODO list as this has been suggested by Rex earlier.
>> ---
>> drivers/gpu/drm/mediatek/Kconfig |8 +
>> drivers/gpu/drm/mediatek/Makefile |2 +
>> drivers/gpu/drm/mediatek/mtk_dp.c | 2221
>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
>> drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
>> 6 files changed, 2801 insertions(+)
>> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
>> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
>>
>> diff --git a/drivers/gpu/drm/mediatek/Kconfig
>> b/drivers/gpu/drm/mediatek/Kconfig
>> index 2976d21e9a34..03ffa9b896c3 100644
>> --- a/drivers/gpu/drm/mediatek/Kconfig
>> +++ b/drivers/gpu/drm/mediatek/Kconfig
>> @@ -28,3 +28,11 @@ config DRM_MEDIATEK_HDMI
>> select PHY_MTK_HDMI
>> help
>>DRM/KMS HDMI driver for Mediatek SoCs
>> +
>> +config MTK_DPTX_SUPPORT
>
>Actually, I think that the best would be DRM_MEDIATEK_DP_TX or
>DRM_MEDIATEK_DP...
>...also, ordering is important, please!
>
I will update the name.
What do you mean by ordering? do you expect the configs to be ordered
alphabetically?
>> +tristate "DRM DPTX Support for Mediatek SoCs"
>> +depends on DRM_MEDIATEK
>> +select PHY_MTK_DP
>> +select DRM_DP_HELPER
>> +help
>> + DRM/KMS Display Port driver for Mediatek SoCs.
>> diff --git a/drivers/gpu/drm/mediatek/Makefile
>> b/drivers/gpu/drm/mediatek/Makefile
>> index 29098d7c8307..d86a6406055e 100644
>> --- a/drivers/gpu/drm/mediatek/Makefile
>> +++ b/drivers/gpu/drm/mediatek/Makefile
>> @@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
>>mtk_hdmi_ddc.o
>>
>> obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
>> +
>> +obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
>> b/drivers/gpu/drm/mediatek/mtk_dp.c
>> new file mode 100644
>> index ..7cd8459cf719
>> --- /dev/null
>> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
>> @@ -0,0 +1,2221 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019 MediaTek Inc.
>> + * Copyright (c) 2021 BayLibre
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "mtk_dp_reg.h"
>> +
>> +#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
>> +#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
>> +
>> +//TODO: platform/device data or dts?
>
>Assuming that your TODO is about the maximum number of lanes,
>{ .compatible = "mediatek,mt8195-edp-tx", .data = _const_structure },
Yes, exactly that, it'll be in v10.
>
>> +#define MTK_DP_MAX_LANES 4
>> +#define MTK_DP_MAX_LINK_RATE MTK_DP_LINKRATE_HBR3
>
>..snip..
>
>> +
>> +static int mtk_dp_bulk_16bit_write(struct mtk_dp *mtk_dp, u32 offset, u8
>> *buf,
>> + size_t length)
>> +{
>> +int i;
>> +int ret = 0;
>> +int num_regs = (length + 1) / 2;
>> +
>> +/* 2 bytes per register */
>> +for (i = 0; i < num_regs; i++) {
>> +u32 val = buf[i * 2] |
>> + (i * 2 + 1 < length ? buf[i * 2 + 1] << 8 : 0);
>> +
>> +ret = mtk_dp_write(mtk_dp, offset + i * 4, val);
>> +if (ret)
>> +goto out;
>
>if (ret)
> return ret;
>
>> +}
>> +
>> +out:
>
>Remove this label.
>
>> +return ret;
>> +}
>> +
>> +static unsigned long mtk_dp_sip_atf_call(unsigned int cmd, unsigned int
>> para)
>> +{
>> +struct arm_smccc_res res;
>> +
>> +arm_smccc_smc(MTK_DP_SIP_CONTROL_AARCH32, cmd, para, 0, 0, 0, 0, 0,
>> + );
>> +
>> +
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
Il 28/03/22 00:39, Guillaume Ranquet ha scritto:
From: Markus Schneider-Pargmann
This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
It supports the mt8195, the embedded DisplayPort units. It offers
DisplayPort 1.4 with up to 4 lanes.
The driver shares its iomap range with the mtk-dp-phy driver using
the regmap/syscon facility.
This driver is based on an initial version by
Jason-JH.Lin .
Signed-off-by: Markus Schneider-Pargmann
Signed-off-by: Guillaume Ranquet
Reported-by: kernel test robot
Hello Guillaume,
as you know, there's some more work to do on this driver.
I will also mention here, not on the code, that at this point, your
mtk_dp_aux_transfer() function has something VERY similar to function
drm_dp_dpcd_access(), so I really believe that you can instead use
functions drm_dp_dpcd_read() and drm_dp_dpcd_write(), avoiding code
duplication around.
Please check drivers/gpu/drm/dp/drm_dp.c.
---
drivers/gpu/drm/mediatek/Kconfig |8 +
drivers/gpu/drm/mediatek/Makefile |2 +
drivers/gpu/drm/mediatek/mtk_dp.c | 2221
drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++
drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
6 files changed, 2801 insertions(+)
create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 2976d21e9a34..03ffa9b896c3 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -28,3 +28,11 @@ config DRM_MEDIATEK_HDMI
select PHY_MTK_HDMI
help
DRM/KMS HDMI driver for Mediatek SoCs
+
+config MTK_DPTX_SUPPORT
Actually, I think that the best would be DRM_MEDIATEK_DP_TX or
DRM_MEDIATEK_DP...
...also, ordering is important, please!
+ tristate "DRM DPTX Support for Mediatek SoCs"
+ depends on DRM_MEDIATEK
+ select PHY_MTK_DP
+ select DRM_DP_HELPER
+ help
+ DRM/KMS Display Port driver for Mediatek SoCs.
diff --git a/drivers/gpu/drm/mediatek/Makefile
b/drivers/gpu/drm/mediatek/Makefile
index 29098d7c8307..d86a6406055e 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
mtk_hdmi_ddc.o
obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
+
+obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
b/drivers/gpu/drm/mediatek/mtk_dp.c
new file mode 100644
index ..7cd8459cf719
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -0,0 +1,2221 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 MediaTek Inc.
+ * Copyright (c) 2021 BayLibre
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "mtk_dp_reg.h"
+
+#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
+#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
+
+//TODO: platform/device data or dts?
Assuming that your TODO is about the maximum number of lanes,
{ .compatible = "mediatek,mt8195-edp-tx", .data = _const_structure },
+#define MTK_DP_MAX_LANES 4
+#define MTK_DP_MAX_LINK_RATE MTK_DP_LINKRATE_HBR3
..snip..
+
+static int mtk_dp_bulk_16bit_write(struct mtk_dp *mtk_dp, u32 offset, u8 *buf,
+ size_t length)
+{
+ int i;
+ int ret = 0;
+ int num_regs = (length + 1) / 2;
+
+ /* 2 bytes per register */
+ for (i = 0; i < num_regs; i++) {
+ u32 val = buf[i * 2] |
+ (i * 2 + 1 < length ? buf[i * 2 + 1] << 8 : 0);
+
+ ret = mtk_dp_write(mtk_dp, offset + i * 4, val);
+ if (ret)
+ goto out;
if (ret)
return ret;
+ }
+
+out:
Remove this label.
+ return ret;
+}
+
+static unsigned long mtk_dp_sip_atf_call(unsigned int cmd, unsigned int para)
+{
+ struct arm_smccc_res res;
+
+ arm_smccc_smc(MTK_DP_SIP_CONTROL_AARCH32, cmd, para, 0, 0, 0, 0, 0,
+ );
+
+ pr_debug("[DPTX]%s cmd 0x%x, p1 0x%x, ret 0x%lx-0x%lx", __func__, cmd,
+para, res.a0, res.a1);
+ return res.a1;
+}
+
+static int mtk_dp_msa_bypass_disable(struct mtk_dp *mtk_dp)
+{
+ const u16 bits_to_set =
+ BIT(HTOTAL_SEL_DP_ENC0_P0_SHIFT) |
+ BIT(VTOTAL_SEL_DP_ENC0_P0_SHIFT) |
+ BIT(HSTART_SEL_DP_ENC0_P0_SHIFT) |
+ BIT(VSTART_SEL_DP_ENC0_P0_SHIFT) |
+ BIT(HWIDTH_SEL_DP_ENC0_P0_SHIFT) |
+ BIT(VHEIGHT_SEL_DP_ENC0_P0_SHIFT) |
+ BIT(HSP_SEL_DP_ENC0_P0_SHIFT) | BIT(HSW_SEL_DP_ENC0_P0_SHIFT) |
+
[PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
From: Markus Schneider-Pargmann
This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
It supports the mt8195, the embedded DisplayPort units. It offers
DisplayPort 1.4 with up to 4 lanes.
The driver shares its iomap range with the mtk-dp-phy driver using
the regmap/syscon facility.
This driver is based on an initial version by
Jason-JH.Lin .
Signed-off-by: Markus Schneider-Pargmann
Signed-off-by: Guillaume Ranquet
Reported-by: kernel test robot
---
drivers/gpu/drm/mediatek/Kconfig |8 +
drivers/gpu/drm/mediatek/Makefile |2 +
drivers/gpu/drm/mediatek/mtk_dp.c | 2221
drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++
drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
6 files changed, 2801 insertions(+)
create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 2976d21e9a34..03ffa9b896c3 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -28,3 +28,11 @@ config DRM_MEDIATEK_HDMI
select PHY_MTK_HDMI
help
DRM/KMS HDMI driver for Mediatek SoCs
+
+config MTK_DPTX_SUPPORT
+ tristate "DRM DPTX Support for Mediatek SoCs"
+ depends on DRM_MEDIATEK
+ select PHY_MTK_DP
+ select DRM_DP_HELPER
+ help
+ DRM/KMS Display Port driver for Mediatek SoCs.
diff --git a/drivers/gpu/drm/mediatek/Makefile
b/drivers/gpu/drm/mediatek/Makefile
index 29098d7c8307..d86a6406055e 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
mtk_hdmi_ddc.o
obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
+
+obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
b/drivers/gpu/drm/mediatek/mtk_dp.c
new file mode 100644
index ..7cd8459cf719
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -0,0 +1,2221 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 MediaTek Inc.
+ * Copyright (c) 2021 BayLibre
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "mtk_dp_reg.h"
+
+#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
+#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
+
+//TODO: platform/device data or dts?
+#define MTK_DP_MAX_LANES 4
+#define MTK_DP_MAX_LINK_RATE MTK_DP_LINKRATE_HBR3
+
+#define MTK_DP_TBC_BUF_READ_START_ADDR 0x08
+
+#define MTK_DP_TRAIN_RETRY_LIMIT 8
+#define MTK_DP_TRAIN_MAX_ITERATIONS 5
+
+#define MTK_DP_AUX_WRITE_READ_WAIT_TIME_US 20
+
+#define MTK_DP_DP_VERSION_11 0x11
+
+enum mtk_dp_state {
+ MTK_DP_STATE_INITIAL,
+ MTK_DP_STATE_IDLE,
+ MTK_DP_STATE_PREPARE,
+ MTK_DP_STATE_NORMAL,
+};
+
+enum mtk_dp_train_state {
+ MTK_DP_TRAIN_STATE_STARTUP = 0,
+ MTK_DP_TRAIN_STATE_CHECKCAP,
+ MTK_DP_TRAIN_STATE_CHECKEDID,
+ MTK_DP_TRAIN_STATE_TRAINING_PRE,
+ MTK_DP_TRAIN_STATE_TRAINING,
+ MTK_DP_TRAIN_STATE_NORMAL,
+ MTK_DP_TRAIN_STATE_DPIDLE,
+};
+
+struct mtk_dp_timings {
+ struct videomode vm;
+
+ u16 htotal;
+ u16 vtotal;
+ u8 frame_rate;
+ u32 pix_rate_khz;
+};
+
+struct mtk_dp_train_info {
+ bool tps3;
+ bool tps4;
+ bool sink_ssc;
+ bool cable_plugged_in;
+ bool cable_state_change;
+ bool cr_done;
+ bool eq_done;
+
+ /* link_rate is in multiple of 0.27Gbps */
+ int link_rate;
+ int lane_count;
+
+ u8 irq_status;
+ int check_cap_count;
+};
+
+/* Same values as used by the DP Spec for MISC0 bits 1 and 2 */
+enum mtk_dp_color_format {
+ MTK_DP_COLOR_FORMAT_RGB_444 = 0,
+ MTK_DP_COLOR_FORMAT_YUV_422 = 1,
+ MTK_DP_COLOR_FORMAT_YUV_444 = 2,
+ MTK_DP_COLOR_FORMAT_YUV_420 = 3,
+ MTK_DP_COLOR_FORMAT_YONLY = 4,
+ MTK_DP_COLOR_FORMAT_RAW = 5,
+ MTK_DP_COLOR_FORMAT_RESERVED = 6,
+ MTK_DP_COLOR_FORMAT_DEFAULT = MTK_DP_COLOR_FORMAT_RGB_444,
+ MTK_DP_COLOR_FORMAT_UNKNOWN = 15,
+};
+
+/* Multiple of 0.27Gbps */
+enum mtk_dp_linkrate {
+ MTK_DP_LINKRATE_RBR = 0x6,
+ MTK_DP_LINKRATE_HBR = 0xA,
+ MTK_DP_LINKRATE_HBR2 = 0x14,
+ MTK_DP_LINKRATE_HBR25 = 0x19,
+ MTK_DP_LINKRATE_HBR3 = 0x1E,
+};
+
+/* Same values as used for DP Spec MISC0 bits 5,6,7 */
+enum mtk_dp_color_depth {
+ MTK_DP_COLOR_DEPTH_6BIT = 0,
+ MTK_DP_COLOR_DEPTH_8BIT = 1,
+ MTK_DP_COLOR_DEPTH_10BIT = 2,
+ MTK_DP_COLOR_DEPTH_12BIT = 3,
+ MTK_DP_COLOR_DEPTH_16BIT = 4,
+ MTK_DP_COLOR_DEPTH_UNKNOWN = 5,
+};
+
+struct mtk_dp_info {
+
