Re: [PATCH v13] drm/bridge: it6505: fix hibernate to resume no display issue
On Wed, 22 May 2024 14:55:28 +0800, kuro wrote: > From: Kuro Chung > > When the system power resumes, the TTL input of IT6505 may experience > some noise before the video signal stabilizes, necessitating a video > reset. This patch is implemented to prevent a loop of video error > interrupts, which can occur when a video reset in the video FIFO error > interrupt triggers another such interrupt. The patch processes the SCDT > and FIFO error interrupts simultaneously and ignores any video FIFO > error interrupts caused by a video reset. > > [...] Applied, thanks! [1/1] drm/bridge: it6505: fix hibernate to resume no display issue https://cgit.freedesktop.org/drm/drm-misc/commit/?id=484436ec5c2b Rob
Re: [PATCH v12] drm/bridge: it6505: fix hibernate to resume no display issue
Hey,
Thanks for spinning another few revisions.
On Tue, May 21, 2024 at 9:51 AM kuro wrote:
>
> From: Kuro Chung
>
> When the system power resumes, the TTL input of IT6505 may experience
> some noise before the video signal stabilizes, necessitating a video
> reset. This patch is implemented to prevent a loop of video error
> interrupts, which can occur when a video reset in the video FIFO error
> interrupt triggers another such interrupt. The patch processes the SCDT
> and FIFO error interrupts simultaneously and ignores any video FIFO
> error interrupts caused by a video reset.
>
> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> Signed-off-by: Kuro Chung
> Signed-off-by: Hermes Wu
>
> ---
> V1->V3: update MAINTAINERS mail list
> V3->V4: remove function
> it6505_irq_video_fifo_error,it6505_irq_io_latch_fifo_overflow
> V4->V5: customer feedback again, update again, kernel build pass
> V5->V6: remove unrelated patch change, split into another patch
> V6->V7: modify code 0x02 to TX_FIFO_RESET by macro define
> V7->V8: fix merge conflict, change mail from 'cc' to 'to'
> V8->V9: modify patch description, patch summary
> V9->V10: modify patch summary, add Fixes
> V10->V11: modify patch description, add Signed-off-by
> V11->V12: moidfy patch description.
>
> ---
> drivers/gpu/drm/bridge/ite-it6505.c | 73 +++--
> 1 file changed, 49 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
> b/drivers/gpu/drm/bridge/ite-it6505.c
> index 469157341f3ab..5703fcf4b7b00 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1307,9 +1307,15 @@ static void it6505_video_reset(struct it6505 *it6505)
> it6505_link_reset_step_train(it6505);
> it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_VID_MUTE, EN_VID_MUTE);
> it6505_set_bits(it6505, REG_INFOFRAME_CTRL, EN_VID_CTRL_PKT, 0x00);
> - it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, VIDEO_RESET);
> +
> + it6505_set_bits(it6505, REG_VID_BUS_CTRL1, TX_FIFO_RESET,
> TX_FIFO_RESET);
> + it6505_set_bits(it6505, REG_VID_BUS_CTRL1, TX_FIFO_RESET, 0x00);
> +
> it6505_set_bits(it6505, REG_501_FIFO_CTRL, RST_501_FIFO,
> RST_501_FIFO);
> it6505_set_bits(it6505, REG_501_FIFO_CTRL, RST_501_FIFO, 0x00);
> +
> + it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, VIDEO_RESET);
> + usleep_range(1000, 2000);
> it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, 0x00);
> }
>
> @@ -2245,12 +2251,11 @@ static void it6505_link_training_work(struct
> work_struct *work)
> if (ret) {
> it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> it6505_link_train_ok(it6505);
> - return;
> } else {
> it6505->auto_train_retry--;
> + it6505_dump(it6505);
> }
>
> - it6505_dump(it6505);
> }
>
> static void it6505_plugged_status_to_codec(struct it6505 *it6505)
> @@ -2471,31 +2476,53 @@ static void it6505_irq_link_train_fail(struct it6505
> *it6505)
> schedule_work(>link_works);
> }
>
> -static void it6505_irq_video_fifo_error(struct it6505 *it6505)
> +static bool it6505_test_bit(unsigned int bit, const unsigned int *addr)
> {
> - struct device *dev = it6505->dev;
> -
> - DRM_DEV_DEBUG_DRIVER(dev, "video fifo overflow interrupt");
> - it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> - flush_work(>link_works);
> - it6505_stop_hdcp(it6505);
> - it6505_video_reset(it6505);
> + return 1 & (addr[bit / BITS_PER_BYTE] >> (bit % BITS_PER_BYTE));
> }
>
> -static void it6505_irq_io_latch_fifo_overflow(struct it6505 *it6505)
> +static void it6505_irq_video_handler(struct it6505 *it6505, const int
> *int_status)
> {
> struct device *dev = it6505->dev;
> + int reg_0d, reg_int03;
>
> - DRM_DEV_DEBUG_DRIVER(dev, "IO latch fifo overflow interrupt");
> - it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> - flush_work(>link_works);
> - it6505_stop_hdcp(it6505);
> - it6505_video_reset(it6505);
> -}
> + /*
> +* When video SCDT change with video not stable,
> +* Or video FIFO error, need video reset
> +*/
>
> -static bool it6505_test_bit(unsigned int bit, const unsigned int *addr)
> -{
> - return 1 & (addr[bit / BITS_PER_BYTE] >> (bit % BITS_PER_BYTE));
> + if ((!it6505_get_video_status(it6505) &&
> + (it6505_test_bit(INT_SCDT_CHANGE, (unsigned int *)
> int_status))) ||
> + (it6505_test_bit(BIT_INT_IO_FIFO_OVERFLOW, (unsigned int *)
> int_status)) ||
> + (it6505_test_bit(BIT_INT_VID_FIFO_ERROR, (unsigned int *)
> int_status))) {
> +
> + it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> + flush_work(>link_works);
> + it6505_stop_hdcp(it6505);
> + it6505_video_reset(it6505);
> +
> +
Re: [PATCH] drm/bridge: tc358767: Enable FRMSYNC timing generator
On Mon, 13 May 2024 04:16:04 +0200, Marek Vasut wrote: > TC9595 datasheet Video Path0 Control (VPCTRL0) Register bit FRMSYNC > description > says "This bit should be disabled only in video mode transmission where Host > transmits video timing together with video data and where pixel clock source > is from DSI clock." . This driver always sources pixel clock from external > xtal, > therefore the FRMSYNC bit must always be enabled, enable it. > > This fixes an actual issue with DSI-to-DPI mode, where the display would > randomly show subtle pixel flickering, or wobble, or shimmering. This is > visible on solid gray color, but the degree of the shimmering differs > between boots, which makes it hard to debug. > > [...] Applied, thanks! [1/1] drm/bridge: tc358767: Enable FRMSYNC timing generator https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d9ca4b760ef6 Rob
Re: [PATCH v7 1/1] drm/bridge: it6505: fix hibernate to resume no display issue
On Mon, May 13, 2024 at 7:42 PM Robert Foss wrote:
>
> On Mon, May 6, 2024 at 11:36 AM kuro wrote:
> >
> > From: Kuro
> >
> > ITE added a FIFO reset bit for input video. When system power resume,
> > the TTL input of it6505 may get some noise before video signal stable
> > and the hardware function reset is required.
> > But the input FIFO reset will also trigger error interrupts of output
> > module rising.
> > Thus, it6505 have to wait a period can clear those expected error interrupts
> > caused by manual hardware reset in one interrupt handler calling to avoid
> > interrupt looping.
> >
> > Signed-off-by: Kuro Chung
> >
> > ---
> > drivers/gpu/drm/bridge/ite-it6505.c | 73 +++--
> > 1 file changed, 49 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
> > b/drivers/gpu/drm/bridge/ite-it6505.c
> > index b53da9bb65a16..64e2706e3d0c3 100644
> > --- a/drivers/gpu/drm/bridge/ite-it6505.c
> > +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> > @@ -1317,9 +1317,15 @@ static void it6505_video_reset(struct it6505 *it6505)
> > it6505_link_reset_step_train(it6505);
> > it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_VID_MUTE,
> > EN_VID_MUTE);
> > it6505_set_bits(it6505, REG_INFOFRAME_CTRL, EN_VID_CTRL_PKT, 0x00);
> > - it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, VIDEO_RESET);
> > +
> > + it6505_set_bits(it6505, REG_VID_BUS_CTRL1, TX_FIFO_RESET,
> > TX_FIFO_RESET);
> > + it6505_set_bits(it6505, REG_VID_BUS_CTRL1, TX_FIFO_RESET, 0x00);
> > +
> > it6505_set_bits(it6505, REG_501_FIFO_CTRL, RST_501_FIFO,
> > RST_501_FIFO);
> > it6505_set_bits(it6505, REG_501_FIFO_CTRL, RST_501_FIFO, 0x00);
> > +
> > + it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, VIDEO_RESET);
> > + usleep_range(1000, 2000);
> > it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, 0x00);
> > }
> >
> > @@ -2249,12 +2255,11 @@ static void it6505_link_training_work(struct
> > work_struct *work)
> > if (ret) {
> > it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> > it6505_link_train_ok(it6505);
> > - return;
> > } else {
> > it6505->auto_train_retry--;
> > + it6505_dump(it6505);
> > }
> >
> > - it6505_dump(it6505);
> > }
> >
> > static void it6505_plugged_status_to_codec(struct it6505 *it6505)
> > @@ -2475,31 +2480,53 @@ static void it6505_irq_link_train_fail(struct
> > it6505 *it6505)
> > schedule_work(>link_works);
> > }
> >
> > -static void it6505_irq_video_fifo_error(struct it6505 *it6505)
> > +static bool it6505_test_bit(unsigned int bit, const unsigned int *addr)
> > {
> > - struct device *dev = >client->dev;
> > -
> > - DRM_DEV_DEBUG_DRIVER(dev, "video fifo overflow interrupt");
> > - it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> > - flush_work(>link_works);
> > - it6505_stop_hdcp(it6505);
> > - it6505_video_reset(it6505);
> > + return 1 & (addr[bit / BITS_PER_BYTE] >> (bit % BITS_PER_BYTE));
> > }
> >
> > -static void it6505_irq_io_latch_fifo_overflow(struct it6505 *it6505)
> > +static void it6505_irq_video_handler(struct it6505 *it6505, const int
> > *int_status)
> > {
> > struct device *dev = >client->dev;
> > + int reg_0d, reg_int03;
> >
> > - DRM_DEV_DEBUG_DRIVER(dev, "IO latch fifo overflow interrupt");
> > - it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> > - flush_work(>link_works);
> > - it6505_stop_hdcp(it6505);
> > - it6505_video_reset(it6505);
> > -}
> > + /*
> > +* When video SCDT change with video not stable,
> > +* Or video FIFO error, need video reset
> > +*/
> >
> > -static bool it6505_test_bit(unsigned int bit, const unsigned int *addr)
> > -{
> > - return 1 & (addr[bit / BITS_PER_BYTE] >> (bit % BITS_PER_BYTE));
> > + if ((!it6505_get_video_status(it6505) &&
> > + (it6505_test_bit(INT_SCDT_CHANGE, (unsigned int *)
> > int_status))) ||
> > + (it6505_test_bit(BIT_INT_IO_FIFO_OVERFLOW, (unsigned int *)
> > int_status)) ||
> > + (it6505_test_bit(BIT_INT_VID_FIFO_ERROR, (unsigned int *)
>
Re: [PATCH v7 1/1] drm/bridge: it6505: fix hibernate to resume no display issue
t6505_read(it6505, INT_STATUS_03);
> + reg_0d = it6505_read(it6505, REG_SYSTEM_STS);
> +
> + reg_int03 &= (BIT(INT_VID_FIFO_ERROR) |
> BIT(INT_IO_LATCH_FIFO_OVERFLOW));
> + it6505_write(it6505, INT_STATUS_03, reg_int03);
> +
> + DRM_DEV_DEBUG_DRIVER(dev, "reg08 = 0x%02x", reg_int03);
> + DRM_DEV_DEBUG_DRIVER(dev, "reg0D = 0x%02x", reg_0d);
> +
> + return;
> + }
> +
> +
> + if (it6505_test_bit(INT_SCDT_CHANGE, (unsigned int *) int_status))
> + it6505_irq_scdt(it6505);
> }
>
> static irqreturn_t it6505_int_threaded_handler(int unused, void *data)
> @@ -2512,15 +2539,12 @@ static irqreturn_t it6505_int_threaded_handler(int
> unused, void *data)
> } irq_vec[] = {
> { BIT_INT_HPD, it6505_irq_hpd },
> { BIT_INT_HPD_IRQ, it6505_irq_hpd_irq },
> - { BIT_INT_SCDT, it6505_irq_scdt },
> { BIT_INT_HDCP_FAIL, it6505_irq_hdcp_fail },
> { BIT_INT_HDCP_DONE, it6505_irq_hdcp_done },
> { BIT_INT_AUX_CMD_FAIL, it6505_irq_aux_cmd_fail },
> { BIT_INT_HDCP_KSV_CHECK, it6505_irq_hdcp_ksv_check },
> { BIT_INT_AUDIO_FIFO_ERROR, it6505_irq_audio_fifo_error },
> { BIT_INT_LINK_TRAIN_FAIL, it6505_irq_link_train_fail },
> - { BIT_INT_VID_FIFO_ERROR, it6505_irq_video_fifo_error },
> - { BIT_INT_IO_FIFO_OVERFLOW, it6505_irq_io_latch_fifo_overflow
> },
> };
> int int_status[3], i;
>
> @@ -2550,6 +2574,7 @@ static irqreturn_t it6505_int_threaded_handler(int
> unused, void *data)
> if (it6505_test_bit(irq_vec[i].bit, (unsigned int
> *)int_status))
> irq_vec[i].handler(it6505);
> }
> + it6505_irq_video_handler(it6505, (unsigned int *) int_status);
> }
>
> pm_runtime_put_sync(dev);
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [RESEND 0/6] drm: struct drm_edid conversions
On Fri, 10 May 2024 16:26:03 +0300, Jani Nikula wrote: > Resend of the remaining patches from [1]. > > BR, > Jani. > > [1] https://lore.kernel.org/r/cover.1713273659.git.jani.nik...@intel.com > > [...] Fixed checkpatch issue in "drm/bochs: switch to struct drm_edid" Applied, thanks! [1/6] drm/bridge/analogix/anx6345: switch to struct drm_edid https://cgit.freedesktop.org/drm/drm-misc/commit/?id=37f3821c7cc8 [2/6] drm/bridge/analogix/anx78xx: switch to struct drm_edid https://cgit.freedesktop.org/drm/drm-misc/commit/?id=8aa8781ba3c1 [3/6] drm/bridge: anx7625: use struct drm_edid more https://cgit.freedesktop.org/drm/drm-misc/commit/?id=7c585f9a71aa [4/6] drm/i2c: tda998x: switch to struct drm_edid https://cgit.freedesktop.org/drm/drm-misc/commit/?id=78e90e003b96 [5/6] drm/bochs: switch to struct drm_edid https://cgit.freedesktop.org/drm/drm-misc/commit/?id=5c465601d423 [6/6] drm/virtio: switch to struct drm_edid https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ac15c653fb09 Rob
Re: [RESEND 3/6] drm/bridge: anx7625: use struct drm_edid more
On Fri, May 10, 2024 at 3:26 PM Jani Nikula wrote:
>
> Prefer struct drm_edid based functions over struct edid.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 26 +++
> drivers/gpu/drm/bridge/analogix/anx7625.h | 10 ++---
> 2 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 59e9ad349969..d19975c5e5e5 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -464,9 +464,11 @@ static int anx7625_odfc_config(struct anx7625_data *ctx,
> */
> static int anx7625_set_k_value(struct anx7625_data *ctx)
> {
> - struct edid *edid = (struct edid *)ctx->slimport_edid_p.edid_raw_data;
> + struct drm_edid_product_id id;
>
> - if (edid->mfg_id[0] == IVO_MID0 && edid->mfg_id[1] == IVO_MID1)
> + drm_edid_get_product_id(ctx->cached_drm_edid, );
> +
> + if (be16_to_cpu(id.manufacturer_name) == IVO_MID)
> return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> MIPI_DIGITAL_ADJ_1, 0x3B);
>
> @@ -1526,7 +1528,8 @@ static int anx7625_wait_hpd_asserted(struct drm_dp_aux
> *aux,
>
> static void anx7625_remove_edid(struct anx7625_data *ctx)
> {
> - ctx->slimport_edid_p.edid_block_num = -1;
> + drm_edid_free(ctx->cached_drm_edid);
> + ctx->cached_drm_edid = NULL;
> }
>
> static void anx7625_dp_adjust_swing(struct anx7625_data *ctx)
> @@ -1787,27 +1790,32 @@ static ssize_t anx7625_aux_transfer(struct drm_dp_aux
> *aux,
> static const struct drm_edid *anx7625_edid_read(struct anx7625_data *ctx)
> {
> struct device *dev = ctx->dev;
> - struct s_edid_data *p_edid = >slimport_edid_p;
> + u8 *edid_buf;
> int edid_num;
>
> - if (ctx->slimport_edid_p.edid_block_num > 0)
> + if (ctx->cached_drm_edid)
> goto out;
>
> + edid_buf = kmalloc(FOUR_BLOCK_SIZE, GFP_KERNEL);
> + if (!edid_buf)
> + return NULL;
> +
> pm_runtime_get_sync(dev);
> _anx7625_hpd_polling(ctx, 5000 * 100);
> - edid_num = sp_tx_edid_read(ctx, p_edid->edid_raw_data);
> + edid_num = sp_tx_edid_read(ctx, edid_buf);
> pm_runtime_put_sync(dev);
>
> if (edid_num < 1) {
> DRM_DEV_ERROR(dev, "Fail to read EDID: %d\n", edid_num);
> + kfree(edid_buf);
> return NULL;
> }
>
> - p_edid->edid_block_num = edid_num;
> + ctx->cached_drm_edid = drm_edid_alloc(edid_buf, FOUR_BLOCK_SIZE);
> + kfree(edid_buf);
>
> out:
> - return drm_edid_alloc(ctx->slimport_edid_p.edid_raw_data,
> - FOUR_BLOCK_SIZE);
> + return drm_edid_dup(ctx->cached_drm_edid);
> }
>
> static enum drm_connector_status anx7625_sink_detect(struct anx7625_data
> *ctx)
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
> b/drivers/gpu/drm/bridge/analogix/anx7625.h
> index 39ed35d33836..eb5580f1ab2f 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> @@ -286,8 +286,7 @@
>
> #define MIPI_LANE_CTRL_10 0x0F
> #define MIPI_DIGITAL_ADJ_1 0x1B
> -#define IVO_MID0 0x26
> -#define IVO_MID1 0xCF
> +#define IVO_MID0x26CF
>
> #define MIPI_PLL_M_NUM_23_16 0x1E
> #define MIPI_PLL_M_NUM_15_80x1F
> @@ -417,11 +416,6 @@ enum audio_wd_len {
> #define EDID_TRY_CNT 3
> #define SUPPORT_PIXEL_CLOCK30
>
> -struct s_edid_data {
> - int edid_block_num;
> - u8 edid_raw_data[FOUR_BLOCK_SIZE];
> -};
> -
> /* Display End */
>
> #define MAX_LANES_SUPPORT 4
> @@ -466,7 +460,7 @@ struct anx7625_data {
> struct anx7625_i2c_client i2c;
> struct i2c_client *last_client;
> struct timer_list hdcp_timer;
> - struct s_edid_data slimport_edid_p;
> + const struct drm_edid *cached_drm_edid;
> struct device *codec_dev;
> hdmi_codec_plugged_cb plugged_cb;
> struct work_struct work;
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [RESEND 2/6] drm/bridge/analogix/anx78xx: switch to struct drm_edid
On Fri, May 10, 2024 at 3:26 PM Jani Nikula wrote:
>
> Prefer struct drm_edid based functions over struct edid.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> ---
> .../drm/bridge/analogix/analogix-anx78xx.c| 23 ++-
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> index 5748a8581af4..ae79bcd8fa55 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> @@ -67,7 +67,7 @@ struct anx78xx {
> struct drm_dp_aux aux;
> struct drm_bridge bridge;
> struct i2c_client *client;
> - struct edid *edid;
> + const struct drm_edid *drm_edid;
> struct drm_connector connector;
> struct anx78xx_platform_data pdata;
> struct mutex lock;
> @@ -830,8 +830,8 @@ static int anx78xx_get_modes(struct drm_connector
> *connector)
> if (WARN_ON(!anx78xx->powered))
> return 0;
>
> - if (anx78xx->edid)
> - return drm_add_edid_modes(connector, anx78xx->edid);
> + if (anx78xx->drm_edid)
> + return drm_edid_connector_add_modes(connector);
>
> mutex_lock(>lock);
>
> @@ -841,20 +841,21 @@ static int anx78xx_get_modes(struct drm_connector
> *connector)
> goto unlock;
> }
>
> - anx78xx->edid = drm_get_edid(connector, >aux.ddc);
> - if (!anx78xx->edid) {
> + anx78xx->drm_edid = drm_edid_read_ddc(connector, >aux.ddc);
> +
> + err = drm_edid_connector_update(connector, anx78xx->drm_edid);
> +
> + if (!anx78xx->drm_edid) {
> DRM_ERROR("Failed to read EDID\n");
> goto unlock;
> }
>
> - err = drm_connector_update_edid_property(connector,
> -anx78xx->edid);
> if (err) {
> DRM_ERROR("Failed to update EDID property: %d\n", err);
> goto unlock;
> }
>
> - num_modes = drm_add_edid_modes(connector, anx78xx->edid);
> + num_modes = drm_edid_connector_add_modes(connector);
>
> unlock:
> mutex_unlock(>lock);
> @@ -1091,8 +1092,8 @@ static bool anx78xx_handle_common_int_4(struct anx78xx
> *anx78xx, u8 irq)
> event = true;
> anx78xx_poweroff(anx78xx);
> /* Free cached EDID */
> - kfree(anx78xx->edid);
> - anx78xx->edid = NULL;
> + drm_edid_free(anx78xx->drm_edid);
> + anx78xx->drm_edid = NULL;
> } else if (irq & SP_HPD_PLUG) {
> DRM_DEBUG_KMS("IRQ: Hot plug detect - cable plug\n");
> event = true;
> @@ -1363,7 +1364,7 @@ static void anx78xx_i2c_remove(struct i2c_client
> *client)
>
> unregister_i2c_dummy_clients(anx78xx);
>
> - kfree(anx78xx->edid);
> + drm_edid_free(anx78xx->drm_edid);
> }
>
> static const struct of_device_id anx78xx_match_table[] = {
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [RESEND 1/6] drm/bridge/analogix/anx6345: switch to struct drm_edid
On Fri, May 10, 2024 at 3:26 PM Jani Nikula wrote:
>
> Prefer struct drm_edid based functions over struct edid.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> ---
> .../gpu/drm/bridge/analogix/analogix-anx6345.c| 15 +++
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> index c9e35731e6a1..42ab6014fe12 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -47,7 +47,7 @@ struct anx6345 {
> struct drm_dp_aux aux;
> struct drm_bridge bridge;
> struct i2c_client *client;
> - struct edid *edid;
> + const struct drm_edid *drm_edid;
> struct drm_connector connector;
> struct drm_panel *panel;
> struct regulator *dvdd12;
> @@ -458,7 +458,7 @@ static int anx6345_get_modes(struct drm_connector
> *connector)
>
> mutex_lock(>lock);
>
> - if (!anx6345->edid) {
> + if (!anx6345->drm_edid) {
> if (!anx6345->powered) {
> anx6345_poweron(anx6345);
> power_off = true;
> @@ -470,19 +470,18 @@ static int anx6345_get_modes(struct drm_connector
> *connector)
> goto unlock;
> }
>
> - anx6345->edid = drm_get_edid(connector, >aux.ddc);
> - if (!anx6345->edid)
> + anx6345->drm_edid = drm_edid_read_ddc(connector,
> >aux.ddc);
> + if (!anx6345->drm_edid)
> DRM_ERROR("Failed to read EDID from panel\n");
>
> - err = drm_connector_update_edid_property(connector,
> -anx6345->edid);
> + err = drm_edid_connector_update(connector, anx6345->drm_edid);
> if (err) {
> DRM_ERROR("Failed to update EDID property: %d\n",
> err);
> goto unlock;
> }
> }
>
> - num_modes += drm_add_edid_modes(connector, anx6345->edid);
> + num_modes += drm_edid_connector_add_modes(connector);
>
> /* Driver currently supports only 6bpc */
> connector->display_info.bpc = 6;
> @@ -793,7 +792,7 @@ static void anx6345_i2c_remove(struct i2c_client *client)
>
> unregister_i2c_dummy_clients(anx6345);
>
> - kfree(anx6345->edid);
> + drm_edid_free(anx6345->drm_edid);
>
> mutex_destroy(>lock);
> }
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [RESEND 4/6] drm/i2c: tda998x: switch to struct drm_edid
On Fri, May 10, 2024 at 3:26 PM Jani Nikula wrote:
>
> Prefer struct drm_edid based functions over struct edid.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: Russell King
> ---
> drivers/gpu/drm/i2c/tda998x_drv.c | 19 ++-
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
> b/drivers/gpu/drm/i2c/tda998x_drv.c
> index d8d7de18dd65..2160f05bbd16 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -1283,7 +1283,7 @@ static int read_edid_block(void *data, u8 *buf,
> unsigned int blk, size_t length)
> static int tda998x_connector_get_modes(struct drm_connector *connector)
> {
> struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
> - struct edid *edid;
> + const struct drm_edid *drm_edid;
> int n;
>
> /*
> @@ -1297,25 +1297,26 @@ static int tda998x_connector_get_modes(struct
> drm_connector *connector)
> if (priv->rev == TDA19988)
> reg_clear(priv, REG_TX4, TX4_PD_RAM);
>
> - edid = drm_do_get_edid(connector, read_edid_block, priv);
> + drm_edid = drm_edid_read_custom(connector, read_edid_block, priv);
>
> if (priv->rev == TDA19988)
> reg_set(priv, REG_TX4, TX4_PD_RAM);
>
> - if (!edid) {
> + drm_edid_connector_update(connector, drm_edid);
> + cec_notifier_set_phys_addr(priv->cec_notify,
> +
> connector->display_info.source_physical_address);
> +
> + if (!drm_edid) {
> dev_warn(>hdmi->dev, "failed to read EDID\n");
> return 0;
> }
>
> - drm_connector_update_edid_property(connector, edid);
> - cec_notifier_set_phys_addr_from_edid(priv->cec_notify, edid);
> -
> mutex_lock(>audio_mutex);
> - n = drm_add_edid_modes(connector, edid);
> - priv->sink_has_audio = drm_detect_monitor_audio(edid);
> + n = drm_edid_connector_add_modes(connector);
> + priv->sink_has_audio = connector->display_info.has_audio;
> mutex_unlock(>audio_mutex);
>
> - kfree(edid);
> + drm_edid_free(drm_edid);
>
> return n;
> }
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [RESEND 5/6] drm/bochs: switch to struct drm_edid
On Fri, May 10, 2024 at 3:27 PM Jani Nikula wrote:
>
> Prefer struct drm_edid based functions over struct edid.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: Gerd Hoffmann
> Cc: virtualizat...@lists.linux.dev
> ---
> drivers/gpu/drm/tiny/bochs.c | 23 ++-
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index 2d7ad808cc0e..d12d6e0bf3df 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -85,7 +85,7 @@ struct bochs_device {
> u16 yres_virtual;
> u32 stride;
> u32 bpp;
> - struct edid *edid;
> + const struct drm_edid *drm_edid;
>
> /* drm */
> struct drm_device *dev;
> @@ -199,10 +199,10 @@ static int bochs_hw_load_edid(struct bochs_device
> *bochs)
> if (drm_edid_header_is_valid(header) != 8)
> return -1;
>
> - kfree(bochs->edid);
> - bochs->edid = drm_do_get_edid(>connector,
> - bochs_get_edid_block, bochs);
> - if (bochs->edid == NULL)
> + drm_edid_free(bochs->drm_edid);
> + bochs->drm_edid = drm_edid_read_custom(>connector,
> + bochs_get_edid_block, bochs);
> + if (bochs->drm_edid == NULL)
> return -1;
>
> return 0;
> @@ -303,7 +303,7 @@ static void bochs_hw_fini(struct drm_device *dev)
> if (bochs->fb_map)
> iounmap(bochs->fb_map);
> pci_release_regions(to_pci_dev(dev->dev));
> - kfree(bochs->edid);
> + drm_edid_free(bochs->drm_edid);
> }
>
> static void bochs_hw_blank(struct bochs_device *bochs, bool blank)
> @@ -471,12 +471,9 @@ static const struct drm_simple_display_pipe_funcs
> bochs_pipe_funcs = {
>
> static int bochs_connector_get_modes(struct drm_connector *connector)
> {
> - struct bochs_device *bochs =
> - container_of(connector, struct bochs_device, connector);
> - int count = 0;
> + int count;
>
> - if (bochs->edid)
> - count = drm_add_edid_modes(connector, bochs->edid);
> + count = drm_edid_connector_add_modes(connector);
>
> if (!count) {
> count = drm_add_modes_noedid(connector, 8192, 8192);
> @@ -507,10 +504,10 @@ static void bochs_connector_init(struct drm_device *dev)
> drm_connector_helper_add(connector,
> _connector_connector_helper_funcs);
>
> bochs_hw_load_edid(bochs);
> - if (bochs->edid) {
> + if (bochs->drm_edid) {
> DRM_INFO("Found EDID data blob.\n");
> drm_connector_attach_edid_property(connector);
> - drm_connector_update_edid_property(connector, bochs->edid);
> + drm_edid_connector_update(>connector, bochs->drm_edid);
> }
> }
>
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [RESEND 6/6] drm/virtio: switch to struct drm_edid
On Fri, May 10, 2024 at 3:34 PM Jani Nikula wrote:
>
> Prefer struct drm_edid based functions over struct edid.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: David Airlie
> Cc: Gerd Hoffmann
> Cc: Gurchetan Singh
> Cc: Chia-I Wu
> Cc: virtualizat...@lists.linux.dev
> ---
> drivers/gpu/drm/virtio/virtgpu_display.c | 10 --
> drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++--
> 3 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
> b/drivers/gpu/drm/virtio/virtgpu_display.c
> index ad924a8502e9..64baf2f22d9f 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -164,11 +164,9 @@ static int virtio_gpu_conn_get_modes(struct
> drm_connector *connector)
> struct drm_display_mode *mode = NULL;
> int count, width, height;
>
> - if (output->edid) {
> - count = drm_add_edid_modes(connector, output->edid);
> - if (count)
> - return count;
> - }
> + count = drm_edid_connector_add_modes(connector);
> + if (count)
> + return count;
>
> width = le32_to_cpu(output->info.r.width);
> height = le32_to_cpu(output->info.r.height);
> @@ -369,5 +367,5 @@ void virtio_gpu_modeset_fini(struct virtio_gpu_device
> *vgdev)
> return;
>
> for (i = 0 ; i < vgdev->num_scanouts; ++i)
> - kfree(vgdev->outputs[i].edid);
> + drm_edid_free(vgdev->outputs[i].drm_edid);
> }
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index bb7d86a0c6a1..64c236169db8 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -179,7 +179,7 @@ struct virtio_gpu_output {
> struct drm_encoder enc;
> struct virtio_gpu_display_one info;
> struct virtio_gpu_update_cursor cursor;
> - struct edid *edid;
> + const struct drm_edid *drm_edid;
> int cur_x;
> int cur_y;
> bool needs_modeset;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
> b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index b1a00c0c25a7..0d3d0d09f39b 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -741,21 +741,21 @@ static void virtio_gpu_cmd_get_edid_cb(struct
> virtio_gpu_device *vgdev,
> (struct virtio_gpu_resp_edid *)vbuf->resp_buf;
> uint32_t scanout = le32_to_cpu(cmd->scanout);
> struct virtio_gpu_output *output;
> - struct edid *new_edid, *old_edid;
> + const struct drm_edid *new_edid, *old_edid;
>
> if (scanout >= vgdev->num_scanouts)
> return;
> output = vgdev->outputs + scanout;
>
> - new_edid = drm_do_get_edid(>conn, virtio_get_edid_block,
> resp);
> - drm_connector_update_edid_property(>conn, new_edid);
> + new_edid = drm_edid_read_custom(>conn, virtio_get_edid_block,
> resp);
> + drm_edid_connector_update(>conn, new_edid);
>
> spin_lock(>display_info_lock);
> - old_edid = output->edid;
> - output->edid = new_edid;
> + old_edid = output->drm_edid;
> + output->drm_edid = new_edid;
> spin_unlock(>display_info_lock);
>
> - kfree(old_edid);
> + drm_edid_free(old_edid);
> wake_up(>resp_wq);
> }
>
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [RESEND 4/6] drm/amdgpu: remove amdgpu_connector_edid() and stop using edid_blob_ptr
_0_audio_write_sad_regs(struct
> drm_encoder *encoder)
> return;
> }
>
> - sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), );
> + sad_count = drm_edid_to_sad(amdgpu_connector->edid, );
> if (sad_count < 0)
> DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
> if (sad_count <= 0)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index db20012600f5..05c0df97f01d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1217,7 +1217,7 @@ static void
> dce_v6_0_audio_write_speaker_allocation(struct drm_encoder *encoder)
> return;
> }
>
> - sad_count =
> drm_edid_to_speaker_allocation(amdgpu_connector_edid(connector), );
> + sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid,
> );
> if (sad_count < 0) {
> DRM_ERROR("Couldn't read Speaker Allocation Data Block:
> %d\n", sad_count);
> sad_count = 0;
> @@ -1292,7 +1292,7 @@ static void dce_v6_0_audio_write_sad_regs(struct
> drm_encoder *encoder)
> return;
> }
>
> - sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), );
> + sad_count = drm_edid_to_sad(amdgpu_connector->edid, );
> if (sad_count < 0)
> DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
> if (sad_count <= 0)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 5b56100ec902..dc73e301d937 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1272,7 +1272,7 @@ static void
> dce_v8_0_audio_write_speaker_allocation(struct drm_encoder *encoder)
> return;
> }
>
> - sad_count =
> drm_edid_to_speaker_allocation(amdgpu_connector_edid(connector), );
> + sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid,
> );
> if (sad_count < 0) {
> DRM_ERROR("Couldn't read Speaker Allocation Data Block:
> %d\n", sad_count);
> sad_count = 0;
> @@ -1340,7 +1340,7 @@ static void dce_v8_0_audio_write_sad_regs(struct
> drm_encoder *encoder)
> return;
> }
>
> - sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), );
> + sad_count = drm_edid_to_sad(amdgpu_connector->edid, );
> if (sad_count < 0)
> DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
> if (sad_count <= 0)
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [RESEND 3/6] drm/radeon: remove radeon_connector_edid() and stop using edid_blob_ptr
On Fri, May 10, 2024 at 5:08 PM Jani Nikula wrote:
>
> radeon_connector_edid() copies the EDID from edid_blob_ptr as a side
> effect if radeon_connector->edid isn't initialized. However, everywhere
> that the returned EDID is used, the EDID should have been set
> beforehands.
>
> Only the drm EDID code should look at the EDID property, anyway, so stop
> using it.
>
> Cc: Alex Deucher
> Cc: Christian König
> Cc: Pan, Xinhui
> Cc: amd-...@lists.freedesktop.org
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/radeon/radeon_audio.c | 7 ---
> drivers/gpu/drm/radeon/radeon_connectors.c | 15 ---
> drivers/gpu/drm/radeon/radeon_mode.h | 2 --
> 3 files changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_audio.c
> b/drivers/gpu/drm/radeon/radeon_audio.c
> index 16c10db3ce6f..0bcd767b9f47 100644
> --- a/drivers/gpu/drm/radeon/radeon_audio.c
> +++ b/drivers/gpu/drm/radeon/radeon_audio.c
> @@ -303,6 +303,7 @@ void radeon_audio_endpoint_wreg(struct radeon_device
> *rdev, u32 offset,
> static void radeon_audio_write_sad_regs(struct drm_encoder *encoder)
> {
> struct drm_connector *connector =
> radeon_get_connector_for_encoder(encoder);
> + struct radeon_connector *radeon_connector =
> to_radeon_connector(connector);
> struct radeon_encoder *radeon_encoder = to_radeon_encoder(encoder);
> struct cea_sad *sads;
> int sad_count;
> @@ -310,7 +311,7 @@ static void radeon_audio_write_sad_regs(struct
> drm_encoder *encoder)
> if (!connector)
> return;
>
> - sad_count = drm_edid_to_sad(radeon_connector_edid(connector), );
> + sad_count = drm_edid_to_sad(radeon_connector->edid, );
> if (sad_count < 0)
> DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
> if (sad_count <= 0)
> @@ -326,6 +327,7 @@ static void radeon_audio_write_sad_regs(struct
> drm_encoder *encoder)
> static void radeon_audio_write_speaker_allocation(struct drm_encoder
> *encoder)
> {
> struct drm_connector *connector =
> radeon_get_connector_for_encoder(encoder);
> + struct radeon_connector *radeon_connector =
> to_radeon_connector(connector);
> struct radeon_encoder *radeon_encoder = to_radeon_encoder(encoder);
> u8 *sadb = NULL;
> int sad_count;
> @@ -333,8 +335,7 @@ static void radeon_audio_write_speaker_allocation(struct
> drm_encoder *encoder)
> if (!connector)
> return;
>
> - sad_count =
> drm_edid_to_speaker_allocation(radeon_connector_edid(connector),
> - );
> + sad_count = drm_edid_to_speaker_allocation(radeon_connector->edid,
> );
> if (sad_count < 0) {
> DRM_DEBUG("Couldn't read Speaker Allocation Data Block: %d\n",
> sad_count);
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
> b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 81b5c3c8f658..80879e946342 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -255,21 +255,6 @@ static struct drm_encoder *radeon_find_encoder(struct
> drm_connector *connector,
> return NULL;
> }
>
> -struct edid *radeon_connector_edid(struct drm_connector *connector)
> -{
> - struct radeon_connector *radeon_connector =
> to_radeon_connector(connector);
> - struct drm_property_blob *edid_blob = connector->edid_blob_ptr;
> -
> - if (radeon_connector->edid) {
> - return radeon_connector->edid;
> - } else if (edid_blob) {
> - struct edid *edid = kmemdup(edid_blob->data,
> edid_blob->length, GFP_KERNEL);
> - if (edid)
> - radeon_connector->edid = edid;
> - }
> - return radeon_connector->edid;
> -}
> -
> static void radeon_connector_get_edid(struct drm_connector *connector)
> {
> struct drm_device *dev = connector->dev;
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
> b/drivers/gpu/drm/radeon/radeon_mode.h
> index 546381a5c918..e0a5af180801 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -701,8 +701,6 @@ extern u16
> radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connecto
> extern bool radeon_connector_is_dp12_capable(struct drm_connector
> *connector);
> extern int radeon_get_monitor_bpc(struct drm_connector *connector);
>
> -extern struct edid *radeon_connector_edid(struct drm_connector *connector);
> -
> extern void radeon_connector_hotplug(struct drm_connector *connector);
> extern int radeon_dp_mode_valid_helper(struct drm_connector *connector,
>struct drm_display_mode *mode);
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [RESEND 2/6] drm/radeon: convert to using is_hdmi and has_audio from display info
> - if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
> + if (connector->display_info.is_hdmi) {
> /* hdmi deep color only implemented on DCE4+ */
> if ((bpc > 8) && !ASIC_IS_DCE4(rdev)) {
> DRM_DEBUG("%s: HDMI deep color %d bpc unsupported.
> Using 8 bpc.\n",
> @@ -1478,7 +1478,7 @@ static enum drm_mode_status
> radeon_dvi_mode_valid(struct drm_connector *connecto
> (radeon_connector->connector_object_id ==
> CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_D) ||
> (radeon_connector->connector_object_id ==
> CONNECTOR_OBJECT_ID_HDMI_TYPE_B))
> return MODE_OK;
> - else if (ASIC_IS_DCE6(rdev) &&
> drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
> + else if (ASIC_IS_DCE6(rdev) &&
> connector->display_info.is_hdmi) {
> /* HDMI 1.3+ supports max clock of 340 Mhz */
> if (mode->clock > 34)
> return MODE_CLOCK_HIGH;
> @@ -1774,7 +1774,7 @@ static enum drm_mode_status radeon_dp_mode_valid(struct
> drm_connector *connector
> (radeon_dig_connector->dp_sink_type ==
> CONNECTOR_OBJECT_ID_eDP)) {
> return radeon_dp_mode_valid_helper(connector, mode);
> } else {
> - if (ASIC_IS_DCE6(rdev) &&
> drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
> + if (ASIC_IS_DCE6(rdev) &&
> connector->display_info.is_hdmi) {
> /* HDMI 1.3+ supports max clock of 340 Mhz */
> if (mode->clock > 34)
> return MODE_CLOCK_HIGH;
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c
> b/drivers/gpu/drm/radeon/radeon_display.c
> index 5f1d24d3120c..843383f7237f 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -1722,7 +1722,7 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc
> *crtc,
> (!(mode->flags & DRM_MODE_FLAG_INTERLACE)) &&
> ((radeon_encoder->underscan_type == UNDERSCAN_ON)
> ||
> ((radeon_encoder->underscan_type ==
> UNDERSCAN_AUTO) &&
> -
> drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
> + connector->display_info.is_hdmi &&
> is_hdtv_mode(mode {
> if (radeon_encoder->underscan_hborder != 0)
> radeon_crtc->h_border =
> radeon_encoder->underscan_hborder;
> diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c
> b/drivers/gpu/drm/radeon/radeon_encoders.c
> index 3de3dce9e89d..0f723292409e 100644
> --- a/drivers/gpu/drm/radeon/radeon_encoders.c
> +++ b/drivers/gpu/drm/radeon/radeon_encoders.c
> @@ -386,7 +386,7 @@ bool radeon_dig_monitor_is_duallink(struct drm_encoder
> *encoder,
> case DRM_MODE_CONNECTOR_HDMIB:
> if (radeon_connector->use_digital) {
> /* HDMI 1.3 supports up to 340 Mhz over single link */
> - if (ASIC_IS_DCE6(rdev) &&
> drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
> + if (ASIC_IS_DCE6(rdev) &&
> connector->display_info.is_hdmi) {
> if (pixel_clock > 34)
> return true;
> else
> @@ -408,7 +408,7 @@ bool radeon_dig_monitor_is_duallink(struct drm_encoder
> *encoder,
> return false;
> else {
> /* HDMI 1.3 supports up to 340 Mhz over single link */
> - if (ASIC_IS_DCE6(rdev) &&
> drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
> + if (ASIC_IS_DCE6(rdev) &&
> connector->display_info.is_hdmi) {
> if (pixel_clock > 34)
> return true;
> else
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: drm/bridge: adv7511: Attach next bridge without creating connector
On Mon, May 13, 2024 at 6:30 PM Sui Jingfeng wrote:
>
> Hi,
>
>
> On 5/13/24 16:02, Liu Ying wrote:
> > The connector is created by either this ADV7511 bridge driver or
> > any DRM device driver/previous bridge driver, so this ADV7511
> > bridge driver should not let the next bridge driver create connector.
> >
> > If the next bridge is a HDMI connector, the next bridge driver
> > would fail to attach bridge from display_connector_attach() without
> > the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
> >
> > Add that flag to drm_bridge_attach() function call in
> > adv7511_bridge_attach() to fix the issue.
> >
> > This fixes the issue where the HDMI connector bridge fails to attach
> > to the previous ADV7535 bridge on i.MX8MP EVK platform:
> >
> > [2.216442] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
> > /hdmi-connector to encoder None-37: -22
> > [2.220675] mmc1: SDHCI controller on 30b5.mmc [30b5.mmc] using
> > ADMA
> > [2.226262] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
> > /soc@0/bus@3080/i2c@30a3/hdmi@3d to encoder None-37: -22
> > [2.245204] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
> > /soc@0/bus@32c0/dsi@32e6 to encoder None-37: -22
> > [2.256445] imx-lcdif 32e8.display-controller: error -EINVAL: Failed
> > to attach bridge for endpoint0
> > [2.265850] imx-lcdif 32e8.display-controller: error -EINVAL: Cannot
> > connect bridge
> > [2.274009] imx-lcdif 32e8.display-controller: probe with driver
> > imx-lcdif failed with error -22
> >
> > Fixes: 14b3cdbd0e5b ("drm/bridge: adv7511: make it honour next bridge in
> > DT")
> > Signed-off-by: Liu Ying
> > ---
> > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > index dd21b81bd28f..66ccb61e2a66 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > @@ -953,7 +953,8 @@ static int adv7511_bridge_attach(struct drm_bridge
> > *bridge,
> > int ret = 0;
> >
> > if (adv->next_bridge) {
> > - ret = drm_bridge_attach(bridge->encoder, adv->next_bridge,
> > bridge, flags);
> > + ret = drm_bridge_attach(bridge->encoder, adv->next_bridge,
> > bridge,
> > + flags |
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>
> As a side note, I think, maybe you could do better in the future.
>
> If we know that the KMS display driver side has the HDMI connector
> already created for us, we should pass DRM_BRIDGE_ATTACH_NO_CONNECTOR
> from the root KMS driver side. Which is to forbidden all potential
> drm bridge drivers to create a connector in the middle.
>
> The KMS display driver side could parse the DT to know if there is
> a hdmi connector, or merely just hdmi connector device node, or
> something else.
>
> However, other maintainer and/or reviewer's opinion are of cause
> more valuable. I send a A-b because I thought the bug is urgency
> and it's probably more important to solve this bug first. And
> maybe you can Cc: if you like.
>
Reviewed-by: Robert Foss
Re: [PATCH] drm/bridge: tc358767: Enable FRMSYNC timing generator
On Mon, May 13, 2024 at 4:16 AM Marek Vasut wrote:
>
> TC9595 datasheet Video Path0 Control (VPCTRL0) Register bit FRMSYNC
> description
> says "This bit should be disabled only in video mode transmission where Host
> transmits video timing together with video data and where pixel clock source
> is from DSI clock." . This driver always sources pixel clock from external
> xtal,
> therefore the FRMSYNC bit must always be enabled, enable it.
>
> This fixes an actual issue with DSI-to-DPI mode, where the display would
> randomly show subtle pixel flickering, or wobble, or shimmering. This is
> visible on solid gray color, but the degree of the shimmering differs
> between boots, which makes it hard to debug.
>
> There is a caveat to the FRMSYNC and this bridge pixel PLL, which can only
> generate pixel clock with limited accuracy, it may therefore be necessary
> to reduce the HFP to fit into line length of input pixel data, to avoid any
> possible overflows, which make the output video look striped horizontally.
>
> Signed-off-by: Marek Vasut
> ---
> Cc: Adam Ford
> Cc: Alexander Stein
> Cc: Andrzej Hajda
> Cc: Daniel Vetter
> Cc: David Airlie
> Cc: Frieder Schrempf
> Cc: Jernej Skrabec
> Cc: Jonas Karlman
> Cc: Laurent Pinchart
> Cc: Lucas Stach
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Michael Walle
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Thomas Zimmermann
> Cc: dri-devel@lists.freedesktop.org
> Cc: ker...@dh-electronics.com
> ---
> drivers/gpu/drm/bridge/tc358767.c | 23 ---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index 166f9a3e9622d..fe2b93546eaef 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -382,6 +382,9 @@ struct tc_data {
>
> /* HPD pin number (0 or 1) or -ENODEV */
> int hpd_pin;
> +
> + /* Number of pixels to subtract from a line due to pixel clock delta
> */
> + u32 line_pixel_subtract;
> };
>
> static inline struct tc_data *aux_to_tc(struct drm_dp_aux *a)
> @@ -577,6 +580,11 @@ static int tc_pllupdate(struct tc_data *tc, unsigned int
> pllctrl)
> return 0;
> }
>
> +static u32 div64_round_up(u64 v, u32 d)
> +{
> + return div_u64(v + d - 1, d);
> +}
> +
Could the DIV_ROUND_UP macro in math,h replace this function?
> static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, u32 pixelclock)
> {
> int ret;
> @@ -658,8 +666,11 @@ static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk,
> u32 pixelclock)
> return -EINVAL;
> }
>
> - dev_dbg(tc->dev, "PLL: got %d, delta %d\n", best_pixelclock,
> - best_delta);
> + tc->line_pixel_subtract = tc->mode.htotal -
> + div64_round_up(tc->mode.htotal * (u64)best_pixelclock,
> pixelclock);
> +
> + dev_dbg(tc->dev, "PLL: got %d, delta %d (subtract %d px)\n",
> best_pixelclock,
> + best_delta, tc->line_pixel_subtract);
> dev_dbg(tc->dev, "PLL: %d / %d / %d * %d / %d\n", refclk,
> ext_div[best_pre], best_div, best_mul, ext_div[best_post]);
>
> @@ -885,6 +896,12 @@ static int tc_set_common_video_mode(struct tc_data *tc,
> upper_margin, lower_margin, vsync_len);
> dev_dbg(tc->dev, "total: %dx%d\n", mode->htotal, mode->vtotal);
>
> + if (right_margin > tc->line_pixel_subtract) {
> + right_margin -= tc->line_pixel_subtract;
> + } else {
> + dev_err(tc->dev, "Bridge pixel clock too slow for mode\n");
> + right_margin = 0;
> + }
>
> /*
> * LCD Ctl Frame Size
> @@ -894,7 +911,7 @@ static int tc_set_common_video_mode(struct tc_data *tc,
> */
> ret = regmap_write(tc->regmap, VPCTRL0,
> FIELD_PREP(VSDELAY, right_margin + 10) |
> - OPXLFMT_RGB888 | FRMSYNC_DISABLED | MSF_DISABLED);
> + OPXLFMT_RGB888 | FRMSYNC_ENABLED | MSF_DISABLED);
> if (ret)
> return ret;
>
> --
> 2.43.0
>
With the above question resolved, feel free to add my r-b. I will
snooze this for a week before looking at applying this.
Reviewed-by: Robert Foss
Re: [PATCH v2 00/12] Remove redundant checks on existence of 'bridge->encoder'
On Mon, 13 May 2024 23:30:57 +0800, Sui Jingfeng wrote: > The checks on the existence of bridge->encoder in the implementation of > drm_bridge_funcs::attach() is not necessary, as it has already been checked > in the drm_bridge_attach() function call by previous bridge or KMS driver. > The drm_bridge_attach() will quit with a negative error code returned if > it fails for some reasons, hence, it is guaranteed that the .encoder member > of the drm_bridge instance is not NULL when various bridge attach functions > are called. > > [...] Applied, thanks! [01/12] drm/bridge: simple-bridge: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=f0a83a2cf9eb [02/12] drm/bridge: tfp410: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=482ade3ec1c5 [03/12] drm/bridge: nxp-ptn3460: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0f4bca4e1be3 [04/12] drm/bridge: panel: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a8f856bf054a [05/12] drm/bridge: it6505: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=8761a39e3f9d [06/12] drm/bridge: adv7511: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=41e6ed85e457 [07/12] drm/bridge: cdns-mhdp8546: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=b24fd6e9eb66 [08/12] drm/bridge: megachips-stdp-ge-b850v3-fw: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0a59deb2fedb [09/12] drm/bridge: synopsys: dw-mipi-dsi: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=80221a89ff95 [10/12] drm/bridge: lt9611uxc: Remove a redundant check on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=91942a37ebba [11/12] drm/bridge: imx: Remove redundant checks on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ec74951a7507 [12/12] drm/bridge: analogix: Remove redundant checks on existence of bridge->encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=591255853a37 Rob
Re: [PATCH] drm/drm-bridge.c: Drop conditionals around of_node pointers
On Wed, 8 May 2024 02:00:00 +0800, Sui Jingfeng wrote:
> Having conditional around the of_node pointer of the drm_bridge structure
> is not necessary, since drm_bridge structure always has the of_node as its
> member.
>
> Let's drop the conditional to get a better looks, please also note that
> this is following the already accepted commitments. see commit d8dfccde2709
> ("drm/bridge: Drop conditionals around of_node pointers") for reference.
>
> [...]
Applied, thanks!
[1/1] drm/drm-bridge.c: Drop conditionals around of_node pointers
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ad3323a6ccb7
Rob
Re: [PATCH 00/14] improve Analogix DP AUX channel handling
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote: > > Currently the AUX channel support in the Analogix DP driver is severely > limited as the AUX block of the bridge is only initialized when the video > link is to be enabled. This is okay for the purposes of link training, > but does not allow to detect displays by probing for EDID. This series > reworks the driver to allow AUX transactions before the video link is > active. > > As this requires to rework some of the controller initialization and > also handling of both internal and external clocks, the series includes > quite a few changes to add better runtime PM handling. > > Lucas Stach (14): > drm/bridge: analogix_dp: remove unused platform power_on_end callback > drm/rockchip: analogix_dp: add runtime PM handling > drm/bridge: analogix_dp: register AUX bus after enabling runtime PM > drm/bridge: analogix_dp: handle clock via runtime PM > drm/bridge: analogix_dp: remove unused analogix_dp_remove > drm/bridge: analogix_dp: remove clk handling from > analogix_dp_set_bridge > drm/bridge: analogix_dp: move platform and PHY power handling into > runtime PM > drm/bridge: analogix_dp: move basic controller init into runtime PM > drm/bridge: analogix_dp: remove PLL lock check from > analogix_dp_config_video > drm/bridge: analogix_dp: move macro reset after link bandwidth setting > drm/bridge: analogix_dp: don't wait for PLL lock too early > drm/bridge: analogix_dp: simplify and correct PLL lock checks > drm/bridge: analogix_dp: only read AUX status when an error occured > drm/bridge: analogix_dp: handle AUX transfer timeouts > > .../drm/bridge/analogix/analogix_dp_core.c| 196 -- > .../drm/bridge/analogix/analogix_dp_core.h| 7 +- > .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 38 ++-- > .../gpu/drm/bridge/analogix/analogix_dp_reg.h | 9 + > drivers/gpu/drm/exynos/exynos_dp.c| 5 +- > .../gpu/drm/rockchip/analogix_dp-rockchip.c | 26 +-- > include/drm/bridge/analogix_dp.h | 4 +- > 7 files changed, 120 insertions(+), 165 deletions(-) > > -- > 2.39.2 > There are some checkpatch --strict warnings, and the patch 10/14 does not apply. Other than that the series looks very good. Maybe rebase on drm-misc/drm-misc-next, fix the applicable checkpatch --strict warnings and send a new version of this series. Then the last patches can be reviewed.
Re: [PATCH 10/14] drm/bridge: analogix_dp: move macro reset after link bandwidth setting
On Fri, May 3, 2024 at 5:13 PM Lucas Stach wrote: > > Setting the link bandwidth may change the PLL parameters, which will cause > the PLL to go out of lock, so make sure to apply the MACRO_RST, which > according to the comment is required to be pulsed after the PLL is locked. > > Signed-off-by: Lucas Stach > --- > .../gpu/drm/bridge/analogix/analogix_dp_core.c | 18 ++ > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index b4a47311cfe8..736b2ed745e1 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > @@ -243,6 +243,11 @@ static int analogix_dp_link_start(struct > analogix_dp_device *dp) > > /* Set link rate and count as you want to establish*/ > analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate); > + /* > +* MACRO_RST must be applied after the PLL_LOCK to avoid > +* the DP inter pair skew issue for at least 10 us > +*/ > + analogix_dp_reset_macro(dp); > analogix_dp_set_lane_count(dp, dp->link_train.lane_count); > > /* Setup RX configuration */ > @@ -565,12 +570,6 @@ static int analogix_dp_full_link_train(struct > analogix_dp_device *dp, > int retval = 0; > bool training_finished = false; > > - /* > -* MACRO_RST must be applied after the PLL_LOCK to avoid > -* the DP inter pair skew issue for at least 10 us > -*/ > - analogix_dp_reset_macro(dp); > - > /* Initialize by reading RX's DPCD */ > analogix_dp_get_max_rx_bandwidth(dp, >link_train.link_rate); > analogix_dp_get_max_rx_lane_count(dp, >link_train.lane_count); > @@ -637,9 +636,12 @@ static int analogix_dp_fast_link_train(struct > analogix_dp_device *dp) > u8 link_align, link_status[2]; > enum pll_status status; > > - analogix_dp_reset_macro(dp); > - > analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate); > + /* > +* MACRO_RST must be applied after the PLL_LOCK to avoid > +* the DP inter pair skew issue for at least 10 us > +*/ > + analogix_dp_reset_macro(dp); > analogix_dp_set_lane_count(dp, dp->link_train.lane_count); > analogix_dp_set_lane_link_training(dp); > > -- > 2.39.2 > This patch does not apply on drm-misc/drm-misc-next as far as I can tell.
Re: [PATCH 09/14] drm/bridge: analogix_dp: remove PLL lock check from analogix_dp_config_video
On Fri, May 3, 2024 at 5:13 PM Lucas Stach wrote:
>
> This check is way too late in the DP enable flow. The PLL must be locked
> much earlier, before any link training can happen. If the PLL is unlocked
> at that point in time there is something seriously wrong in the enable flow.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index fdb2c2a2b69a..b4a47311cfe8 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -720,11 +720,6 @@ static int analogix_dp_config_video(struct
> analogix_dp_device *dp)
>
> analogix_dp_set_video_color_format(dp);
>
> - if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> - dev_err(dp->dev, "PLL is not locked yet.\n");
> - return -EINVAL;
> - }
> -
> for (;;) {
> timeout_loop++;
> if (analogix_dp_is_slave_video_stream_clock_on(dp) == 0)
> --
> 2.39.2
>
Reviewed-by: Robet Foss
Re: [PATCH 08/14] drm/bridge: analogix_dp: move basic controller init into runtime PM
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> Make sure the controller is in a basic working state after runtime
> resume. Keep the analog function enable in the mode set path as this
> enables parts of the PHY that are only required to be powered when
> there is a data stream being sent out.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 14 +-
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 3281c00a39cd..fdb2c2a2b69a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -41,10 +41,8 @@ struct bridge_init {
> struct device_node *node;
> };
>
> -static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> +static void analogix_dp_init_dp(struct analogix_dp_device *dp)
> {
> - int ret;
> -
> analogix_dp_reset(dp);
>
> analogix_dp_swreset(dp);
> @@ -56,13 +54,9 @@ static int analogix_dp_init_dp(struct analogix_dp_device
> *dp)
> analogix_dp_enable_sw_function(dp);
>
> analogix_dp_config_interrupt(dp);
> - ret = analogix_dp_init_analog_func(dp);
> - if (ret)
> - return ret;
>
> analogix_dp_init_hpd(dp);
> analogix_dp_init_aux(dp);
> - return 0;
> }
>
> static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
> @@ -1258,9 +1252,9 @@ static int analogix_dp_set_bridge(struct
> analogix_dp_device *dp)
>
> pm_runtime_get_sync(dp->dev);
>
> - ret = analogix_dp_init_dp(dp);
> + ret = analogix_dp_init_analog_func(dp);
> if (ret)
> - goto out_dp_init;
> + return ret;
>
> /*
> * According to DP spec v1.3 chap 3.5.1.2 Link Training,
> @@ -1726,6 +1720,8 @@ int analogix_dp_resume(struct analogix_dp_device *dp)
>
> phy_power_on(dp->phy);
>
> + analogix_dp_init_dp(dp);
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(analogix_dp_resume);
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 07/14] drm/bridge: analogix_dp: move platform and PHY power handling into runtime PM
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> Platform and PHY power isn't only required when the actual display data
> stream is active, but may be required earlier to support AUX channel
> transactions. Move them into the runtime PM calls, so they are properly
> managed whenever various other parts of the driver need them to be active.
>
> Signed-off-by: Lucas Stach
> ---
> .../drm/bridge/analogix/analogix_dp_core.c| 23 ---
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 311e1e67486d..3281c00a39cd 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1258,11 +1258,6 @@ static int analogix_dp_set_bridge(struct
> analogix_dp_device *dp)
>
> pm_runtime_get_sync(dp->dev);
>
> - if (dp->plat_data->power_on)
> - dp->plat_data->power_on(dp->plat_data);
> -
> - phy_power_on(dp->phy);
> -
> ret = analogix_dp_init_dp(dp);
> if (ret)
> goto out_dp_init;
> @@ -1288,10 +1283,6 @@ static int analogix_dp_set_bridge(struct
> analogix_dp_device *dp)
> return 0;
>
> out_dp_init:
> - phy_power_off(dp->phy);
> - if (dp->plat_data->power_off)
> - dp->plat_data->power_off(dp->plat_data);
> -
> pm_runtime_put_sync(dp->dev);
>
> return ret;
> @@ -1354,11 +1345,7 @@ static void analogix_dp_bridge_disable(struct
> drm_bridge *bridge)
>
> disable_irq(dp->irq);
>
> - if (dp->plat_data->power_off)
> - dp->plat_data->power_off(dp->plat_data);
> -
> analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
> - phy_power_off(dp->phy);
>
> pm_runtime_put_sync(dp->dev);
>
> @@ -1713,6 +1700,11 @@ EXPORT_SYMBOL_GPL(analogix_dp_probe);
>
> int analogix_dp_suspend(struct analogix_dp_device *dp)
> {
> + phy_power_off(dp->phy);
> +
> + if (dp->plat_data->power_off)
> + dp->plat_data->power_off(dp->plat_data);
> +
> clk_disable_unprepare(dp->clock);
>
> return 0;
> @@ -1729,6 +1721,11 @@ int analogix_dp_resume(struct analogix_dp_device *dp)
> return ret;
> }
>
> + if (dp->plat_data->power_on)
> + dp->plat_data->power_on(dp->plat_data);
> +
> + phy_power_on(dp->phy);
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(analogix_dp_resume);
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 06/14] drm/bridge: analogix_dp: remove clk handling from analogix_dp_set_bridge
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> The clock is already managed by runtime PM, which is properly invoked
> from the analogix_dp_set_bridge function, so there is no need for an
> additional reference.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +--
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 9f1dfa6f2175..311e1e67486d 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1258,12 +1258,6 @@ static int analogix_dp_set_bridge(struct
> analogix_dp_device *dp)
>
> pm_runtime_get_sync(dp->dev);
>
> - ret = clk_prepare_enable(dp->clock);
> - if (ret < 0) {
> - DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n",
> ret);
> - goto out_dp_clk_pre;
> - }
> -
> if (dp->plat_data->power_on)
> dp->plat_data->power_on(dp->plat_data);
>
> @@ -1297,8 +1291,7 @@ static int analogix_dp_set_bridge(struct
> analogix_dp_device *dp)
> phy_power_off(dp->phy);
> if (dp->plat_data->power_off)
> dp->plat_data->power_off(dp->plat_data);
> - clk_disable_unprepare(dp->clock);
> -out_dp_clk_pre:
> +
> pm_runtime_put_sync(dp->dev);
>
> return ret;
> @@ -1367,8 +1360,6 @@ static void analogix_dp_bridge_disable(struct
> drm_bridge *bridge)
> analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
> phy_power_off(dp->phy);
>
> - clk_disable_unprepare(dp->clock);
> -
> pm_runtime_put_sync(dp->dev);
>
> ret = analogix_dp_prepare_panel(dp, false, true);
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 05/14] drm/bridge: analogix_dp: remove unused analogix_dp_remove
On Fri, May 3, 2024 at 5:13 PM Lucas Stach wrote:
>
> Now that the clock is handled dynamically through
> analogix_dp_resume/suspend and it isn't statically enabled in the
> driver probe routine, there is no need for the remove function anymore.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> drivers/gpu/drm/exynos/exynos_dp.c | 3 ---
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 9 +
> include/drm/bridge/analogix_dp.h | 1 -
> 4 files changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 9e3308257586..9f1dfa6f2175 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1813,11 +1813,6 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
> }
> EXPORT_SYMBOL_GPL(analogix_dp_unbind);
>
> -void analogix_dp_remove(struct analogix_dp_device *dp)
> -{
> -}
> -EXPORT_SYMBOL_GPL(analogix_dp_remove);
> -
> int analogix_dp_start_crc(struct drm_connector *connector)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
> b/drivers/gpu/drm/exynos/exynos_dp.c
> index 30c8750187ad..097f8c4617de 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -251,10 +251,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
>
> static void exynos_dp_remove(struct platform_device *pdev)
> {
> - struct exynos_dp_device *dp = platform_get_drvdata(pdev);
> -
> component_del(>dev, _dp_ops);
> - analogix_dp_remove(dp->adp);
> }
>
> static int exynos_dp_suspend(struct device *dev)
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 8214265f1497..362c7951ca4a 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -414,21 +414,14 @@ static int rockchip_dp_probe(struct platform_device
> *pdev)
>
> ret = component_add(dev, _dp_component_ops);
> if (ret)
> - goto err_dp_remove;
> + return ret;
>
> return 0;
> -
> -err_dp_remove:
> - analogix_dp_remove(dp->adp);
> - return ret;
> }
>
> static void rockchip_dp_remove(struct platform_device *pdev)
> {
> - struct rockchip_dp_device *dp = platform_get_drvdata(pdev);
> -
> component_del(>dev, _dp_component_ops);
> - analogix_dp_remove(dp->adp);
> }
>
> static int rockchip_dp_suspend(struct device *dev)
> diff --git a/include/drm/bridge/analogix_dp.h
> b/include/drm/bridge/analogix_dp.h
> index 8709b6a74c0f..6002c5666031 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -44,7 +44,6 @@ struct analogix_dp_device *
> analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data
> *plat_data);
> int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device
> *drm_dev);
> void analogix_dp_unbind(struct analogix_dp_device *dp);
> -void analogix_dp_remove(struct analogix_dp_device *dp);
>
> int analogix_dp_start_crc(struct drm_connector *connector);
> int analogix_dp_stop_crc(struct drm_connector *connector);
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 04/14] drm/bridge: analogix_dp: handle clock via runtime PM
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> There is no reason to enable the controller clock in driver probe, as
> there is no HW initialization done in this function. Instead rely on
> either runtime PM to handle the controller clock or statically enable
> it in the driver bind routine, after which real hardware access is
> required to work.
>
> Signed-off-by: Lucas Stach
> ---
> .../drm/bridge/analogix/analogix_dp_core.c| 78 +++
> 1 file changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 0af2a70ae5bf..9e3308257586 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1658,8 +1658,6 @@ analogix_dp_probe(struct device *dev, struct
> analogix_dp_plat_data *plat_data)
> return ERR_CAST(dp->clock);
> }
>
> - clk_prepare_enable(dp->clock);
> -
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> dp->reg_base = devm_ioremap_resource(>dev, res);
> @@ -1721,6 +1719,29 @@ analogix_dp_probe(struct device *dev, struct
> analogix_dp_plat_data *plat_data)
> }
> EXPORT_SYMBOL_GPL(analogix_dp_probe);
>
> +
> +int analogix_dp_suspend(struct analogix_dp_device *dp)
> +{
> + clk_disable_unprepare(dp->clock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_suspend);
> +
> +int analogix_dp_resume(struct analogix_dp_device *dp)
> +{
> + int ret;
> +
> + ret = clk_prepare_enable(dp->clock);
> + if (ret < 0) {
> + DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n",
> ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_resume);
> +
> int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device
> *drm_dev)
> {
> int ret;
> @@ -1728,9 +1749,15 @@ int analogix_dp_bind(struct analogix_dp_device *dp,
> struct drm_device *drm_dev)
> dp->drm_dev = drm_dev;
> dp->encoder = dp->plat_data->encoder;
>
> - pm_runtime_use_autosuspend(dp->dev);
> - pm_runtime_set_autosuspend_delay(dp->dev, 100);
> - pm_runtime_enable(dp->dev);
> + if (IS_ENABLED(CONFIG_PM)) {
> + pm_runtime_use_autosuspend(dp->dev);
> + pm_runtime_set_autosuspend_delay(dp->dev, 100);
> + pm_runtime_enable(dp->dev);
> + } else {
> + ret = analogix_dp_resume(dp);
> + if (ret)
> + return ret;
> + }
>
> dp->aux.name = "DP-AUX";
> dp->aux.transfer = analogix_dpaux_transfer;
> @@ -1754,8 +1781,12 @@ int analogix_dp_bind(struct analogix_dp_device *dp,
> struct drm_device *drm_dev)
> err_unregister_aux:
> drm_dp_aux_unregister(>aux);
> err_disable_pm_runtime:
> - pm_runtime_dont_use_autosuspend(dp->dev);
> - pm_runtime_disable(dp->dev);
> + if (IS_ENABLED(CONFIG_PM)) {
> + pm_runtime_dont_use_autosuspend(dp->dev);
> + pm_runtime_disable(dp->dev);
> + } else {
> + analogix_dp_suspend(dp);
> + }
>
> return ret;
> }
> @@ -1772,40 +1803,21 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
> }
>
> drm_dp_aux_unregister(>aux);
> - pm_runtime_dont_use_autosuspend(dp->dev);
> - pm_runtime_disable(dp->dev);
> +
> + if (IS_ENABLED(CONFIG_PM)) {
> + pm_runtime_dont_use_autosuspend(dp->dev);
> + pm_runtime_disable(dp->dev);
> + } else {
> + analogix_dp_suspend(dp);
> + }
> }
> EXPORT_SYMBOL_GPL(analogix_dp_unbind);
>
> void analogix_dp_remove(struct analogix_dp_device *dp)
> {
> - clk_disable_unprepare(dp->clock);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_remove);
>
> -#ifdef CONFIG_PM
> -int analogix_dp_suspend(struct analogix_dp_device *dp)
> -{
> - clk_disable_unprepare(dp->clock);
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(analogix_dp_suspend);
> -
> -int analogix_dp_resume(struct analogix_dp_device *dp)
> -{
> - int ret;
> -
> - ret = clk_prepare_enable(dp->clock);
> - if (ret < 0) {
> - DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n",
> ret);
> - return ret;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(analogix_dp_resume);
> -#endif
> -
> int analogix_dp_start_crc(struct drm_connector *connector)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 03/14] drm/bridge: analogix_dp: register AUX bus after enabling runtime PM
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> AUX transactions require the controller to be in working state and
> take a runtime PM reference. To avoid potential races beween the
> first transactions on the bus and runtime PM being set up, move the
> AUX registration behind the runtime PM setup.
>
> Signed-off-by: Lucas Stach
> ---
> .../drm/bridge/analogix/analogix_dp_core.c| 19 +++
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index b39721588980..0af2a70ae5bf 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1728,31 +1728,34 @@ int analogix_dp_bind(struct analogix_dp_device *dp,
> struct drm_device *drm_dev)
> dp->drm_dev = drm_dev;
> dp->encoder = dp->plat_data->encoder;
>
> + pm_runtime_use_autosuspend(dp->dev);
> + pm_runtime_set_autosuspend_delay(dp->dev, 100);
> + pm_runtime_enable(dp->dev);
> +
> dp->aux.name = "DP-AUX";
> dp->aux.transfer = analogix_dpaux_transfer;
> dp->aux.dev = dp->dev;
> dp->aux.drm_dev = drm_dev;
>
> ret = drm_dp_aux_register(>aux);
> - if (ret)
> - return ret;
> -
> - pm_runtime_use_autosuspend(dp->dev);
> - pm_runtime_set_autosuspend_delay(dp->dev, 100);
> - pm_runtime_enable(dp->dev);
> + if (ret) {
> + DRM_ERROR("failed to register AUX (%d)\n", ret);
> + goto err_disable_pm_runtime;
> + }
>
> ret = analogix_dp_create_bridge(drm_dev, dp);
> if (ret) {
> DRM_ERROR("failed to create bridge (%d)\n", ret);
> - goto err_disable_pm_runtime;
> + goto err_unregister_aux;
> }
>
> return 0;
>
> +err_unregister_aux:
> + drm_dp_aux_unregister(>aux);
> err_disable_pm_runtime:
> pm_runtime_dont_use_autosuspend(dp->dev);
> pm_runtime_disable(dp->dev);
> - drm_dp_aux_unregister(>aux);
>
> return ret;
> }
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 02/14] drm/rockchip: analogix_dp: add runtime PM handling
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> Hook up the runtime PM suspend/resume paths to make the rockchip
> glue behave more like the exynos one. The same suspend/resume
> functions are used for system sleep via the runtime PM force
> suspend/resume.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 13 -
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index baeb41875a4b..8214265f1497 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -430,7 +431,6 @@ static void rockchip_dp_remove(struct platform_device
> *pdev)
> analogix_dp_remove(dp->adp);
> }
>
> -#ifdef CONFIG_PM_SLEEP
> static int rockchip_dp_suspend(struct device *dev)
> {
> struct rockchip_dp_device *dp = dev_get_drvdata(dev);
> @@ -450,14 +450,9 @@ static int rockchip_dp_resume(struct device *dev)
>
> return analogix_dp_resume(dp->adp);
> }
> -#endif
>
> -static const struct dev_pm_ops rockchip_dp_pm_ops = {
> -#ifdef CONFIG_PM_SLEEP
> - .suspend_late = rockchip_dp_suspend,
> - .resume_early = rockchip_dp_resume,
> -#endif
> -};
> +static DEFINE_RUNTIME_DEV_PM_OPS(rockchip_dp_pm_ops, rockchip_dp_suspend,
> + rockchip_dp_resume, NULL);
>
> static const struct rockchip_dp_chip_data rk3399_edp = {
> .lcdsel_grf_reg = RK3399_GRF_SOC_CON20,
> @@ -485,7 +480,7 @@ struct platform_driver rockchip_dp_driver = {
> .remove_new = rockchip_dp_remove,
> .driver = {
>.name = "rockchip-dp",
> - .pm = _dp_pm_ops,
> + .pm = pm_ptr(_dp_pm_ops),
>.of_match_table = rockchip_dp_dt_ids,
> },
> };
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 01/14] drm/bridge: analogix_dp: remove unused platform power_on_end callback
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> This isn't used, but gives the impression of the power on and power off
> platform calls being non-symmetrical. Remove the unused callback and
> rename the power_on_start to simplay power_on.
s/simplay/simply
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 7 ++-
> drivers/gpu/drm/exynos/exynos_dp.c | 2 +-
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 4 ++--
> include/drm/bridge/analogix_dp.h | 3 +--
> 4 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 98454f0af90e..b39721588980 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1264,8 +1264,8 @@ static int analogix_dp_set_bridge(struct
> analogix_dp_device *dp)
> goto out_dp_clk_pre;
> }
>
> - if (dp->plat_data->power_on_start)
> - dp->plat_data->power_on_start(dp->plat_data);
> + if (dp->plat_data->power_on)
> + dp->plat_data->power_on(dp->plat_data);
>
> phy_power_on(dp->phy);
>
> @@ -1290,9 +1290,6 @@ static int analogix_dp_set_bridge(struct
> analogix_dp_device *dp)
> goto out_dp_init;
> }
>
> - if (dp->plat_data->power_on_end)
> - dp->plat_data->power_on_end(dp->plat_data);
> -
> enable_irq(dp->irq);
> return 0;
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
> b/drivers/gpu/drm/exynos/exynos_dp.c
> index f48c4343f469..30c8750187ad 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -233,7 +233,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
> /* The remote port can be either a panel or a bridge */
> dp->plat_data.panel = panel;
> dp->plat_data.dev_type = EXYNOS_DP;
> - dp->plat_data.power_on_start = exynos_dp_poweron;
> + dp->plat_data.power_on = exynos_dp_poweron;
> dp->plat_data.power_off = exynos_dp_poweroff;
> dp->plat_data.attach = exynos_dp_bridge_attach;
> dp->plat_data.get_modes = exynos_dp_get_modes;
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 7069a3d4d581..baeb41875a4b 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -92,7 +92,7 @@ static int rockchip_dp_pre_init(struct rockchip_dp_device
> *dp)
> return 0;
> }
>
> -static int rockchip_dp_poweron_start(struct analogix_dp_plat_data *plat_data)
> +static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data)
> {
> struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
> int ret;
> @@ -397,7 +397,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
> dp->data = dp_data;
> dp->plat_data.panel = panel;
> dp->plat_data.dev_type = dp->data->chip_type;
> - dp->plat_data.power_on_start = rockchip_dp_poweron_start;
> + dp->plat_data.power_on = rockchip_dp_poweron;
> dp->plat_data.power_off = rockchip_dp_powerdown;
> dp->plat_data.get_modes = rockchip_dp_get_modes;
>
> diff --git a/include/drm/bridge/analogix_dp.h
> b/include/drm/bridge/analogix_dp.h
> index b0dcc07334a1..8709b6a74c0f 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -29,8 +29,7 @@ struct analogix_dp_plat_data {
> struct drm_connector *connector;
> bool skip_connector;
>
> - int (*power_on_start)(struct analogix_dp_plat_data *);
> - int (*power_on_end)(struct analogix_dp_plat_data *);
> + int (*power_on)(struct analogix_dp_plat_data *);
> int (*power_off)(struct analogix_dp_plat_data *);
> int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *,
> struct drm_connector *);
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 2/2] drm/bridge: anx7625: Change TDM setting accroding to dt property
On Thu, May 2, 2024 at 11:03 AM Hsin-Te Yuan wrote:
>
> For some SoCs, the TDM setting is not to shift the first audio data bit,
> which is not the default setting of anx7625. In such cases, the TDM
> setting should be changed according to the device tree property.
>
> Signed-off-by: Hsin-Te Yuan
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 8
> drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 29d91493b101a..538edddf313c9 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1709,6 +1709,9 @@ static int anx7625_parse_dt(struct device *dev,
> if (of_property_read_bool(np, "analogix,audio-enable"))
> pdata->audio_en = 1;
>
> + if(!of_property_read_bool(np, "no-shift-audio-data"))
> + pdata->shift_audio_data = 1;
checkpatch --strict reports this:
ERROR: space required before the open parenthesis '('
#27: FILE: drivers/gpu/drm/bridge/analogix/anx7625.c:1712:
+if(!of_property_read_bool(np, "no-shift-audio-data"))
> +
> return 0;
> }
>
> @@ -1866,6 +1869,11 @@ static int anx7625_audio_hw_params(struct device *dev,
> void *data,
>~TDM_SLAVE_MODE,
>I2S_SLAVE_MODE);
>
> + if (!ctx->pdata.shift_audio_data)
> + ret |= anx7625_write_or(ctx, ctx->i2c.tx_p2_client,
> + AUDIO_CONTROL_REGISTER,
> + TDM_TIMING_MODE);
> +
> /* Word length */
> switch (params->sample_width) {
> case 16:
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
> b/drivers/gpu/drm/bridge/analogix/anx7625.h
> index 39ed35d338363..41b395725913a 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> @@ -441,6 +441,7 @@ struct anx7625_platform_data {
> u8 lane1_reg_data[DP_TX_SWING_REG_CNT];
> u32 low_power_mode;
> struct device_node *mipi_host_node;
> + int shift_audio_data;
> };
>
> struct anx7625_i2c_client {
>
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
>
Re: [PATCH RESEND] Revert "drm/bridge: ti-sn65dsi83: Fix enable error path"
On Fri, 26 Apr 2024 14:22:59 +0200, Luca Ceresoli wrote: > This reverts commit 8a91b29f1f50ce7742cdbe5cf11d17f128511f3f. > > The regulator_disable() added by the original commit solves one kind of > regulator imbalance but adds another one as it allows the regulator to be > disabled one more time than it is enabled in the following scenario: > > 1. Start video pipeline -> sn65dsi83_atomic_pre_enable -> regulator_enable > 2. PLL lock fails -> regulator_disable > 3. Stop video pipeline -> sn65dsi83_atomic_disable -> regulator_disable > > [...] Applied, thanks! [1/1] Revert "drm/bridge: ti-sn65dsi83: Fix enable error path" https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2940ee03b232 Rob
Re: [PATCH V2] drm/bridge: imx: Fix unmet depenency for PHY_FSL_SAMSUNG_HDMI_PHY
On Mon, 22 Apr 2024 05:33:52 -0500, Adam Ford wrote: > When enabling i.MX8MP DWC HDMI driver, it automatically selects > PHY_FSL_SAMSUNG_HDMI_PHY, since it wont' work without the phy. > This may cause some Kconfig warnings during various build tests. > Fix this by implying the phy instead of selecting the phy. > > To prevent this from happening with the DRM_IMX8MP_HDMI_PVI, also > imply it instead of selecting it. > > [...] Applied, thanks! [1/1] drm/bridge: imx: Fix unmet depenency for PHY_FSL_SAMSUNG_HDMI_PHY https://cgit.freedesktop.org/drm/drm-misc/commit/?id=cbdbd9ca718e Rob
Re: [PATCH 1/2] drm/print: drop include debugfs.h and include where needed
On Mon, Apr 22, 2024 at 2:10 PM Jani Nikula wrote: > > Surprisingly many places depend on debugfs.h to be included via > drm_print.h. Fix them. > > v3: Also fix armada, ite-it6505, imagination, msm, sti, vc4, and xe > > v2: Also fix ivpu and vmwgfx > > Reviewed-by: Andrzej Hajda > Acked-by: Maxime Ripard > Link: > https://patchwork.freedesktop.org/patch/msgid/20240410141434.157908-1-jani.nik...@intel.com > Signed-off-by: Jani Nikula > > --- > > Cc: Jacek Lawrynowicz > Cc: Stanislaw Gruszka > Cc: Oded Gabbay > Cc: Russell King > Cc: David Airlie > Cc: Daniel Vetter > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: Jani Nikula > Cc: Rodrigo Vivi > Cc: Joonas Lahtinen > Cc: Tvrtko Ursulin > Cc: Frank Binns > Cc: Matt Coster > Cc: Rob Clark > Cc: Abhinav Kumar > Cc: Dmitry Baryshkov > Cc: Sean Paul > Cc: Marijn Suijten > Cc: Karol Herbst > Cc: Lyude Paul > Cc: Danilo Krummrich > Cc: Alex Deucher > Cc: "Christian König" > Cc: "Pan, Xinhui" > Cc: Alain Volmat > Cc: Huang Rui > Cc: Zack Rusin > Cc: Broadcom internal kernel review list > > Cc: Lucas De Marchi > Cc: "Thomas Hellström" > Cc: dri-devel@lists.freedesktop.org > Cc: intel-...@lists.freedesktop.org > Cc: intel...@lists.freedesktop.org > Cc: linux-arm-...@vger.kernel.org > Cc: freedr...@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > Cc: amd-...@lists.freedesktop.org > --- > drivers/accel/ivpu/ivpu_debugfs.c | 2 ++ > drivers/gpu/drm/armada/armada_debugfs.c | 1 + > drivers/gpu/drm/bridge/ite-it6505.c | 1 + > drivers/gpu/drm/bridge/panel.c | 2 ++ > drivers/gpu/drm/drm_print.c | 6 +++--- > drivers/gpu/drm/i915/display/intel_dmc.c| 1 + > drivers/gpu/drm/imagination/pvr_fw_trace.c | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 ++ > drivers/gpu/drm/nouveau/dispnv50/crc.c | 2 ++ > drivers/gpu/drm/radeon/r100.c | 1 + > drivers/gpu/drm/radeon/r300.c | 1 + > drivers/gpu/drm/radeon/r420.c | 1 + > drivers/gpu/drm/radeon/r600.c | 3 ++- > drivers/gpu/drm/radeon/radeon_fence.c | 1 + > drivers/gpu/drm/radeon/radeon_gem.c | 1 + > drivers/gpu/drm/radeon/radeon_ib.c | 2 ++ > drivers/gpu/drm/radeon/radeon_pm.c | 1 + > drivers/gpu/drm/radeon/radeon_ring.c| 2 ++ > drivers/gpu/drm/radeon/radeon_ttm.c | 1 + > drivers/gpu/drm/radeon/rs400.c | 1 + > drivers/gpu/drm/radeon/rv515.c | 1 + > drivers/gpu/drm/sti/sti_drv.c | 1 + > drivers/gpu/drm/ttm/ttm_device.c| 1 + > drivers/gpu/drm/ttm/ttm_resource.c | 3 ++- > drivers/gpu/drm/ttm/ttm_tt.c| 5 +++-- > drivers/gpu/drm/vc4/vc4_drv.h | 1 + > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 ++ > drivers/gpu/drm/xe/xe_debugfs.c | 1 + > drivers/gpu/drm/xe/xe_gt_debugfs.c | 2 ++ > drivers/gpu/drm/xe/xe_uc_debugfs.c | 2 ++ > include/drm/drm_print.h | 2 +- > 31 files changed, 46 insertions(+), 8 deletions(-) > > diff --git a/drivers/accel/ivpu/ivpu_debugfs.c > b/drivers/accel/ivpu/ivpu_debugfs.c > index d09d29775b3f..e07e447d08d1 100644 > --- a/drivers/accel/ivpu/ivpu_debugfs.c > +++ b/drivers/accel/ivpu/ivpu_debugfs.c > @@ -3,6 +3,8 @@ > * Copyright (C) 2020-2023 Intel Corporation > */ > > +#include > + > #include > #include > #include > diff --git a/drivers/gpu/drm/armada/armada_debugfs.c > b/drivers/gpu/drm/armada/armada_debugfs.c > index 29f4b52e3c8d..a763349dd89f 100644 > --- a/drivers/gpu/drm/armada/armada_debugfs.c > +++ b/drivers/gpu/drm/armada/armada_debugfs.c > @@ -5,6 +5,7 @@ > */ > > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c > b/drivers/gpu/drm/bridge/ite-it6505.c > index 27334173e911..3f68c82888c2 100644 > --- a/drivers/gpu/drm/bridge/ite-it6505.c > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > @@ -3,6 +3,7 @@ > * Copyright (c) 2020, The Linux Foundation. All rights reserved. > */ > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 7f41525f7a6e..32506524d9a2 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -4,6 +4,8 @@
Re: [PATCH v5 00/10] Improvments for tc358775 with support for tc358765
On Sun, 25 Feb 2024 08:19:29 +0200, Tony Lindgren wrote: > Here are v5 patches to improve tc358775 driver and add support for > tc358765. > > Regards, > > Tony > > [...] Thanks for the really nice series, sorry about the delay in applying it. Applied, thanks! [01/10] dt-bindings: display: bridge: tc358775: make stby gpio optional https://cgit.freedesktop.org/drm/drm-misc/commit/?id=51debb6d4a21 [02/10] dt-bindings: display: bridge: tc358775: Add data-lanes https://cgit.freedesktop.org/drm/drm-misc/commit/?id=497f0a1bdc06 [03/10] dt-bindings: display: bridge: tc358775: Add support for tc358765 https://cgit.freedesktop.org/drm/drm-misc/commit/?id=efcfac3e8e37 [04/10] drm/bridge: tc358775: fix support for jeida-18 and jeida-24 https://cgit.freedesktop.org/drm/drm-misc/commit/?id=30ea09a182cb [05/10] drm/bridge: tc358775: make standby GPIO optional https://cgit.freedesktop.org/drm/drm-misc/commit/?id=733daaebe250 [06/10] drm/bridge: tc358775: Get bridge data lanes instead of the DSI host lanes https://cgit.freedesktop.org/drm/drm-misc/commit/?id=005102727d9e [07/10] drm/bridge: tc358775: Add burst and low-power modes https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a4ed72e85c46 [08/10] drm/bridge: tc358775: Enable pre_enable_prev_first flag https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e2ee8e82cf42 [09/10] drm/bridge: tc358775: Add support for tc358765 https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ce2196dbba66 [10/10] drm/bridge: tc358775: Configure hs_rate and lp_rate https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ec710af54a1c Rob
Re: [PATCH v6 1/1] drm/bridge: it6505: fix hibernate to resume no display issue
On Tue, Apr 23, 2024 at 10:16 AM kuro wrote:
>
> From: Kuro
>
> ITE added a FIFO reset bit for input video. When system power resume,
> the TTL input of it6505 may get some noise before video signal stable
> and the hardware function reset is required.
> But the input FIFO reset will also trigger error interrupts of output module
> rising.
> Thus, it6505 have to wait a period can clear those expected error interrupts
> caused by manual hardware reset in one interrupt handler calling to avoid
> interrupt looping.
>
> Signed-off-by: Kuro Chung
>
> ---
> drivers/gpu/drm/bridge/ite-it6505.c | 73 +++--
> 1 file changed, 49 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
> b/drivers/gpu/drm/bridge/ite-it6505.c
> index b53da9bb65a16..ae7f4c7ec6dd0 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1317,9 +1317,15 @@ static void it6505_video_reset(struct it6505 *it6505)
> it6505_link_reset_step_train(it6505);
> it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_VID_MUTE, EN_VID_MUTE);
> it6505_set_bits(it6505, REG_INFOFRAME_CTRL, EN_VID_CTRL_PKT, 0x00);
> - it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, VIDEO_RESET);
> +
> + it6505_set_bits(it6505, REG_VID_BUS_CTRL1, TX_FIFO_RESET, 0x02);
> + it6505_set_bits(it6505, REG_VID_BUS_CTRL1, TX_FIFO_RESET, 0x00);
> +
> it6505_set_bits(it6505, REG_501_FIFO_CTRL, RST_501_FIFO,
> RST_501_FIFO);
> it6505_set_bits(it6505, REG_501_FIFO_CTRL, RST_501_FIFO, 0x00);
> +
> + it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, VIDEO_RESET);
> + usleep_range(1000, 2000);
> it6505_set_bits(it6505, REG_RESET_CTRL, VIDEO_RESET, 0x00);
Can any of these magic values be defined as macros?
> }
>
> @@ -2249,12 +2255,11 @@ static void it6505_link_training_work(struct
> work_struct *work)
> if (ret) {
> it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> it6505_link_train_ok(it6505);
> - return;
> } else {
> it6505->auto_train_retry--;
> + it6505_dump(it6505);
> }
>
> - it6505_dump(it6505);
> }
>
> static void it6505_plugged_status_to_codec(struct it6505 *it6505)
> @@ -2475,31 +2480,53 @@ static void it6505_irq_link_train_fail(struct it6505
> *it6505)
> schedule_work(>link_works);
> }
>
> -static void it6505_irq_video_fifo_error(struct it6505 *it6505)
> +static bool it6505_test_bit(unsigned int bit, const unsigned int *addr)
> {
> - struct device *dev = >client->dev;
> -
> - DRM_DEV_DEBUG_DRIVER(dev, "video fifo overflow interrupt");
> - it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> - flush_work(>link_works);
> - it6505_stop_hdcp(it6505);
> - it6505_video_reset(it6505);
> + return 1 & (addr[bit / BITS_PER_BYTE] >> (bit % BITS_PER_BYTE));
> }
>
> -static void it6505_irq_io_latch_fifo_overflow(struct it6505 *it6505)
> +static void it6505_irq_video_handler(struct it6505 *it6505, const int
> *int_status)
> {
> struct device *dev = >client->dev;
> + int reg_0d, reg_int03;
>
> - DRM_DEV_DEBUG_DRIVER(dev, "IO latch fifo overflow interrupt");
> - it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> - flush_work(>link_works);
> - it6505_stop_hdcp(it6505);
> - it6505_video_reset(it6505);
> -}
> + /*
> +* When video SCDT change with video not stable,
> +* Or video FIFO error, need video reset
> +*/
>
> -static bool it6505_test_bit(unsigned int bit, const unsigned int *addr)
> -{
> - return 1 & (addr[bit / BITS_PER_BYTE] >> (bit % BITS_PER_BYTE));
> + if ((!it6505_get_video_status(it6505) &&
> + (it6505_test_bit(INT_SCDT_CHANGE, (unsigned int *)
> int_status))) ||
> + (it6505_test_bit(BIT_INT_IO_FIFO_OVERFLOW, (unsigned int *)
> int_status)) ||
> + (it6505_test_bit(BIT_INT_VID_FIFO_ERROR, (unsigned int *)
> int_status))) {
> +
> + it6505->auto_train_retry = AUTO_TRAIN_RETRY;
> + flush_work(>link_works);
> + it6505_stop_hdcp(it6505);
> + it6505_video_reset(it6505);
> +
> + usleep_range(1, 11000);
> +
> + /*
> +* Clear FIFO error IRQ to prevent fifo error -> reset loop
> +* HW will trigger SCDT change IRQ again when video stable
> +*/
> +
> + reg_int03 = it6505_read(it6505, INT_STATUS_03);
> + reg_0d = it6505_read(it6505, REG_SYSTEM_STS);
> +
> + reg_int03 &= (BIT(INT_VID_FIFO_ERROR) |
> BIT(INT_IO_LATCH_FIFO_OVERFLOW));
> + it6505_write(it6505, INT_STATUS_03, reg_int03);
> +
> + DRM_DEV_DEBUG_DRIVER(dev, "reg08 = 0x%02x", reg_int03);
Is this correct? Doesreg_int03 contain reg08?
> +
Re: [PATCH] drm/stm: dsi: relax mode_valid clock tolerance
On Mon, Apr 22, 2024 at 4:06 PM Sean Nyekjaer wrote:
>
> On Fri, Mar 22, 2024 at 11:47:31AM +0100, Sean Nyekjaer wrote:
> > When using the DSI interface via DSI2LVDS bridge, it seems a bit harsh
> > to reguire the requested and the actual px clock to be within
> > 50Hz. A typical LVDS display requires the px clock to be within +-10%.
> >
> > In case for HDMI .5% tolerance is required.
> >
> > Fixes: e01356d18273 ("drm/stm: dsi: provide the implementation of
> > mode_valid()")
> > Signed-off-by: Sean Nyekjaer
> > ---
> Any feedback on this?
>
> > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > index d5f8c923d7bc..97936b0ef702 100644
> > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > @@ -322,8 +322,6 @@ dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned
> > int lane_mbps,
> > return 0;
> > }
> >
> > -#define CLK_TOLERANCE_HZ 50
> > -
> > static enum drm_mode_status
> > dw_mipi_dsi_stm_mode_valid(void *priv_data,
> > const struct drm_display_mode *mode,
> > @@ -375,9 +373,10 @@ dw_mipi_dsi_stm_mode_valid(void *priv_data,
> > /*
> >* Filter modes according to the clock value, particularly
> > useful for
> >* hdmi modes that require precise pixel clocks.
> > + * Check that px_clock is within .5% tolerance.
> >*/
> > - if (px_clock_hz < target_px_clock_hz - CLK_TOLERANCE_HZ ||
> > - px_clock_hz > target_px_clock_hz + CLK_TOLERANCE_HZ)
> > + if (px_clock_hz < mult_frac(target_px_clock_hz, 995, 1000) ||
> > + px_clock_hz > mult_frac(target_px_clock_hz, 1005, 1000))
> > return MODE_CLOCK_RANGE;
> >
> > /* sync packets are codes as DSI short packets (4 bytes) */
> > --
> > 2.44.0
> >
>
Reviewed-by: Robert Foss
Re: [PATCH v2 1/2] drm/print: drop include debugfs.h and include where needed
Hey Jani, Thanks for doing this cleanup. On Thu, Apr 18, 2024 at 12:13 PM Jani Nikula wrote: > > Surprisingly many places depend on debugfs.h to be included via > drm_print.h. Fix them. > > v2: Also fix ivpu and vmwgfx > > Reviewed-by: Andrzej Hajda > Acked-by: Maxime Ripard > Link: > https://patchwork.freedesktop.org/patch/msgid/20240410141434.157908-1-jani.nik...@intel.com > Signed-off-by: Jani Nikula > > --- > > Cc: Jacek Lawrynowicz > Cc: Stanislaw Gruszka > Cc: Oded Gabbay > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > Cc: Jani Nikula > Cc: Rodrigo Vivi > Cc: Joonas Lahtinen > Cc: Tvrtko Ursulin > Cc: Karol Herbst > Cc: Lyude Paul > Cc: Danilo Krummrich > Cc: Alex Deucher > Cc: "Christian König" > Cc: "Pan, Xinhui" > Cc: Huang Rui > Cc: Zack Rusin > Cc: Broadcom internal kernel review list > > Cc: dri-devel@lists.freedesktop.org > Cc: intel-...@lists.freedesktop.org > Cc: intel...@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > Cc: amd-...@lists.freedesktop.org > --- > drivers/accel/ivpu/ivpu_debugfs.c| 2 ++ > drivers/gpu/drm/bridge/panel.c | 2 ++ > drivers/gpu/drm/drm_print.c | 6 +++--- > drivers/gpu/drm/i915/display/intel_dmc.c | 1 + > drivers/gpu/drm/nouveau/dispnv50/crc.c | 2 ++ > drivers/gpu/drm/radeon/r100.c| 1 + > drivers/gpu/drm/radeon/r300.c| 1 + > drivers/gpu/drm/radeon/r420.c| 1 + > drivers/gpu/drm/radeon/r600.c| 3 ++- > drivers/gpu/drm/radeon/radeon_fence.c| 1 + > drivers/gpu/drm/radeon/radeon_gem.c | 1 + > drivers/gpu/drm/radeon/radeon_ib.c | 2 ++ > drivers/gpu/drm/radeon/radeon_pm.c | 1 + > drivers/gpu/drm/radeon/radeon_ring.c | 2 ++ > drivers/gpu/drm/radeon/radeon_ttm.c | 1 + > drivers/gpu/drm/radeon/rs400.c | 1 + > drivers/gpu/drm/radeon/rv515.c | 1 + > drivers/gpu/drm/ttm/ttm_device.c | 1 + > drivers/gpu/drm/ttm/ttm_resource.c | 3 ++- > drivers/gpu/drm/ttm/ttm_tt.c | 5 +++-- > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 ++ > include/drm/drm_print.h | 2 +- > 22 files changed, 34 insertions(+), 8 deletions(-) > > diff --git a/drivers/accel/ivpu/ivpu_debugfs.c > b/drivers/accel/ivpu/ivpu_debugfs.c > index d09d29775b3f..e07e447d08d1 100644 > --- a/drivers/accel/ivpu/ivpu_debugfs.c > +++ b/drivers/accel/ivpu/ivpu_debugfs.c > @@ -3,6 +3,8 @@ > * Copyright (C) 2020-2023 Intel Corporation > */ > > +#include > + > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 7f41525f7a6e..32506524d9a2 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -4,6 +4,8 @@ > * Copyright (C) 2017 Broadcom > */ > > +#include > + > #include > #include > #include > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c > index 699b7dbffd7b..cf2efb44722c 100644 > --- a/drivers/gpu/drm/drm_print.c > +++ b/drivers/gpu/drm/drm_print.c > @@ -23,13 +23,13 @@ > * Rob Clark > */ > > -#include > - > +#include > +#include > #include > #include > #include > #include > -#include > +#include > > #include > #include > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c > b/drivers/gpu/drm/i915/display/intel_dmc.c > index a34ff3383fd3..370d61c7e342 100644 > --- a/drivers/gpu/drm/i915/display/intel_dmc.c > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c > @@ -22,6 +22,7 @@ > * > */ > > +#include > #include > > #include "i915_drv.h" > diff --git a/drivers/gpu/drm/nouveau/dispnv50/crc.c > b/drivers/gpu/drm/nouveau/dispnv50/crc.c > index 9c942fbd836d..5936b6b3b15d 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/crc.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/crc.c > @@ -1,5 +1,7 @@ > // SPDX-License-Identifier: MIT > +#include > #include > + > #include > #include > #include > diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c > index 86b8b770af19..0b1e19345f43 100644 > --- a/drivers/gpu/drm/radeon/r100.c > +++ b/drivers/gpu/drm/radeon/r100.c > @@ -26,6 +26,7 @@ > * Jerome Glisse > */ > > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/radeon/r30
Re: [PATCH RESEND v3] drm/bridge: anx7625: Update audio status while detecting
On Tue, 16 Apr 2024 07:21:35 +, Hsin-Te Yuan wrote: > Previously, the audio status was not updated during detection, leading > to a persistent audio despite hot plugging events. To resolve this > issue, update the audio status during detection. > > Applied, thanks! [1/1] drm/bridge: anx7625: Update audio status while detecting https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a665b4e60369 Rob
Re: [PATCH v2] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
Hey Aleksandr,
On Fri, Apr 12, 2024 at 10:40 AM Aleksandr Mishin wrote:
>
> In cdns_mhdp_atomic_enable(), the return value of drm_mode_duplicate() is
> assigned to mhdp_state->current_mode, and there is a dereference of it in
> drm_mode_set_name(), which will lead to a NULL pointer dereference on
> failure of drm_mode_duplicate().
>
> Fix this bug by adding a check of mhdp_state->current_mode.
>
> Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP
> bridge")
> Signed-off-by: Aleksandr Mishin
> ---
> v2: Fix a mistake where the mutex remained locked
>
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index e226acc5c15e..5b831d6d7764 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -2059,6 +2059,11 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge
> *bridge,
> mhdp_state = to_cdns_mhdp_bridge_state(new_state);
>
> mhdp_state->current_mode = drm_mode_duplicate(bridge->dev, mode);
> + if (!mhdp_state->current_mode) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
This chunk no longer applies on drm-misc-next.
I think the approach here is still better than what is in
drm-misc-next since it unlocks link_mutex.
Can you rebase + reword the commit message and send that out as v3?
> drm_mode_set_name(mhdp_state->current_mode);
>
> dev_dbg(mhdp->dev, "%s: Enabling mode %s\n", __func__, mode->name);
> --
> 2.30.2
>
Re: [PATCH v3 0/9] drm: Switch from dev_err to dev_err_probe for missing DSI host error path
On Mon, 15 Apr 2024 17:49:28 -0400, Nícolas F. R. A. Prado wrote:
> This series changes every occurrence of the following pattern:
>
> dsi_host = of_find_mipi_dsi_host_by_node(dsi);
> if (!dsi_host) {
> dev_err(dev, "failed to find dsi host\n");
> return -EPROBE_DEFER;
> }
>
> [...]
Applied, thanks!
[1/9] drm/bridge: anx7625: Don't log an error when DSI host can't be found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ef4a9204d594
[2/9] drm/bridge: icn6211: Don't log an error when DSI host can't be found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=275fafe58faa
[3/9] drm/bridge: lt8912b: Don't log an error when DSI host can't be found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=b3b4695ff47c
[4/9] drm/bridge: lt9611: Don't log an error when DSI host can't be found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=cd0a2c6a081f
[5/9] drm/bridge: lt9611uxc: Don't log an error when DSI host can't be found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=6d9e877cde7e
[6/9] drm/bridge: tc358775: Don't log an error when DSI host can't be found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=272377aa0e3d
[7/9] drm/bridge: dpc3433: Don't log an error when DSI host can't be found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=24f4f575214d
[8/9] drm/panel: novatek-nt35950: Don't log an error when DSI host can't be
found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=5ff5505b9a2d
[9/9] drm/panel: truly-nt35597: Don't log an error when DSI host can't be found
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=c1e4d3a6de48
Rob
Re: [PATCH] drm/bridge: imx8mp-hdmi-tx: Convert to platform remove callback returning void
On Mon, 4 Mar 2024 10:10:06 +0100, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is ignored (apart > from emitting a warning) and this typically results in resource leaks. > > To improve here there is a quest to make the remove callback return > void. In the first step of this quest all drivers are converted to > .remove_new(), which already returns void. Eventually after all drivers > are converted, .remove_new() will be renamed to .remove(). > > [...] Applied, thanks! [1/1] drm/bridge: imx8mp-hdmi-tx: Convert to platform remove callback returning void https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2caad4f7b024 Rob
Re: [PATCH] drm/bridge: imx8mp-hdmi-pvi: Convert to platform remove callback returning void
On Mon, 4 Mar 2024 10:05:56 +0100, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is ignored (apart > from emitting a warning) and this typically results in resource leaks. > > To improve here there is a quest to make the remove callback return > void. In the first step of this quest all drivers are converted to > .remove_new(), which already returns void. Eventually after all drivers > are converted, .remove_new() will be renamed to .remove(). > > [...] Applied, thanks! [1/1] drm/bridge: imx8mp-hdmi-pvi: Convert to platform remove callback returning void https://cgit.freedesktop.org/drm/drm-misc/commit/?id=29b39672bc1d Rob
Re: [PATCH] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
On Mon, 8 Apr 2024 15:58:10 +0300, Aleksandr Mishin wrote: > In cdns_mhdp_atomic_enable(), the return value of drm_mode_duplicate() is > assigned to mhdp_state->current_mode, and there is a dereference of it in > drm_mode_set_name(), which will lead to a NULL pointer dereference on > failure of drm_mode_duplicate(). > > Fix this bug add a check of mhdp_state->current_mode. > > [...] Applied, thanks! [1/1] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference https://cgit.freedesktop.org/drm/drm-misc/commit/?id=935a92a1c400 Rob
Re: [PATCH v3 2/2] drm/bridge: adv7511: get edid in hpd_work to update CEC phys address
Sorry to ask for this again, but this series has non-trivial merge conflicts with upstream again. Could you rebase it and send out an updated version?
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Tue, 28 Mar 2023 22:37:51 +0530, Jagan Teki wrote: > For a given bridge pipeline if any bridge sets pre_enable_prev_first > flag then the pre_enable for the previous bridge will be called before > pre_enable of this bridge and opposite is done for post_disable. > > These are the potential bridge flags to alter bridge init order in order > to satisfy the MIPI DSI host and downstream panel or bridge to function. > However the existing pre_enable_prev_first logic with associated bridge > ordering has broken for both pre_enable and post_disable calls. > > [...] Please excuse the delay, patches touching the core bridge code are a little bit tougher to merge due to increased risks of breaking unrelated things. Applied, thanks! [1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e18aeeda0b69 [2/2] drm/bridge: Document bridge init order with pre_enable_prev_first https://cgit.freedesktop.org/drm/drm-misc/commit/?id=113cc3ad8566 Rob
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Thu, Feb 29, 2024 at 12:39 PM Frieder Schrempf
wrote:
>
> Hi,
>
> On 28.03.23 19:07, Jagan Teki wrote:
> > For a given bridge pipeline if any bridge sets pre_enable_prev_first
> > flag then the pre_enable for the previous bridge will be called before
> > pre_enable of this bridge and opposite is done for post_disable.
> >
> > These are the potential bridge flags to alter bridge init order in order
> > to satisfy the MIPI DSI host and downstream panel or bridge to function.
> > However the existing pre_enable_prev_first logic with associated bridge
> > ordering has broken for both pre_enable and post_disable calls.
> >
> > [pre_enable]
> >
> > The altered bridge ordering has failed if two consecutive bridges on a
> > given pipeline enables the pre_enable_prev_first flag.
> >
> > Example:
> > - Panel
> > - Bridge 1
> > - Bridge 2 pre_enable_prev_first
> > - Bridge 3
> > - Bridge 4 pre_enable_prev_first
> > - Bridge 5 pre_enable_prev_first
> > - Bridge 6
> > - Encoder
> >
> > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
> >
> > The logic looks for a bridge which enabled pre_enable_prev_first flag
> > on each iteration and assigned the previou bridge to limit pointer
> > if the bridge doesn't enable pre_enable_prev_first flags.
> >
> > If control found Bridge 2 is pre_enable_prev_first then the iteration
> > looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
> > it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
> > and Bridge 2 and assign iter pointer with limit which is Bridge 4.
> >
> > Here is the actual problem, for the next iteration control look for
> > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
> > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
> > found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
> > to Encoder. From next iteration Encoder skips as it is the last bridge
> > for reverse order pipeline.
> >
> > So, the resulting pre_enable bridge order would be,
> > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
> >
> > This patch fixes this by assigning limit to next pointer instead of
> > previous bridge since the iteration always looks for bridge that does
> > NOT request prev so assigning next makes sure the last bridge on a
> > given iteration what exactly the limit bridge is.
> >
> > So, the resulting pre_enable bridge order with fix would be,
> > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> > Encoder.
> >
> > [post_disable]
> >
> > The altered bridge ordering has failed if two consecutive bridges on a
> > given pipeline enables the pre_enable_prev_first flag.
> >
> > Example:
> > - Panel
> > - Bridge 1
> > - Bridge 2 pre_enable_prev_first
> > - Bridge 3
> > - Bridge 4 pre_enable_prev_first
> > - Bridge 5 pre_enable_prev_first
> > - Bridge 6
> > - Encoder
> >
> > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
> >
> > The logic looks for a bridge which enabled pre_enable_prev_first flags
> > on each iteration and assigned the previou bridge to next and next to
> > limit pointer if the bridge does enable pre_enable_prev_first flag.
> >
> > If control starts from Bridge 6 then it found next Bridge 5 is
> > pre_enable_prev_first and immediately the next assigned to previous
> > Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> > of Bridge 6 even though the next consecutive Bridge 5 is enabled with
> > pre_enable_prev_first. This clearly misses the logic to find the state
> > of next conducive bridge as everytime the next and limit assigns
> > previous bridge if given bridge enabled pre_enable_prev_first.
> >
> > So, the resulting post_disable bridge order would be,
> > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
> > Panel.
> >
> > This patch fixes this by assigning next with previou bridge only if the
> > bridge doesn't enable pre_enable_prev_first flag and the next further
> > assign it to limit. This way we can find the bridge that NOT requested
> > prev to disable last.
> >
> > So, the resulting pre_enable bridge order with fix would be,
> > - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
> > Panel.
> >
> > Validated the bridge init ordering by incorporating dummy bridges in
> > the sun6i-mipi-dsi pipeline
> >
> > Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> > alter bridge init order")
> > Signed-off-by: Jagan Teki
>
> This patch is now almost 1 year old and it has been tested and reviewed
> and there have been multiple pings.
>
> Is there anything missing? Why is it not applied yet?
Sorry about the delay. This has been tested and reviewed properly, so
I will apply it now.
>
> Andrzej, Neil, Robert: As DRM bridge maintainers, can you take care of this?
>
> Thanks
> Frieder
>
Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
On Fri, 23 Feb 2024 16:03:33 +0100, Thierry Reding wrote: > From: Thierry Reding > > Tegra DRM doesn't support display on Tegra234 and later, so make sure > not to remove any existing framebuffers in that case. > > v2: - add comments explaining how this situation can come about > - clear DRIVER_MODESET and DRIVER_ATOMIC feature bits > > [...] Applied, thanks! [1/1] drm/tegra: Remove existing framebuffer only if we support display https://cgit.freedesktop.org/drm/drm-misc/commit/?id=86bf8cfda6d2 Rob
Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
On Mon, Feb 26, 2024 at 12:36 PM Javier Martinez Canillas
wrote:
>
> Thomas Zimmermann writes:
>
> > Hi
> >
> > Am 23.02.24 um 16:03 schrieb Thierry Reding:
> >> From: Thierry Reding
> >>
> >> Tegra DRM doesn't support display on Tegra234 and later, so make sure
> >> not to remove any existing framebuffers in that case.
> >>
> >> v2: - add comments explaining how this situation can come about
> >> - clear DRIVER_MODESET and DRIVER_ATOMIC feature bits
> >>
Fixes: 6848c291a54f ("drm/aperture: Convert drivers to aperture interfaces")
Maybe this is more of a philosophical question, but either the
introduction of this hardware generation is where this regression was
introduced or this possibly this commit.
Either way, I'd like to get this into the drm-misc-fixes branch.
> >> Signed-off-by: Thierry Reding
> >
> > Makes sense as far as the aperture helpers are concerned.
> >
> > Reviewed-by: Thomas Zimmermann
> >
>
> I believe this is drm-misc-fixes material. Since the tegra DRM will remove
> simple{fb,drm} for Tegra234, even when the driver does not support display
> on that platform, leaving the system with no display output at all.
>
> Are you going to push this patch or is going to be done by Thierry?
I'm on it.
>
> > Best regards
> > Thomas
> >
>
> --
> Best regards,
>
> Javier Martinez Canillas
> Core Platforms
> Red Hat
>
Re: [PATCH v2] drm/bridge: adv7511: fix crash on irq during probe
On Mon, 19 Feb 2024 21:21:47 +0100, Alvin Šipraga wrote: > From: Mads Bligaard Nielsen > > Moved IRQ registration down to end of adv7511_probe(). > > If an IRQ already is pending during adv7511_probe > (before adv7511_cec_init) then cec_received_msg_ts > could crash using uninitialized data: > > [...] Applied, thanks! [1/1] drm/bridge: adv7511: fix crash on irq during probe https://cgit.freedesktop.org/drm/drm-misc/commit/?id=aeedaee5ef54 Rob
Re: [PATCH v2 1/1] drm/bridge: Silence error messages upon probe deferral
On Tue, Feb 20, 2024 at 3:43 PM Alexander Stein
wrote:
>
> Hi everyone,
>
> Am Dienstag, 22. August 2023, 13:52:00 CET schrieb Alexander Stein:
> > When -EPROBE_DEFER is returned do not raise an error, but silently return
> > this error instead. Fixes error like this:
> > [drm:drm_bridge_attach] *ERROR* failed to attach bridge
> > /soc@0/bus@3080/mipi-dsi@30a0 to encoder None-34: -517
> > [drm:drm_bridge_attach] *ERROR* failed to attach bridge
> > /soc@0/bus@3080/mipi-dsi@30a0 to encoder None-34: -517
> >
> > Signed-off-by: Alexander Stein
> > ---
> > Changes in v2:
> > * Adjust the indentation
> >
> > Considering Laurent's input IMHO -517 should not occur when using component
> > framework, e.g. drivers/gpu/drm/mcde/mcde_drv.c. This should warrant to only
> > print an error if it is not deferred probe.
> > dev_err_probe() sounds reasonable, but this is something which should be
> > done
> > in drivers. It is also arguable if this message is "hidden" within a debug
> > statement.
>
> Any additional feedback on this? I'd like to get rid of an error message
> which just prints
> a regular probe deferral.
>
> Best regards,
> Alexander
>
> > drivers/gpu/drm/drm_bridge.c | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 39e68e45bb124..132180a03c0eb 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -352,13 +352,15 @@ int drm_bridge_attach(struct drm_encoder *encoder,
> > struct drm_bridge *bridge,
> > bridge->encoder = NULL;
> > list_del(>chain_node);
> >
> > + if (ret != -EPROBE_DEFER) {
> > #ifdef CONFIG_OF
> > - DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
> > - bridge->of_node, encoder->name, ret);
> > + DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
> > + bridge->of_node, encoder->name, ret);
> > #else
> > - DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
> > - encoder->name, ret);
> > + DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
> > + encoder->name, ret);
checkpatch --strict warns for both of the above chunks.
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
With this fixed please add my r-b.
Re: [PATCH 0/2] drm/bridge: sii902x: Crash fixes
On Wed, 03 Jan 2024 15:31:06 +0200, Tomi Valkeinen wrote: > Two small fixes to sii902x for crashes. > > Applied, thanks! [1/2] drm/bridge: sii902x: Fix probing race issue https://cgit.freedesktop.org/drm/drm-misc/commit/?id=dffdfb8f5de1 [2/2] drm/bridge: sii902x: Fix audio codec unregistration https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bc77bde2d3f0 Rob
Re: [PATCH] drm/bridge: tc358767: Limit the Pixel PLL input range
On Thu, 18 Jan 2024 23:02:31 +0100, Marek Vasut wrote: > According to new configuration spreadsheet from Toshiba for TC9595, > the Pixel PLL input clock have to be in range 6..40 MHz. The sheet > calculates those PLL input clock as reference clock divided by both > pre-dividers. Add the extra limit. > > Applied, thanks! [1/1] drm/bridge: tc358767: Limit the Pixel PLL input range https://cgit.freedesktop.org/drm/drm-misc/commit/?id=f86ae204bec4 Rob
Re: [PATCH v3 11/20] drivers/gpu/drm/bridge/synopsys/dw-hdmi.c: remove I2C_CLASS_DDC support
On Sun, Nov 19, 2023 at 11:15 AM Heiner Kallweit wrote: > > After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in > olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. > Class-based device auto-detection is a legacy mechanism and shouldn't > be used in new code. So we can remove this class completely now. > > Preferably this series should be applied via the i2c tree. > > Signed-off-by: Heiner Kallweit > > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 52d91a0df..aca5bb086 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -515,7 +515,6 @@ static struct i2c_adapter *dw_hdmi_i2c_adapter(struct > dw_hdmi *hdmi) > init_completion(>cmp); > > adap = >adap; > - adap->class = I2C_CLASS_DDC; > adap->owner = THIS_MODULE; > adap->dev.parent = hdmi->dev; > adap->algo = _hdmi_algorithm; > Acked-by: Robert Foss
Re: [PATCH v2 2/2] drm/bridge: adv7511: get edid in hpd_work to update CEC phys address
On Fri, Nov 24, 2023 at 4:20 PM Alvin Šipraga wrote:
>
> From: Alvin Šipraga
>
> The adv7511 driver is solely responsible for setting the physical
> address of its CEC adapter. To do this, it must read the EDID. However,
> EDID is only read when either the drm_bridge_funcs :: get_edid or
> drm_connector_helper_funcs :: get_modes ops are called. Without loss of
> generality, it cannot be assumed that these ops are called when a sink
> gets attached. Therefore there exist scenarios in which the CEC physical
> address will be invalid (f.f.f.f), rendering the CEC adapter inoperable.
>
> Address this problem by always fetching the EDID in the HPD work when we
> detect a connection. The CEC physical address is set in the process.
> This is done by moving the EDID DRM helper into an internal helper
> function so that it can be cleanly called from an earlier section of
> the code. The EDID getter has not changed in practice.
>
> Signed-off-by: Alvin Šipraga
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 74
> ++--
> 1 file changed, 48 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 5ffc5904bd59..1f1d3a440895 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -542,6 +542,36 @@ static int adv7511_get_edid_block(void *data, u8 *buf,
> unsigned int block,
> return 0;
> }
>
> +static struct edid *__adv7511_get_edid(struct adv7511 *adv7511,
> + struct drm_connector *connector)
> +{
> + struct edid *edid;
> +
> + /* Reading the EDID only works if the device is powered */
> + if (!adv7511->powered) {
> + unsigned int edid_i2c_addr =
> + (adv7511->i2c_edid->addr << 1);
> +
> + __adv7511_power_on(adv7511);
> +
> + /* Reset the EDID_I2C_ADDR register as it might be cleared */
> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
> +edid_i2c_addr);
> + }
> +
> + edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511);
> +
> + if (!adv7511->powered)
> + __adv7511_power_off(adv7511);
> +
> + adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
> + drm_detect_hdmi_monitor(edid));
> +
> + cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
> +
> + return edid;
> +}
> +
> /*
> -
> * Hotplug handling
> */
> @@ -595,8 +625,24 @@ static void adv7511_hpd_work(struct work_struct *work)
> adv7511->connector.status = status;
>
> if (adv7511->connector.dev) {
> - if (status == connector_status_disconnected)
> + if (status == connector_status_disconnected) {
> cec_phys_addr_invalidate(adv7511->cec_adap);
> + } else {
> + struct edid *edid;
> +
> + /*
> +* Get the updated EDID so that the CEC
> +* subsystem gets informed of any change in
> CEC
> +* address. The helper returns a newly
> allocated
> +* edid structure, so free it to prevent
> +* leakage.
> +*/
> + edid = __adv7511_get_edid(adv7511,
> +
> >connector);
> + if (edid)
> + kfree(edid);
kfree(NULL) is safe, so the if statement can be removed.
With this fixed, feel free to add my r-b to this full series.
Reviewed-by: Robert Foss
Re: [PATCH v4 107/115] drm/bridge: ti-sn65dsi86: Make use of devm_pwmchip_alloc() function
On Wed, Dec 6, 2023 at 12:49 PM Uwe Kleine-König
wrote:
>
> This prepares the pwm driver of the ti-sn65dsi86 to further changes of
> the pwm core outlined in the commit introducing devm_pwmchip_alloc().
> There is no intended semantical change and the driver should behave as
> before.
>
> Signed-off-by: Uwe Kleine-König
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 27 ++-
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index c45c07840f64..33eb2ed0a729 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -197,7 +197,7 @@ struct ti_sn65dsi86 {
> DECLARE_BITMAP(gchip_output, SN_NUM_GPIOS);
> #endif
> #if defined(CONFIG_PWM)
> - struct pwm_chip pchip;
> + struct pwm_chip *pchip;
> boolpwm_enabled;
> atomic_tpwm_pin_busy;
> #endif
> @@ -1372,7 +1372,7 @@ static void ti_sn_pwm_pin_release(struct ti_sn65dsi86
> *pdata)
>
> static struct ti_sn65dsi86 *pwm_chip_to_ti_sn_bridge(struct pwm_chip *chip)
> {
> - return container_of(chip, struct ti_sn65dsi86, pchip);
> + return pwmchip_get_drvdata(chip);
> }
>
> static int ti_sn_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> @@ -1585,22 +1585,31 @@ static const struct pwm_ops ti_sn_pwm_ops = {
> static int ti_sn_pwm_probe(struct auxiliary_device *adev,
>const struct auxiliary_device_id *id)
> {
> + struct pwm_chip *chip;
> struct ti_sn65dsi86 *pdata = dev_get_drvdata(adev->dev.parent);
>
> - pdata->pchip.dev = pdata->dev;
> - pdata->pchip.ops = _sn_pwm_ops;
> - pdata->pchip.npwm = 1;
> - pdata->pchip.of_xlate = of_pwm_single_xlate;
> - pdata->pchip.of_pwm_n_cells = 1;
> + /*
> +* This should better use adev->dev instead of pdata->dev. See
> +*
> https://lore.kernel.org/dri-devel/20231127101547.734061-2-u.kleine-koe...@pengutronix.de
> +*/
> + pdata->pchip = chip = devm_pwmchip_alloc(pdata->dev, 1, 0);
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
>
> - return pwmchip_add(>pchip);
> + pwmchip_set_drvdata(chip, pdata);
> +
> + chip->ops = _sn_pwm_ops;
> + chip->of_xlate = of_pwm_single_xlate;
> + chip->of_pwm_n_cells = 1;
> +
> + return pwmchip_add(chip);
> }
>
> static void ti_sn_pwm_remove(struct auxiliary_device *adev)
> {
> struct ti_sn65dsi86 *pdata = dev_get_drvdata(adev->dev.parent);
>
> - pwmchip_remove(>pchip);
> + pwmchip_remove(pdata->pchip);
>
> if (pdata->pwm_enabled)
> pm_runtime_put_sync(pdata->dev);
> --
> 2.42.0
>
Acked-by: Robert Foss
Re: [PATCH v2 2/2] drm/bridge: ti-sn65dsi86: Never store more than msg->size bytes in AUX xfer
On Thu, Dec 14, 2023 at 9:32 PM Douglas Anderson wrote:
>
> For aux reads, the value `msg->size` indicates the size of the buffer
> provided by `msg->buffer`. We should never in any circumstances write
> more bytes to the buffer since it may overflow the buffer.
>
> In the ti-sn65dsi86 driver there is one code path that reads the
> transfer length from hardware. Even though it's never been seen to be
> a problem, we should make extra sure that the hardware isn't
> increasing the length since doing so would cause us to overrun the
> buffer.
>
> Fixes: 982f589bde7a ("drm/bridge: ti-sn65dsi86: Update reply on aux failures")
> Signed-off-by: Douglas Anderson
> ---
>
> Changes in v2:
> - Updated patch subject to match ps8640 patch.
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 9095d1453710..62cc3893dca5 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -527,6 +527,7 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
> u32 request_val = AUX_CMD_REQ(msg->request);
> u8 *buf = msg->buffer;
> unsigned int len = msg->size;
> + unsigned int short_len;
> unsigned int val;
> int ret;
> u8 addr_len[SN_AUX_LENGTH_REG + 1 - SN_AUX_ADDR_19_16_REG];
> @@ -600,7 +601,8 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
> }
>
> if (val & AUX_IRQ_STATUS_AUX_SHORT) {
> - ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, );
> + ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG,
> _len);
> + len = min(len, short_len);
> if (ret)
> goto exit;
> } else if (val & AUX_IRQ_STATUS_NAT_I2C_FAIL) {
> --
> 2.43.0.472.g3155946c3a-goog
>
>
Reviewed-by: Robert Foss
Re: [PATCH v9 0/2] Add displays support for bsh-smm-s2/pro boards
On Mon, 18 Dec 2023 09:43:36 +0100, Dario Binacchi wrote: > The series adds drivers for the displays used by bsh-smm-s2/pro boards. > This required applying some patches to the samsung-dsim driver and the > drm_bridge.c module. > > Changes in v9: > - Updated commit message > - Drop [3/3] arm64: dts: imx8mn-bsh-smm-s2/pro: add display setup > because applied. > > [...] Applied, thanks! [1/2] drm: bridge: samsung-dsim: enter display mode in the enable() callback https://cgit.freedesktop.org/drm/drm-misc/commit/?id=b2fe2292624a [2/2] drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting https://cgit.freedesktop.org/drm/drm-misc/commit/?id=72a0cfdc3ad7 Rob
Re: [PATCH RESEND] drm/bridge: Fixed a DP link training bug
On Thu, 21 Dec 2023 17:30:57 +0800, xiazhengqiao wrote: > To have better compatibility for DP sink, there is a retry mechanism > for the link training process to switch between different training process. > The original driver code doesn't reset the retry counter when training > state is pass. If the system triggers link training over 3 times, > there will be a chance to causes the driver to use the wrong training > method and return a training fail result. > > [...] Applied, thanks! [1/1] drm/bridge: Fixed a DP link training bug https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ca077ff8cac5 Rob
Re: [PATCH] drm/bridge: samsung-dsim: check the return value only if necessary
On Thu, 7 Dec 2023 17:10:43 +0100, Dario Binacchi wrote: > It was useless to check again the "ret" variable if the function > register_host() was not called. > > Applied, thanks! [1/1] drm/bridge: samsung-dsim: check the return value only if necessary https://cgit.freedesktop.org/drm/drm-misc/commit/?id=045159f5018e Rob
Re: [PATCH v3 0/7] Improve tc358767 regmap usage
On Tue, Dec 12, 2023 at 8:53 AM Alexander Stein
wrote:
>
> Hi,
>
> this series improves the regmap usage by cleaning up current usage as well as
> adding more registers to the list of volatile registers. SYSSTAT is added
> to the list of precious registers as it is cleared upon read.
> This series is based on [1].
>
> Changes in v2:
> * Patch 3: Use more symbolic names instead of register address numbers
> * Patch 3: Add register group description for 0x300 and 0x400 area
>
> Changes in v3:
> * Rebased to next-20231212
> * No changes despite the context due to commit 4dd9368671fb7 ("drm/bridge:
> tc358767: Convert to use maple tree register cache")
>
> Best regards,
> Alexander
>
> [1]
> https://lore.kernel.org/dri-devel/20230817075234.1075736-1-alexander.st...@ew.tq-group.com/T/#t
>
> Alexander Stein (7):
> drm/bridge: tc358767: Use regmap_access_table for writeable registers
> drm/bridge: tc358767: Fix order of register defines
> drm/bridge: tc358767: Add more registers to non-writeable range
> drm/bridge: tc358767: Sort volatile registers according to address
> drm/bridge: tc358767: Add more volatile registers
> drm/bridge: tc358767: Add precious register SYSSTAT
> drm/bridge: tc358767: Add descriptions to register definitions
>
> drivers/gpu/drm/bridge/tc358767.c | 171 +++---
> 1 file changed, 133 insertions(+), 38 deletions(-)
>
> --
> 2.34.1
>
Applied to drm-misc-next
Re: [PATCH v3 7/7] drm/bridge: tc358767: Add descriptions to register definitions
On Tue, Dec 12, 2023 at 8:53 AM Alexander Stein wrote: > > Use the register names from the datasheet. No functional change intended. > > Signed-off-by: Alexander Stein > --- > drivers/gpu/drm/bridge/tc358767.c | 30 +++--- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index 93fa057eca8dc..43e860796e683 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -145,10 +145,10 @@ > #define VFUEN BIT(0) /* Video Frame Timing Upload > */ > > /* System */ > -#define TC_IDREG 0x0500 > -#define SYSSTAT0x0508 > -#define SYSRSTENB 0x050c > +#define TC_IDREG 0x0500 /* Chip ID and Revision ID */ > #define SYSBOOT0x0504 /* System BootStrap Status > Register */ > +#define SYSSTAT0x0508 /* System Status Register */ > +#define SYSRSTENB 0x050c /* System Reset/Enable Register */ > #define ENBI2C (1 << 0) > #define ENBLCD0(1 << 2) > #define ENBBM (1 << 3) > @@ -162,12 +162,12 @@ > #define DP0_VIDSRC_DSI_RX (1 << 0) > #define DP0_VIDSRC_DPI_RX (2 << 0) > #define DP0_VIDSRC_COLOR_BAR (3 << 0) > -#define GPIOM 0x0540 > -#define GPIOC 0x0544 > -#define GPIOO 0x0548 > -#define GPIOI 0x054c > -#define INTCTL_G 0x0560 > -#define INTSTS_G 0x0564 > +#define GPIOM 0x0540 /* GPIO Mode Control Register */ > +#define GPIOC 0x0544 /* GPIO Direction Control Register */ > +#define GPIOO 0x0548 /* GPIO Output Register */ > +#define GPIOI 0x054c /* GPIO Input Register */ > +#define INTCTL_G 0x0560 /* General Interrupts Control > Register */ > +#define INTSTS_G 0x0564 /* General Interrupts Status Register > */ > > #define INT_SYSERR BIT(16) > #define INT_GPIO_H(x) (1 << (x == 0 ? 2 : 10)) > @@ -176,8 +176,8 @@ > #define TEST_INT_C 0x0570 /* Test Interrupts Control Register */ > #define TEST_INT_S 0x0574 /* Test Interrupts Status Register */ > > -#define INT_GP0_LCNT 0x0584 > -#define INT_GP1_LCNT 0x0588 > +#define INT_GP0_LCNT 0x0584 /* Interrupt GPIO0 Low Count Value > Register */ > +#define INT_GP1_LCNT 0x0588 /* Interrupt GPIO1 Low Count Value > Register */ > > /* Control */ > #define DP0CTL 0x0600 > @@ -187,9 +187,9 @@ > #define DP_EN BIT(0) /* Enable DPTX function */ > > /* Clocks */ > -#define DP0_VIDMNGEN0 0x0610 > -#define DP0_VIDMNGEN1 0x0614 > -#define DP0_VMNGENSTATUS 0x0618 > +#define DP0_VIDMNGEN0 0x0610 /* DP0 Video Force M Value Register */ > +#define DP0_VIDMNGEN1 0x0614 /* DP0 Video Force N Value Register */ > +#define DP0_VMNGENSTATUS 0x0618 /* DP0 Video Current M Value Register > */ > #define DP0_AUDMNGEN0 0x0628 /* DP0 Audio Force M Value Register */ > #define DP0_AUDMNGEN1 0x062c /* DP0 Audio Force N Value Register */ > #define DP0_AMNGENSTATUS 0x0630 /* DP0 Audio Current M Value Register > */ > @@ -277,7 +277,7 @@ > #define AUDIFDATA5 0x071c /* DP0 Audio Info Frame Bytes 23 to > 20 */ > #define AUDIFDATA6 0x0720 /* DP0 Audio Info Frame Bytes 27 to > 24 */ > > -#define DP1_SRCCTRL0x07a0 > +#define DP1_SRCCTRL0x07a0 /* DP1 Control Register */ > > /* PHY */ > #define DP_PHY_CTRL0x0800 > -- > 2.34.1 > Reviewed-by: Robert Foss
Re: [PATCH v3 6/7] drm/bridge: tc358767: Add precious register SYSSTAT
On Tue, Dec 12, 2023 at 8:53 AM Alexander Stein
wrote:
>
> This is the single register which clears its value upon read operation.
>
> Signed-off-by: Alexander Stein
> ---
> drivers/gpu/drm/bridge/tc358767.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index 152a6dc916079..93fa057eca8dc 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -2070,6 +2070,15 @@ static const struct regmap_access_table
> tc_volatile_table = {
> .n_yes_ranges = ARRAY_SIZE(tc_volatile_ranges),
> };
>
> +static const struct regmap_range tc_precious_ranges[] = {
> + regmap_reg_range(SYSSTAT, SYSSTAT),
> +};
> +
> +static const struct regmap_access_table tc_precious_table = {
> + .yes_ranges = tc_precious_ranges,
> + .n_yes_ranges = ARRAY_SIZE(tc_precious_ranges),
> +};
> +
> static const struct regmap_range tc_non_writeable_ranges[] = {
> regmap_reg_range(PPI_BUSYPPI, PPI_BUSYPPI),
> regmap_reg_range(DSI_BUSYDSI, DSI_BUSYDSI),
> @@ -2093,6 +2102,7 @@ static const struct regmap_config tc_regmap_config = {
> .cache_type = REGCACHE_MAPLE,
> .readable_reg = tc_readable_reg,
> .volatile_table = _volatile_table,
> + .precious_table = _precious_table,
> .wr_table = _writeable_table,
> .reg_format_endian = REGMAP_ENDIAN_BIG,
> .val_format_endian = REGMAP_ENDIAN_LITTLE,
> --
> 2.34.1
>
Reviewed-by: Robert Foss
Re: [PATCH v3 5/7] drm/bridge: tc358767: Add more volatile registers
On Tue, Dec 12, 2023 at 8:53 AM Alexander Stein
wrote:
>
> These registers might change their value without any host write operation.
>
> Signed-off-by: Alexander Stein
> ---
> drivers/gpu/drm/bridge/tc358767.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index 61d8710f46293..152a6dc916079 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -2049,9 +2049,16 @@ static bool tc_readable_reg(struct device *dev,
> unsigned int reg)
> }
>
> static const struct regmap_range tc_volatile_ranges[] = {
> + regmap_reg_range(PPI_BUSYPPI, PPI_BUSYPPI),
> + regmap_reg_range(DSI_BUSYDSI, DSI_BUSYDSI),
> + regmap_reg_range(DSI_LANESTATUS0, DSI_INTSTATUS),
> + regmap_reg_range(DSIERRCNT, DSIERRCNT),
> regmap_reg_range(VFUEN0, VFUEN0),
> + regmap_reg_range(SYSSTAT, SYSSTAT),
> regmap_reg_range(GPIOI, GPIOI),
> regmap_reg_range(INTSTS_G, INTSTS_G),
> + regmap_reg_range(DP0_VMNGENSTATUS, DP0_VMNGENSTATUS),
> + regmap_reg_range(DP0_AMNGENSTATUS, DP0_AMNGENSTATUS),
> regmap_reg_range(DP0_AUXWDATA(0), DP0_AUXSTATUS),
> regmap_reg_range(DP0_LTSTAT, DP0_SNKLTCHGREQ),
> regmap_reg_range(DP_PHY_CTRL, DP_PHY_CTRL),
> --
> 2.34.1
>
Reviewed-by: Robert Foss
Re: [PATCH v3 4/7] drm/bridge: tc358767: Sort volatile registers according to address
On Tue, Dec 12, 2023 at 8:53 AM Alexander Stein
wrote:
>
> Sort the list by the starting address to ease adding new entries.
> No functional change intended.
>
> Signed-off-by: Alexander Stein
> ---
> drivers/gpu/drm/bridge/tc358767.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index 5c0292b71e9fa..61d8710f46293 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -2049,13 +2049,13 @@ static bool tc_readable_reg(struct device *dev,
> unsigned int reg)
> }
>
> static const struct regmap_range tc_volatile_ranges[] = {
> + regmap_reg_range(VFUEN0, VFUEN0),
> + regmap_reg_range(GPIOI, GPIOI),
> + regmap_reg_range(INTSTS_G, INTSTS_G),
> regmap_reg_range(DP0_AUXWDATA(0), DP0_AUXSTATUS),
> regmap_reg_range(DP0_LTSTAT, DP0_SNKLTCHGREQ),
> regmap_reg_range(DP_PHY_CTRL, DP_PHY_CTRL),
> regmap_reg_range(DP0_PLLCTRL, PXL_PLLCTRL),
> - regmap_reg_range(VFUEN0, VFUEN0),
> - regmap_reg_range(INTSTS_G, INTSTS_G),
> - regmap_reg_range(GPIOI, GPIOI),
> };
>
> static const struct regmap_access_table tc_volatile_table = {
> --
> 2.34.1
>
Reviewed-by: Robert Foss
Re: [PATCH v3 3/7] drm/bridge: tc358767: Add more registers to non-writeable range
t; */
> +#define AUDCFG10x0704 /* DP0 Audio Config1 Register
> */
> +#define AUDIFDATA0 0x0708 /* DP0 Audio Info Frame Bytes 3 to 0
> */
> +#define AUDIFDATA1 0x070c /* DP0 Audio Info Frame Bytes 7 to 4
> */
> +#define AUDIFDATA2 0x0710 /* DP0 Audio Info Frame Bytes 11 to 8
> */
> +#define AUDIFDATA3 0x0714 /* DP0 Audio Info Frame Bytes 15 to
> 12 */
> +#define AUDIFDATA4 0x0718 /* DP0 Audio Info Frame Bytes 19 to
> 16 */
> +#define AUDIFDATA5 0x071c /* DP0 Audio Info Frame Bytes 23 to
> 20 */
> +#define AUDIFDATA6 0x0720 /* DP0 Audio Info Frame Bytes 27 to
> 24 */
>
> #define DP1_SRCCTRL0x07a0
>
> @@ -238,6 +290,25 @@
> #define PHY_2LANE BIT(2) /* PHY Enable 2 lanes */
> #define PHY_A0_EN BIT(1) /* PHY Aux Channel0 Enable */
> #define PHY_M0_EN BIT(0) /* PHY Main Channel0 Enable
> */
> +#define DP_PHY_CFG_WR 0x0810 /* DP PHY Configuration Test Write
> Register */
> +#define DP_PHY_CFG_RD 0x0814 /* DP PHY Configuration Test Read
> Register */
> +#define DP0_AUX_PHY_CTRL 0x0820 /* DP0 AUX PHY Control Register */
> +#define DP0_MAIN_PHY_DBG 0x0840 /* DP0 Main PHY Test Debug Register */
> +
> +/* I2S */
> +#define I2SCFG 0x0880 /* I2S Audio Config 0 Register */
> +#define I2SCH0STAT00x0888 /* I2S Audio Channel 0 Status Bytes 3
> to 0 */
> +#define I2SCH0STAT10x088c /* I2S Audio Channel 0 Status Bytes 7
> to 4 */
> +#define I2SCH0STAT20x0890 /* I2S Audio Channel 0 Status Bytes
> 11 to 8 */
> +#define I2SCH0STAT30x0894 /* I2S Audio Channel 0 Status Bytes
> 15 to 12 */
> +#define I2SCH0STAT40x0898 /* I2S Audio Channel 0 Status Bytes
> 19 to 16 */
> +#define I2SCH0STAT50x089c /* I2S Audio Channel 0 Status Bytes
> 23 to 20 */
> +#define I2SCH1STAT00x08a0 /* I2S Audio Channel 1 Status Bytes 3
> to 0 */
> +#define I2SCH1STAT10x08a4 /* I2S Audio Channel 1 Status Bytes 7
> to 4 */
> +#define I2SCH1STAT20x08a8 /* I2S Audio Channel 1 Status Bytes
> 11 to 8 */
> +#define I2SCH1STAT30x08ac /* I2S Audio Channel 1 Status Bytes
> 15 to 12 */
> +#define I2SCH1STAT40x08b0 /* I2S Audio Channel 1 Status Bytes
> 19 to 16 */
> +#define I2SCH1STAT50x08b4 /* I2S Audio Channel 1 Status Bytes
> 23 to 20 */
>
> /* PLL */
> #define DP0_PLLCTRL0x0900
> @@ -1833,16 +1904,16 @@ static bool tc_readable_reg(struct device *dev,
> unsigned int reg)
> case 0x1f4:
> /* DSI Protocol Layer */
> case DSI_STARTDSI:
> - case 0x208:
> + case DSI_BUSYDSI:
> case DSI_LANEENABLE:
> - case 0x214:
> - case 0x218:
> - case 0x220:
> + case DSI_LANESTATUS0:
> + case DSI_LANESTATUS1:
> + case DSI_INTSTATUS:
> case 0x224:
> case 0x228:
> case 0x230:
> /* DSI General */
> - case 0x300:
> + case DSIERRCNT:
> /* DSI Application Layer */
> case 0x400:
> case 0x404:
> @@ -1993,7 +2064,11 @@ static const struct regmap_access_table
> tc_volatile_table = {
> };
>
> static const struct regmap_range tc_non_writeable_ranges[] = {
> - regmap_reg_range(TC_IDREG, TC_IDREG),
> + regmap_reg_range(PPI_BUSYPPI, PPI_BUSYPPI),
> + regmap_reg_range(DSI_BUSYDSI, DSI_BUSYDSI),
> + regmap_reg_range(DSI_LANESTATUS0, DSI_INTSTATUS),
> + regmap_reg_range(TC_IDREG, SYSSTAT),
> + regmap_reg_range(GPIOI, GPIOI),
> regmap_reg_range(DP0_LTSTAT, DP0_SNKLTCHGREQ),
> };
>
> --
> 2.34.1
>
Reviewed-by: Robert Foss
Re: [PATCH v3 2/7] drm/bridge: tc358767: Fix order of register defines
On Tue, Dec 12, 2023 at 8:53 AM Alexander Stein wrote: > > 0x0510 is bigger than 0x50c, order them accordingly. > No functional change intended. > > Signed-off-by: Alexander Stein > --- > drivers/gpu/drm/bridge/tc358767.c | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index 258eacb4125a4..ab0710a35d3d1 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -116,13 +116,6 @@ > /* System */ > #define TC_IDREG 0x0500 > #define SYSSTAT0x0508 > -#define SYSCTRL0x0510 > -#define DP0_AUDSRC_NO_INPUT(0 << 3) > -#define DP0_AUDSRC_I2S_RX (1 << 3) > -#define DP0_VIDSRC_NO_INPUT(0 << 0) > -#define DP0_VIDSRC_DSI_RX (1 << 0) > -#define DP0_VIDSRC_DPI_RX (2 << 0) > -#define DP0_VIDSRC_COLOR_BAR (3 << 0) > #define SYSRSTENB 0x050c > #define ENBI2C (1 << 0) > #define ENBLCD0(1 << 2) > @@ -130,6 +123,13 @@ > #define ENBDSIRX (1 << 4) > #define ENBREG (1 << 5) > #define ENBHDCP(1 << 8) > +#define SYSCTRL0x0510 /* System Control Register */ > +#define DP0_AUDSRC_NO_INPUT(0 << 3) > +#define DP0_AUDSRC_I2S_RX (1 << 3) > +#define DP0_VIDSRC_NO_INPUT(0 << 0) > +#define DP0_VIDSRC_DSI_RX (1 << 0) > +#define DP0_VIDSRC_DPI_RX (2 << 0) > +#define DP0_VIDSRC_COLOR_BAR (3 << 0) > #define GPIOM 0x0540 > #define GPIOC 0x0544 > #define GPIOO 0x0548 > -- > 2.34.1 > Reviewed-by: Robert Foss
Re: [PATCH] drm/bridge: nxp-ptn3460: simplify some error checking
On Wed, 6 Dec 2023 18:05:15 +0300, Dan Carpenter wrote: > The i2c_master_send/recv() functions return negative error codes or > they return "len" on success. So the error handling here can be written > as just normal checks for "if (ret < 0) return ret;". No need to > complicate things. > > Btw, in this code the "len" parameter can never be zero, but even if > it were, then I feel like this would still be the best way to write it. > > [...] Added suggested by tag, to reflect Neils feedback. Applied, thanks! [1/1] drm/bridge: nxp-ptn3460: simplify some error checking https://cgit.freedesktop.org/drm/drm-misc/commit/?id=28d3d0696688 Rob
Re: [PATCH] drm/bridge: nxp-ptn3460: fix i2c_master_send() error checking
On Tue, Dec 5, 2023, 15:01 Dan Carpenter wrote: > On Tue, Dec 05, 2023 at 02:48:26PM +0100, Robert Foss wrote: > > On Mon, 4 Dec 2023 15:29:00 +0300, Dan Carpenter wrote: > > > The i2c_master_send/recv() functions return negative error codes or the > > > number of bytes that were able to be sent/received. This code has > > > two problems. 1) Instead of checking if all the bytes were sent or > > > received, it checks that at least one byte was sent or received. > > > 2) If there was a partial send/receive then we should return a negative > > > error code but this code returns success. > > > > > > [...] > > > > Applied, thanks! > > > > [1/1] drm/bridge: nxp-ptn3460: fix i2c_master_send() error checking > > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=914437992876 > > > > Wait. That was unexpected. Neil's review comments were correct. I was > planning to send a v2 patch which was just a cleanup. > Sorry Dan, I was too quick on the draw. Can you send a fixup and I'll apply it too? > regards, > dan carpenter > >
Re: [PATCH] drm/bridge: nxp-ptn3460: fix i2c_master_send() error checking
On Mon, 4 Dec 2023 15:29:00 +0300, Dan Carpenter wrote: > The i2c_master_send/recv() functions return negative error codes or the > number of bytes that were able to be sent/received. This code has > two problems. 1) Instead of checking if all the bytes were sent or > received, it checks that at least one byte was sent or received. > 2) If there was a partial send/receive then we should return a negative > error code but this code returns success. > > [...] Applied, thanks! [1/1] drm/bridge: nxp-ptn3460: fix i2c_master_send() error checking https://cgit.freedesktop.org/drm/drm-misc/commit/?id=914437992876 Rob
Re: [PATCH v1 1/3] drm/bridge: lt8912b: Add suspend/resume support
On Wed, Nov 15, 2023 at 1:14 PM Francesco Dolcini wrote:
>
> From: Stefan Eichenberger
>
> Add support for suspend and resume. The lt8912b will power off when
> going into suspend and power on when resuming.
>
> Signed-off-by: Stefan Eichenberger
> Signed-off-by: Francesco Dolcini
> ---
> drivers/gpu/drm/bridge/lontium-lt8912b.c | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 03532efb893b..097ab04234b7 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -634,6 +634,33 @@ static const struct drm_bridge_funcs lt8912_bridge_funcs
> = {
> .get_edid = lt8912_bridge_get_edid,
> };
>
> +static int lt8912_bridge_resume(struct device *dev)
> +{
> + struct lt8912 *lt = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = lt8912_hard_power_on(lt);
> + if (ret)
> + return ret;
> +
> + ret = lt8912_soft_power_on(lt);
> + if (ret)
> + return ret;
> +
> + return lt8912_video_on(lt);
> +}
> +
> +static int lt8912_bridge_suspend(struct device *dev)
> +{
> + struct lt8912 *lt = dev_get_drvdata(dev);
> +
> + lt8912_hard_power_off(lt);
> +
> + return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(lt8912_bridge_pm_ops, lt8912_bridge_suspend,
> lt8912_bridge_resume);
> +
> static int lt8912_parse_dt(struct lt8912 *lt)
> {
> struct gpio_desc *gp_reset;
> @@ -770,6 +797,7 @@ static struct i2c_driver lt8912_i2c_driver = {
> .driver = {
> .name = "lt8912",
> .of_match_table = lt8912_dt_match,
> + .pm = pm_sleep_ptr(_bridge_pm_ops),
> },
> .probe = lt8912_probe,
> .remove = lt8912_remove,
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH v1 3/3] drm/bridge: lt8912b: Add power supplies
On Wed, Nov 15, 2023 at 1:14 PM Francesco Dolcini wrote:
>
> From: Stefan Eichenberger
>
> Add supplies to the driver that can be used to turn the Lontium lt8912b
> on and off. It can have up to 7 independent supplies, we add them all
> and enable/disable them with bulk_enable/disable.
>
> Signed-off-by: Stefan Eichenberger
> Signed-off-by: Francesco Dolcini
> ---
> drivers/gpu/drm/bridge/lontium-lt8912b.c | 30
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 097ab04234b7..273157428c82 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -43,6 +43,8 @@ struct lt8912 {
>
> struct videomode mode;
>
> + struct regulator_bulk_data supplies[7];
> +
> u8 data_lanes;
> bool is_power_on;
> };
> @@ -257,6 +259,12 @@ static int lt8912_free_i2c(struct lt8912 *lt)
>
> static int lt8912_hard_power_on(struct lt8912 *lt)
> {
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(lt->supplies), lt->supplies);
> + if (ret)
> + return ret;
> +
> gpiod_set_value_cansleep(lt->gp_reset, 0);
> msleep(20);
>
> @@ -267,6 +275,9 @@ static void lt8912_hard_power_off(struct lt8912 *lt)
> {
> gpiod_set_value_cansleep(lt->gp_reset, 1);
> msleep(20);
> +
> + regulator_bulk_disable(ARRAY_SIZE(lt->supplies), lt->supplies);
> +
> lt->is_power_on = false;
> }
>
> @@ -661,6 +672,21 @@ static int lt8912_bridge_suspend(struct device *dev)
>
> static DEFINE_SIMPLE_DEV_PM_OPS(lt8912_bridge_pm_ops, lt8912_bridge_suspend,
> lt8912_bridge_resume);
>
> +static int lt8912_get_regulators(struct lt8912 *lt)
> +{
> + unsigned int i;
> + const char * const supply_names[] = {
> + "vdd", "vccmipirx", "vccsysclk", "vcclvdstx",
> + "vcchdmitx", "vcclvdspll", "vcchdmipll"
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(lt->supplies); i++)
> + lt->supplies[i].supply = supply_names[i];
> +
> + return devm_regulator_bulk_get(lt->dev, ARRAY_SIZE(lt->supplies),
> + lt->supplies);
> +}
> +
> static int lt8912_parse_dt(struct lt8912 *lt)
> {
> struct gpio_desc *gp_reset;
> @@ -712,6 +738,10 @@ static int lt8912_parse_dt(struct lt8912 *lt)
> goto err_free_host_node;
> }
>
> + ret = lt8912_get_regulators(lt);
> + if (ret)
> + goto err_free_host_node;
> +
> of_node_put(port_node);
> return 0;
>
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH 1/2] Revert "drm/bridge: Add 200ms delay to wait FW HPD status stable"
On Mon, 20 Nov 2023 17:10:36 +0800, Xin Ji wrote: > This reverts commit 330140d7319fcc4ec68bd924ea212e476bf12275 > > 200ms delay will cause panel display image later than backlight > turn on, revert this patch. > > Applied, thanks! [1/2] Revert "drm/bridge: Add 200ms delay to wait FW HPD status stable" https://cgit.freedesktop.org/drm/drm-misc/commit/?id=af3145aa142c [2/2] drm/bridge: anx7625: Fix Set HPD irq detect window to 2ms https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e3af7053de3f Rob
Re: [PATCH v2] drm/bridge: imx93-mipi-dsi: Fix a couple of building warnings
On Thu, 23 Nov 2023 13:18:07 +0800, Liu Ying wrote: > Fix a couple of building warnings on used uninitialized 'best_m' and > 'best_n' local variables by initializing 'best_m' to zero and 'best_n' > to UINT_MAX. This makes compiler happy only. No functional change. > > Applied, thanks! [1/1] drm/bridge: imx93-mipi-dsi: Fix a couple of building warnings https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9f83f37ca76d Rob
Re: [PATCH] drm/bridge: Fix typo in post_disable() description
On Fri, 24 Nov 2023 10:42:30 +0100, Dario Binacchi wrote: > s/singals/signals/ > > Applied, thanks! [1/1] drm/bridge: Fix typo in post_disable() description https://cgit.freedesktop.org/drm/drm-misc/commit/?id=288b039db225 Rob
Re: [PATCH 2/2] fbdev/simplefb: Add support for generic power-domains
On Wed, Oct 18, 2023 at 12:35 PM Robert Foss wrote:
>
> On Wed, Oct 11, 2023 at 4:38 PM Thierry Reding
> wrote:
> >
> > From: Thierry Reding
> >
> > The simple-framebuffer device tree bindings document the power-domains
> > property, so make sure that simplefb supports it. This ensures that the
> > power domains remain enabled as long as simplefb is active.
> >
> > Signed-off-by: Thierry Reding
> > ---
> > drivers/video/fbdev/simplefb.c | 93 +-
> > 1 file changed, 91 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> > index 18025f34fde7..e69fb0ad2d54 100644
> > --- a/drivers/video/fbdev/simplefb.c
> > +++ b/drivers/video/fbdev/simplefb.c
> > @@ -25,6 +25,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > static const struct fb_fix_screeninfo simplefb_fix = {
> > @@ -78,6 +79,11 @@ struct simplefb_par {
> > unsigned int clk_count;
> > struct clk **clks;
> > #endif
> > +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> > + unsigned int num_genpds;
> > + struct device **genpds;
> > + struct device_link **genpd_links;
> > +#endif
> > #if defined CONFIG_OF && defined CONFIG_REGULATOR
> > bool regulators_enabled;
> > u32 regulator_count;
> > @@ -432,6 +438,83 @@ static void simplefb_regulators_enable(struct
> > simplefb_par *par,
> > static void simplefb_regulators_destroy(struct simplefb_par *par) { }
> > #endif
> >
> > +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> > +static void simplefb_detach_genpds(void *res)
> > +{
> > + struct simplefb_par *par = res;
> > + unsigned int i = par->num_genpds;
> > +
> > + if (par->num_genpds <= 1)
> > + return;
> > +
> > + while (i--) {
> > + if (par->genpd_links[i])
> > + device_link_del(par->genpd_links[i]);
> > +
> > + if (!IS_ERR_OR_NULL(par->genpds[i]))
> > + dev_pm_domain_detach(par->genpds[i], true);
> > + }
> > +}
> > +
> > +static int simplefb_attach_genpd(struct simplefb_par *par,
> > +struct platform_device *pdev)
> > +{
> > + struct device *dev = >dev;
> > + unsigned int i;
> > + int err;
> > +
> > + par->num_genpds = of_count_phandle_with_args(dev->of_node,
> > +"power-domains",
> > +"#power-domain-cells");
> > + /*
> > +* Single power-domain devices are handled by the driver core, so
> > +* nothing to do here.
> > +*/
> > + if (par->num_genpds <= 1)
> > + return 0;
> > +
> > + par->genpds = devm_kcalloc(dev, par->num_genpds,
> > sizeof(*par->genpds),
> > + GFP_KERNEL);
> > + if (!par->genpds)
> > + return -ENOMEM;
> > +
> > + par->genpd_links = devm_kcalloc(dev, par->num_genpds,
> > + sizeof(*par->genpd_links),
> > + GFP_KERNEL);
> > + if (!par->genpd_links)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < par->num_genpds; i++) {
> > + par->genpds[i] = dev_pm_domain_attach_by_id(dev, i);
> > + if (IS_ERR(par->genpds[i])) {
> > + err = PTR_ERR(par->genpds[i]);
> > + if (err == -EPROBE_DEFER) {
> > + simplefb_detach_genpds(par);
> > + return err;
> > + }
> > +
> > + dev_warn(dev, "failed to attach domain %u: %d\n",
> > i, err);
> > + continue;
> > + }
> > +
> > + par->genpd_links[i] = device_link_add(dev, par->genpds[i],
> > + DL_FLAG_STATELESS |
> > + DL_FLAG_PM_RUNTIME |
> > + DL_FLAG_RPM_ACTIVE);
> > +
Re: [PATCH 2/2] fbdev/simplefb: Add support for generic power-domains
On Wed, Oct 11, 2023 at 4:38 PM Thierry Reding wrote:
>
> From: Thierry Reding
>
> The simple-framebuffer device tree bindings document the power-domains
> property, so make sure that simplefb supports it. This ensures that the
> power domains remain enabled as long as simplefb is active.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/video/fbdev/simplefb.c | 93 +-
> 1 file changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 18025f34fde7..e69fb0ad2d54 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> static const struct fb_fix_screeninfo simplefb_fix = {
> @@ -78,6 +79,11 @@ struct simplefb_par {
> unsigned int clk_count;
> struct clk **clks;
> #endif
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> + unsigned int num_genpds;
> + struct device **genpds;
> + struct device_link **genpd_links;
> +#endif
> #if defined CONFIG_OF && defined CONFIG_REGULATOR
> bool regulators_enabled;
> u32 regulator_count;
> @@ -432,6 +438,83 @@ static void simplefb_regulators_enable(struct
> simplefb_par *par,
> static void simplefb_regulators_destroy(struct simplefb_par *par) { }
> #endif
>
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> +static void simplefb_detach_genpds(void *res)
> +{
> + struct simplefb_par *par = res;
> + unsigned int i = par->num_genpds;
> +
> + if (par->num_genpds <= 1)
> + return;
> +
> + while (i--) {
> + if (par->genpd_links[i])
> + device_link_del(par->genpd_links[i]);
> +
> + if (!IS_ERR_OR_NULL(par->genpds[i]))
> + dev_pm_domain_detach(par->genpds[i], true);
> + }
> +}
> +
> +static int simplefb_attach_genpd(struct simplefb_par *par,
> +struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + unsigned int i;
> + int err;
> +
> + par->num_genpds = of_count_phandle_with_args(dev->of_node,
> +"power-domains",
> +"#power-domain-cells");
> + /*
> +* Single power-domain devices are handled by the driver core, so
> +* nothing to do here.
> +*/
> + if (par->num_genpds <= 1)
> + return 0;
> +
> + par->genpds = devm_kcalloc(dev, par->num_genpds, sizeof(*par->genpds),
> + GFP_KERNEL);
> + if (!par->genpds)
> + return -ENOMEM;
> +
> + par->genpd_links = devm_kcalloc(dev, par->num_genpds,
> + sizeof(*par->genpd_links),
> + GFP_KERNEL);
> + if (!par->genpd_links)
> + return -ENOMEM;
> +
> + for (i = 0; i < par->num_genpds; i++) {
> + par->genpds[i] = dev_pm_domain_attach_by_id(dev, i);
> + if (IS_ERR(par->genpds[i])) {
> + err = PTR_ERR(par->genpds[i]);
> + if (err == -EPROBE_DEFER) {
> + simplefb_detach_genpds(par);
> + return err;
> + }
> +
> + dev_warn(dev, "failed to attach domain %u: %d\n", i,
> err);
> + continue;
> + }
> +
> + par->genpd_links[i] = device_link_add(dev, par->genpds[i],
> + DL_FLAG_STATELESS |
> + DL_FLAG_PM_RUNTIME |
> + DL_FLAG_RPM_ACTIVE);
> + if (!par->genpd_links[i])
> + dev_warn(dev, "failed to link power-domain %u\n", i);
> + }
> +
> + return devm_add_action_or_reset(dev, simplefb_detach_genpds, par);
> +}
> +#else
> +static int simplefb_attach_genpd(struct simplefb_par *par,
> +struct platform_device *pdev)
> +{
> + return 0;
> +}
> +#endif
> +
> static int simplefb_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -518,6 +601,10 @@ static int simplefb_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_clocks;
>
> + ret = simplefb_attach_genpd(par, pdev);
> + if (ret < 0)
> + goto error_regulators;
> +
> simplefb_clocks_enable(par, pdev);
> simplefb_regulators_enable(par, pdev);
>
> @@ -534,18 +621,20 @@ static int simplefb_probe(struct platform_device *pdev)
> ret = devm_aperture_acquire_for_platform_device(pdev, par->base,
> par->size);
> if (ret) {
> dev_err(>dev, "Unable
Re: [PATCH] drm/bridge: lt9611uxc: fix the race in the error path
On Thu, 12 Oct 2023 01:00:02 +0300, Dmitry Baryshkov wrote: > If DSI host attachment fails, the LT9611UXC driver will remove the > bridge without ensuring that there is no outstanding HPD work being > done. In rare cases this can result in the warnings regarding the mutex > being incorrect. Fix this by forcebly freing IRQ and flushing the work. > > DEBUG_LOCKS_WARN_ON(lock->magic != lock) > WARNING: CPU: 0 PID: 10 at kernel/locking/mutex.c:582 __mutex_lock+0x468/0x77c > Modules linked in: > CPU: 0 PID: 10 Comm: kworker/0:1 Tainted: G U > 6.6.0-rc5-next-20231011-gd81f81c2b682-dirty #1206 > Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT) > Workqueue: events lt9611uxc_hpd_work > pstate: 6045 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : __mutex_lock+0x468/0x77c > lr : __mutex_lock+0x468/0x77c > sp : 8000800a3c70 > x29: 8000800a3c70 x28: x27: d595fe333000 > x26: 7c2f0002c005 x25: d595ff1b3000 x24: d595fccda5a0 > x23: x22: 0002 x21: 7c2f056d91c8 > x20: x19: 7c2f056d91c8 x18: fffe8db0 > x17: 00040044 x16: 005000f2b5503510 x15: > x14: 0006efb8 x13: x12: 0037 > x11: 0001 x10: 1470 x9 : 8000800a3ae0 > x8 : 7c2f0027f8d0 x7 : 7c2f0027e400 x6 : d595fc702b54 > x5 : x4 : 8000800a x3 : > x2 : x1 : x0 : 7c2f0027e400 > Call trace: > __mutex_lock+0x468/0x77c > mutex_lock_nested+0x24/0x30 > drm_bridge_hpd_notify+0x2c/0x5c > lt9611uxc_hpd_work+0x6c/0x80 > process_one_work+0x1ec/0x51c > worker_thread+0x1ec/0x3e4 > kthread+0x120/0x124 > ret_from_fork+0x10/0x20 > irq event stamp: 15799 > hardirqs last enabled at (15799): [] > finish_task_switch.isra.0+0xa8/0x278 > hardirqs last disabled at (15798): [] __schedule+0x7b8/0xbd8 > softirqs last enabled at (15794): [] > __do_softirq+0x498/0x4e0 > softirqs last disabled at (15771): [] > do_softirq+0x10/0x1c > > [...] Applied, thanks! [1/1] drm/bridge: lt9611uxc: fix the race in the error path https://cgit.freedesktop.org/drm/drm-misc/commit/?id=15fe53be46ea Rob
Re: [PATCH v3 RESEND 0/9] drm/bridge: imx: Add i.MX93 MIPI DSI support
On Mon, 21 Aug 2023 11:39:59 +0800, Liu Ying wrote: > This series aims to add MIPI DSI support for Freescale i.MX93 SoC. > > There is a Synopsys DesignWare MIPI DSI host controller and a Synopsys > Designware MIPI DPHY embedded in i.MX93. Some configurations and > extensions to them are controlled by i.MX93 media blk-ctrl. > > Add a DRM bridge for i.MX93 MIPI DSI by using existing DW MIPI DSI > bridge helpers and implementing i.MX93 MIPI DSI specific extensions. > > [...] Applied, thanks! [1/9] drm/bridge: synopsys: dw-mipi-dsi: Add dw_mipi_dsi_get_bridge() helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ec20c510ee2d [2/9] drm/bridge: synopsys: dw-mipi-dsi: Add input bus format negotiation support https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0de852d4c23a [3/9] drm/bridge: synopsys: dw-mipi-dsi: Force input bus flags https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d5116fb29dc0 [4/9] drm/bridge: synopsys: dw-mipi-dsi: Add mode fixup support https://cgit.freedesktop.org/drm/drm-misc/commit/?id=5a67ec8c64ec [5/9] drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate lbcc https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ac87d23694f4 [6/9] drm/bridge: synopsys: dw-mipi-dsi: Set minimum lane byte clock cycles for HSA and HBP https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d22e9a6df2db [7/9] drm/bridge: synopsys: dw-mipi-dsi: Disable HSTX and LPRX timeout check https://cgit.freedesktop.org/drm/drm-misc/commit/?id=743bf594a3b1 [8/9] dt-bindings: display: bridge: Document Freescale i.MX93 MIPI DSI https://cgit.freedesktop.org/drm/drm-misc/commit/?id=db95a55ccec7 [9/9] drm/bridge: imx: Add i.MX93 MIPI DSI support https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ce62f8ea7e3f Rob
Re: [PATCH v3 RESEND 9/9] drm/bridge: imx: Add i.MX93 MIPI DSI support
gt; +
> + return 0;
> +}
> +
> +static const struct dw_mipi_dsi_host_ops imx93_dsi_host_ops = {
> + .attach = imx93_dsi_host_attach,
> +};
> +
> +static int imx93_dsi_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct device_node *np = dev->of_node;
> + struct imx93_dsi *dsi;
> + int ret;
> +
> + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> + if (!dsi)
> + return -ENOMEM;
> +
> + dsi->regmap = syscon_regmap_lookup_by_phandle(np,
> "fsl,media-blk-ctrl");
> + if (IS_ERR(dsi->regmap)) {
> + ret = PTR_ERR(dsi->regmap);
> + dev_err(dev, "failed to get block ctrl regmap: %d\n", ret);
> + return ret;
> + }
> +
> + dsi->clk_pixel = devm_clk_get(dev, "pix");
> + if (IS_ERR(dsi->clk_pixel))
> + return dev_err_probe(dev, PTR_ERR(dsi->clk_pixel),
> +"failed to get pixel clock\n");
> +
> + dsi->clk_cfg = devm_clk_get(dev, "phy_cfg");
> + if (IS_ERR(dsi->clk_cfg))
> + return dev_err_probe(dev, PTR_ERR(dsi->clk_cfg),
> +"failed to get phy cfg clock\n");
> +
> + dsi->clk_ref = devm_clk_get(dev, "phy_ref");
> + if (IS_ERR(dsi->clk_ref))
> + return dev_err_probe(dev, PTR_ERR(dsi->clk_ref),
> +"failed to get phy ref clock\n");
> +
> + dsi->ref_clk_rate = clk_get_rate(dsi->clk_ref);
> + if (dsi->ref_clk_rate < REF_CLK_RATE_MIN ||
> + dsi->ref_clk_rate > REF_CLK_RATE_MAX) {
> + dev_err(dev, "invalid phy ref clock rate %lu\n",
> + dsi->ref_clk_rate);
> + return -EINVAL;
> + }
> + dev_dbg(dev, "phy ref clock rate: %lu\n", dsi->ref_clk_rate);
> +
> + dsi->dev = dev;
> + dsi->pdata.max_data_lanes = 4;
> + dsi->pdata.mode_valid = imx93_dsi_mode_valid;
> + dsi->pdata.mode_fixup = imx93_dsi_mode_fixup;
> + dsi->pdata.get_input_bus_fmts = imx93_dsi_get_input_bus_fmts;
> + dsi->pdata.phy_ops = _dsi_phy_ops;
> + dsi->pdata.host_ops = _dsi_host_ops;
> + dsi->pdata.priv_data = dsi;
> + platform_set_drvdata(pdev, dsi);
> +
> + dsi->dmd = dw_mipi_dsi_probe(pdev, >pdata);
> + if (IS_ERR(dsi->dmd))
> + return dev_err_probe(dev, PTR_ERR(dsi->dmd),
> +"failed to probe dw_mipi_dsi\n");
> +
> + return 0;
> +}
> +
> +static void imx93_dsi_remove(struct platform_device *pdev)
> +{
> + struct imx93_dsi *dsi = platform_get_drvdata(pdev);
> +
> + dw_mipi_dsi_remove(dsi->dmd);
> +}
> +
> +static const struct of_device_id imx93_dsi_dt_ids[] = {
> + { .compatible = "fsl,imx93-mipi-dsi", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx93_dsi_dt_ids);
> +
> +static struct platform_driver imx93_dsi_driver = {
> + .probe = imx93_dsi_probe,
> + .remove_new = imx93_dsi_remove,
> + .driver = {
> + .of_match_table = imx93_dsi_dt_ids,
> + .name = "imx93_mipi_dsi",
> + },
> +};
> +module_platform_driver(imx93_dsi_driver);
> +
> +MODULE_DESCRIPTION("Freescale i.MX93 MIPI DSI driver");
> +MODULE_AUTHOR("Liu Ying ");
> +MODULE_LICENSE("GPL");
> --
> 2.37.1
>
Reviewed-by: Robert Foss
Re: [PATCH v3 RESEND 1/9] drm/bridge: synopsys: dw-mipi-dsi: Add dw_mipi_dsi_get_bridge() helper
On Mon, Aug 21, 2023 at 5:37 AM Liu Ying wrote:
>
> Add dw_mipi_dsi_get_bridge() helper so that it can be used by vendor
> drivers which implement vendor specific extensions to Synopsys DW MIPI DSI.
>
> Signed-off-by: Liu Ying
> ---
> v1->v3:
> * No change.
>
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++
> include/drm/bridge/dw_mipi_dsi.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 04d4a1a10698..ba3cd2a3e339 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -1211,6 +1211,12 @@ void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi,
> struct dw_mipi_dsi *slave)
> }
> EXPORT_SYMBOL_GPL(dw_mipi_dsi_set_slave);
>
> +struct drm_bridge *dw_mipi_dsi_get_bridge(struct dw_mipi_dsi *dsi)
> +{
> + return >bridge;
> +}
> +EXPORT_SYMBOL_GPL(dw_mipi_dsi_get_bridge);
> +
> /*
> * Probe/remove API, used from platforms based on the DRM bridge API.
> */
> diff --git a/include/drm/bridge/dw_mipi_dsi.h
> b/include/drm/bridge/dw_mipi_dsi.h
> index 5286a53a1875..f54621b17a69 100644
> --- a/include/drm/bridge/dw_mipi_dsi.h
> +++ b/include/drm/bridge/dw_mipi_dsi.h
> @@ -11,6 +11,7 @@
>
> #include
>
> +#include
> #include
>
> struct drm_display_mode;
> @@ -68,5 +69,6 @@ void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi);
> int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder);
> void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi);
> void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi
> *slave);
> +struct drm_bridge *dw_mipi_dsi_get_bridge(struct dw_mipi_dsi *dsi);
>
> #endif /* __DW_MIPI_DSI__ */
> --
> 2.37.1
>
Reviewed-by: Robert Foss
Re: [PATCH v3 RESEND 2/9] drm/bridge: synopsys: dw-mipi-dsi: Add input bus format negotiation support
On Mon, Aug 21, 2023 at 5:37 AM Liu Ying wrote:
>
> Introduce ->get_input_bus_fmts() callback to struct dw_mipi_dsi_plat_data
> so that vendor drivers can implement specific methods to get input bus
> formats for Synopsys DW MIPI DSI.
>
> While at it, implement a generic callback for ->atomic_get_input_bus_fmts(),
> where we try to get the input bus formats through pdata->get_input_bus_fmts()
> first. If it's unavailable, fall back to the only format -
> MEDIA_BUS_FMT_FIXED,
> which matches the default behavior if ->atomic_get_input_bus_fmts() is not
> implemented as ->atomic_get_input_bus_fmts()'s kerneldoc indicates.
>
> Signed-off-by: Liu Ying
> ---
> v1->v3:
> * No change.
>
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 30 +++
> include/drm/bridge/dw_mipi_dsi.h | 11 +++
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index ba3cd2a3e339..945d46a76995 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -538,6 +539,34 @@ static const struct mipi_dsi_host_ops
> dw_mipi_dsi_host_ops = {
> .transfer = dw_mipi_dsi_host_transfer,
> };
>
> +static u32 *
> +dw_mipi_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> +struct drm_bridge_state
> *bridge_state,
> +struct drm_crtc_state
> *crtc_state,
> +struct drm_connector_state
> *conn_state,
> +u32 output_fmt,
> +unsigned int *num_input_fmts)
> +{
> + struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> + const struct dw_mipi_dsi_plat_data *pdata = dsi->plat_data;
> + u32 *input_fmts;
> +
> + if (pdata->get_input_bus_fmts)
> + return pdata->get_input_bus_fmts(pdata->priv_data,
> +bridge, bridge_state,
> +crtc_state, conn_state,
> +output_fmt, num_input_fmts);
> +
> + /* Fall back to MEDIA_BUS_FMT_FIXED as the only input format. */
> + input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
> + if (!input_fmts)
> + return NULL;
> + input_fmts[0] = MEDIA_BUS_FMT_FIXED;
> + *num_input_fmts = 1;
> +
> + return input_fmts;
> +}
> +
> static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
> {
> u32 val;
> @@ -1006,6 +1035,7 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge
> *bridge,
> static const struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> + .atomic_get_input_bus_fmts =
> dw_mipi_dsi_bridge_atomic_get_input_bus_fmts,
> .atomic_reset = drm_atomic_helper_bridge_reset,
> .atomic_pre_enable = dw_mipi_dsi_bridge_atomic_pre_enable,
> .atomic_enable = dw_mipi_dsi_bridge_atomic_enable,
> diff --git a/include/drm/bridge/dw_mipi_dsi.h
> b/include/drm/bridge/dw_mipi_dsi.h
> index f54621b17a69..246650f2814f 100644
> --- a/include/drm/bridge/dw_mipi_dsi.h
> +++ b/include/drm/bridge/dw_mipi_dsi.h
> @@ -11,7 +11,10 @@
>
> #include
>
> +#include
> #include
> +#include
> +#include
> #include
>
> struct drm_display_mode;
> @@ -56,6 +59,14 @@ struct dw_mipi_dsi_plat_data {
>unsigned long mode_flags,
>u32 lanes, u32 format);
>
> + u32 *(*get_input_bus_fmts)(void *priv_data,
> + struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state,
> + u32 output_fmt,
> + unsigned int *num_input_fmts);
> +
> const struct dw_mipi_dsi_phy_ops *phy_ops;
> const struct dw_mipi_dsi_host_ops *host_ops;
>
> --
> 2.37.1
>
Reviewed-by: Robert Foss
Re: [PATCH v3 RESEND 3/9] drm/bridge: synopsys: dw-mipi-dsi: Force input bus flags
On Mon, Aug 21, 2023 at 5:37 AM Liu Ying wrote:
>
> The DATAEN_ACTIVE_LOW bit in DSI_DPI_CFG_POL register is set to zero,
> so set the DRM_BUS_FLAG_DE_HIGH flag in input_bus_cfg.flags. It appears
> that the DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE flag also makes sense, so
> set it in input_bus_cfg.flags too. With this patch, the flags set by
> drm_atomic_bridge_propagate_bus_flags() are overridden (see comment in
> that function) in case any downstream bridges propagates invalid flags
> to this bridge. A real problematic case is to connect a RM67191 MIPI
> DSI panel whose driver sets DRM_BUS_FLAG_DE_LOW and
> DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE bus flags.
>
> Signed-off-by: Liu Ying
> ---
> v1->v3:
> * No change.
>
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 945d46a76995..ed9288a9c444 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -567,6 +568,17 @@ dw_mipi_dsi_bridge_atomic_get_input_bus_fmts(struct
> drm_bridge *bridge,
> return input_fmts;
> }
>
> +static int dw_mipi_dsi_bridge_atomic_check(struct drm_bridge *bridge,
> + struct drm_bridge_state
> *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state
> *conn_state)
> +{
> + bridge_state->input_bus_cfg.flags =
> + DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;
> +
> + return 0;
> +}
> +
> static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
> {
> u32 val;
> @@ -1036,6 +1048,7 @@ static const struct drm_bridge_funcs
> dw_mipi_dsi_bridge_funcs = {
> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> .atomic_get_input_bus_fmts =
> dw_mipi_dsi_bridge_atomic_get_input_bus_fmts,
> + .atomic_check = dw_mipi_dsi_bridge_atomic_check,
> .atomic_reset = drm_atomic_helper_bridge_reset,
> .atomic_pre_enable = dw_mipi_dsi_bridge_atomic_pre_enable,
> .atomic_enable = dw_mipi_dsi_bridge_atomic_enable,
> --
> 2.37.1
>
Reviewed-by: Robert Foss
Re: [PATCH v3 RESEND 4/9] drm/bridge: synopsys: dw-mipi-dsi: Add mode fixup support
On Mon, Aug 21, 2023 at 5:37 AM Liu Ying wrote:
>
> Vendor drivers may need to fixup mode due to pixel clock tree limitation,
> so introduce the ->mode_fixup() callcack to struct dw_mipi_dsi_plat_data
> and call it at atomic check stage if available.
>
> Signed-off-by: Liu Ying
> ---
> v1->v3:
> * No change.
>
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 14 ++
> include/drm/bridge/dw_mipi_dsi.h | 3 +++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index ed9288a9c444..b2da803c9de7 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -573,9 +573,23 @@ static int dw_mipi_dsi_bridge_atomic_check(struct
> drm_bridge *bridge,
>struct drm_crtc_state *crtc_state,
>struct drm_connector_state
> *conn_state)
> {
> + struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> + const struct dw_mipi_dsi_plat_data *pdata = dsi->plat_data;
> + bool ret;
> +
> bridge_state->input_bus_cfg.flags =
> DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;
>
> + if (pdata->mode_fixup) {
> + ret = pdata->mode_fixup(pdata->priv_data, _state->mode,
> + _state->adjusted_mode);
> + if (!ret) {
> + DRM_DEBUG_DRIVER("failed to fixup mode " DRM_MODE_FMT
> "\n",
> +DRM_MODE_ARG(_state->mode));
> + return -EINVAL;
> + }
> + }
> +
> return 0;
> }
>
> diff --git a/include/drm/bridge/dw_mipi_dsi.h
> b/include/drm/bridge/dw_mipi_dsi.h
> index 246650f2814f..65d5e68065e3 100644
> --- a/include/drm/bridge/dw_mipi_dsi.h
> +++ b/include/drm/bridge/dw_mipi_dsi.h
> @@ -59,6 +59,9 @@ struct dw_mipi_dsi_plat_data {
>unsigned long mode_flags,
>u32 lanes, u32 format);
>
> + bool (*mode_fixup)(void *priv_data, const struct drm_display_mode
> *mode,
> + struct drm_display_mode *adjusted_mode);
> +
> u32 *(*get_input_bus_fmts)(void *priv_data,
>struct drm_bridge *bridge,
>struct drm_bridge_state *bridge_state,
> --
> 2.37.1
>
Reviewed-by: Robert Foss
Re: [PATCH v2 0/8] ADV7511 driver enhancements
On Wed, 30 Aug 2023 15:23:50 +0100, Biju Das wrote: > This patch series aims to improve ADV7511 driver by adding > feature bits and data instead of comparing enum adv7511_type for > various hardware differences between ADV7511, ADV7533 and ADV7535. > > This patch series tested with[1] on RZ/G2L SMARC EVK which embeds > ADV7535. > > [...] I aplogize for the delay in merging this very neat series. Applied, thanks! [1/8] drm: adv7511: Add struct adv7511_chip_info and use i2c_get_match_data() https://cgit.freedesktop.org/drm/drm-misc/commit/?id=19e77c7aef57 [2/8] drm: adv7511: Add max_mode_clock_khz variable to struct adv7511_chip_info https://cgit.freedesktop.org/drm/drm-misc/commit/?id=11ae4e406dd9 [3/8] drm: adv7511: Add max_lane_freq_khz variable to struct adv7511_chip_info https://cgit.freedesktop.org/drm/drm-misc/commit/?id=399562fc02d8 [4/8] drm: adv7511: Add supply_names and num_supplies variables to struct adv7511_chip_info https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ac196fb9a17 [5/8] drm: adv7511: Add reg_cec_offset variable to struct adv7511_chip_info https://cgit.freedesktop.org/drm/drm-misc/commit/?id=8d6cf5719011 [6/8] drm: adv7511: Add has_dsi variable to struct adv7511_chip_info https://cgit.freedesktop.org/drm/drm-misc/commit/?id=c75551214858 [7/8] drm: adv7511: Add link_config variable to struct adv7511_chip_info https://cgit.freedesktop.org/drm/drm-misc/commit/?id=7618aa3ab38e [8/8] drm: adv7511: Add hpd_override_enable variable to struct adv7511_chip_info https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e12c4703cec0 Rob
Re: [PATCH v2 1/8] drm: adv7511: Add struct adv7511_chip_info and use i2c_get_match_data()
"adv7511", (kernel_ulong_t)_chip_info },
> + { "adv7511w", (kernel_ulong_t)_chip_info },
> + { "adv7513", (kernel_ulong_t)_chip_info },
> + { "adv7533", (kernel_ulong_t)_chip_info },
> + { "adv7535", (kernel_ulong_t)_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids);
>
> static const struct of_device_id adv7511_of_ids[] = {
> - { .compatible = "adi,adv7511", .data = (void *)ADV7511 },
> - { .compatible = "adi,adv7511w", .data = (void *)ADV7511 },
> - { .compatible = "adi,adv7513", .data = (void *)ADV7511 },
> - { .compatible = "adi,adv7533", .data = (void *)ADV7533 },
> - { .compatible = "adi,adv7535", .data = (void *)ADV7535 },
> + { .compatible = "adi,adv7511", .data = _chip_info },
> + { .compatible = "adi,adv7511w", .data = _chip_info },
> + { .compatible = "adi,adv7513", .data = _chip_info },
> + { .compatible = "adi,adv7533", .data = _chip_info },
> + { .compatible = "adi,adv7535", .data = _chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(of, adv7511_of_ids);
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index 7e3e56441aed..c452c4dc1c3f 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -108,11 +108,11 @@ enum drm_mode_status adv7533_mode_valid(struct adv7511
> *adv,
> u8 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>
> /* Check max clock for either 7533 or 7535 */
> - if (mode->clock > (adv->type == ADV7533 ? 8 : 148500))
> + if (mode->clock > (adv->info->type == ADV7533 ? 8 : 148500))
> return MODE_CLOCK_HIGH;
>
> /* Check max clock for each lane */
> - max_lane_freq = (adv->type == ADV7533 ? 80 : 891000);
> + max_lane_freq = (adv->info->type == ADV7533 ? 80 : 891000);
>
> if (mode->clock * bpp > max_lane_freq * adv->num_dsi_lanes)
> return MODE_CLOCK_HIGH;
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH v2 2/8] drm: adv7511: Add max_mode_clock_khz variable to struct adv7511_chip_info
On Wed, Aug 30, 2023 at 4:24 PM Biju Das wrote:
>
> The ADV7533 supports a maximum pixel clock of 80MHz whereas it is 148.5MHz
> for ADV7535. Add max_mode_clock_khz variable to struct adv7511_chip_info to
> handle this difference.
>
> Signed-off-by: Biju Das
> Reviewed-by: Adam Ford
> Tested-by: Adam Ford #imx8mm-beacon
> Reviewed-by: Laurent Pinchart
> ---
> * Added Rb tag from Adam and Laurent
> * Added tested by tag from Adam.
> * Replaced max_mode_clock->max_mode_clock_khz in struct adv7511_chip_info
> * Replaced variable type from unsigned int->unsigned long.
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 1 +
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 ++
> drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +-
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index 59e8ef10d72e..b9c6c1e8a353 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -335,6 +335,7 @@ enum adv7511_type {
>
> struct adv7511_chip_info {
> enum adv7511_type type;
> + unsigned int max_mode_clock_khz;
> };
>
> struct adv7511 {
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index d869dbe41873..12ceffd6a9eb 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1369,10 +1369,12 @@ static const struct adv7511_chip_info
> adv7511_chip_info = {
>
> static const struct adv7511_chip_info adv7533_chip_info = {
> .type = ADV7533,
> + .max_mode_clock_khz = 8,
> };
>
> static const struct adv7511_chip_info adv7535_chip_info = {
> .type = ADV7535,
> + .max_mode_clock_khz = 148500,
> };
>
> static const struct i2c_device_id adv7511_i2c_ids[] = {
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index c452c4dc1c3f..1d113489754c 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -108,7 +108,7 @@ enum drm_mode_status adv7533_mode_valid(struct adv7511
> *adv,
> u8 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>
> /* Check max clock for either 7533 or 7535 */
> - if (mode->clock > (adv->info->type == ADV7533 ? 8 : 148500))
> + if (mode->clock > adv->info->max_mode_clock_khz)
> return MODE_CLOCK_HIGH;
>
> /* Check max clock for each lane */
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH v2 3/8] drm: adv7511: Add max_lane_freq_khz variable to struct adv7511_chip_info
On Wed, Aug 30, 2023 at 4:24 PM Biju Das wrote:
>
> The ADV7533 supports a maximum lane clock of 800MHz whereas it is 891MHz
> for ADV7535. Add max_lane_freq_khz variable to struct adv7511_chip_info to
> handle this difference.
>
> While at it, drop the unused local variable max_lane_freq.
>
> Signed-off-by: Biju Das
> Reviewed-by: Laurent Pinchart
> ---
> v1->v2:
> * Added Rb tag from Laurent.
> * Replaced max_lane_freq->max_lane_freq_khz in struct adv7511_chip_info.
> * Replaced variable type from unsigned long->unsigned int.
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 1 +
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 ++
> drivers/gpu/drm/bridge/adv7511/adv7533.c | 5 +
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index b9c6c1e8a353..f8d61f2fa30e 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -336,6 +336,7 @@ enum adv7511_type {
> struct adv7511_chip_info {
> enum adv7511_type type;
> unsigned int max_mode_clock_khz;
> + unsigned int max_lane_freq_khz;
> };
>
> struct adv7511 {
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 12ceffd6a9eb..1c76aa5a5d5b 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1370,11 +1370,13 @@ static const struct adv7511_chip_info
> adv7511_chip_info = {
> static const struct adv7511_chip_info adv7533_chip_info = {
> .type = ADV7533,
> .max_mode_clock_khz = 8,
> + .max_lane_freq_khz = 80,
> };
>
> static const struct adv7511_chip_info adv7535_chip_info = {
> .type = ADV7535,
> .max_mode_clock_khz = 148500,
> + .max_lane_freq_khz = 891000,
> };
>
> static const struct i2c_device_id adv7511_i2c_ids[] = {
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index 1d113489754c..4481489aaf5e 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -103,7 +103,6 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
> enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
> const struct drm_display_mode *mode)
> {
> - unsigned long max_lane_freq;
> struct mipi_dsi_device *dsi = adv->dsi;
> u8 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>
> @@ -112,9 +111,7 @@ enum drm_mode_status adv7533_mode_valid(struct adv7511
> *adv,
> return MODE_CLOCK_HIGH;
>
> /* Check max clock for each lane */
> - max_lane_freq = (adv->info->type == ADV7533 ? 80 : 891000);
> -
> - if (mode->clock * bpp > max_lane_freq * adv->num_dsi_lanes)
> + if (mode->clock * bpp > adv->info->max_lane_freq_khz *
> adv->num_dsi_lanes)
> return MODE_CLOCK_HIGH;
>
> return MODE_OK;
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH v2 4/8] drm: adv7511: Add supply_names and num_supplies variables to struct adv7511_chip_info
On Wed, Aug 30, 2023 at 4:24 PM Biju Das wrote:
>
> The ADV7511 has 5 power supplies compared to 7 that of ADV75{33,35}. Add
> supply_names and num_supplies variables to struct adv7511_chip_info to
> handle this difference.
>
> Signed-off-by: Biju Das
> Reviewed-by: Laurent Pinchart
> ---
> v1->v2:
> * Added Rb tag from Laurent.
> * Added trailing commas for num_supplies in adv753{3,5}_chip_info.
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 3 ++-
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 27 ++--
> 2 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index f8d61f2fa30e..edf7be9c21d3 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -337,6 +337,8 @@ struct adv7511_chip_info {
> enum adv7511_type type;
> unsigned int max_mode_clock_khz;
> unsigned int max_lane_freq_khz;
> + const char * const *supply_names;
> + unsigned int num_supplies;
> };
>
> struct adv7511 {
> @@ -375,7 +377,6 @@ struct adv7511 {
> struct gpio_desc *gpio_pd;
>
> struct regulator_bulk_data *supplies;
> - unsigned int num_supplies;
>
> /* ADV7533 DSI RX related params */
> struct device_node *host_node;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 1c76aa5a5d5b..2bcd17953221 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1004,37 +1004,30 @@ static const char * const adv7533_supply_names[] = {
>
> static int adv7511_init_regulators(struct adv7511 *adv)
> {
> + const char * const *supply_names = adv->info->supply_names;
> + unsigned int num_supplies = adv->info->num_supplies;
> struct device *dev = >i2c_main->dev;
> - const char * const *supply_names;
> unsigned int i;
> int ret;
>
> - if (adv->info->type == ADV7511) {
> - supply_names = adv7511_supply_names;
> - adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
> - } else {
> - supply_names = adv7533_supply_names;
> - adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
> - }
> -
> - adv->supplies = devm_kcalloc(dev, adv->num_supplies,
> + adv->supplies = devm_kcalloc(dev, num_supplies,
> sizeof(*adv->supplies), GFP_KERNEL);
> if (!adv->supplies)
> return -ENOMEM;
>
> - for (i = 0; i < adv->num_supplies; i++)
> + for (i = 0; i < num_supplies; i++)
> adv->supplies[i].supply = supply_names[i];
>
> - ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
> + ret = devm_regulator_bulk_get(dev, num_supplies, adv->supplies);
> if (ret)
> return ret;
>
> - return regulator_bulk_enable(adv->num_supplies, adv->supplies);
> + return regulator_bulk_enable(num_supplies, adv->supplies);
> }
>
> static void adv7511_uninit_regulators(struct adv7511 *adv)
> {
> - regulator_bulk_disable(adv->num_supplies, adv->supplies);
> + regulator_bulk_disable(adv->info->num_supplies, adv->supplies);
> }
>
> static bool adv7511_cec_register_volatile(struct device *dev, unsigned int
> reg)
> @@ -1365,18 +1358,24 @@ static void adv7511_remove(struct i2c_client *i2c)
>
> static const struct adv7511_chip_info adv7511_chip_info = {
> .type = ADV7511,
> + .supply_names = adv7511_supply_names,
> + .num_supplies = ARRAY_SIZE(adv7511_supply_names),
> };
>
> static const struct adv7511_chip_info adv7533_chip_info = {
> .type = ADV7533,
> .max_mode_clock_khz = 8,
> .max_lane_freq_khz = 80,
> + .supply_names = adv7533_supply_names,
> + .num_supplies = ARRAY_SIZE(adv7533_supply_names),
> };
>
> static const struct adv7511_chip_info adv7535_chip_info = {
> .type = ADV7535,
> .max_mode_clock_khz = 148500,
> .max_lane_freq_khz = 891000,
> + .supply_names = adv7533_supply_names,
> + .num_supplies = ARRAY_SIZE(adv7533_supply_names),
> };
>
> static const struct i2c_device_id adv7511_i2c_ids[] = {
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH v2 6/8] drm: adv7511: Add has_dsi variable to struct adv7511_chip_info
On Wed, Aug 30, 2023 at 4:24 PM Biju Das wrote:
>
> The ADV7533 and ADV7535 have DSI support. Add a variable has_dsi to
> struct adv7511_chip_info for handling configuration related to DSI.
>
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * Replaced variable type from unsigned->bool.
> * Restored check using type for low_refresh_rate and
>regmap_register_patch().
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 1 +
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 10 ++
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index a728bfb33d03..0dd56e311039 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -340,6 +340,7 @@ struct adv7511_chip_info {
> const char * const *supply_names;
> unsigned int num_supplies;
> unsigned int reg_cec_offset;
> + bool has_dsi;
> };
>
> struct adv7511 {
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index d806c870bf76..9d88c29b6f59 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -373,7 +373,7 @@ static void adv7511_power_on(struct adv7511 *adv7511)
> */
> regcache_sync(adv7511->regmap);
>
> - if (adv7511->info->type == ADV7533 || adv7511->info->type == ADV7535)
> + if (adv7511->info->has_dsi)
> adv7533_dsi_power_on(adv7511);
> adv7511->powered = true;
> }
> @@ -397,7 +397,7 @@ static void __adv7511_power_off(struct adv7511 *adv7511)
> static void adv7511_power_off(struct adv7511 *adv7511)
> {
> __adv7511_power_off(adv7511);
> - if (adv7511->info->type == ADV7533 || adv7511->info->type == ADV7535)
> + if (adv7511->info->has_dsi)
> adv7533_dsi_power_off(adv7511);
> adv7511->powered = false;
> }
> @@ -921,7 +921,7 @@ static enum drm_mode_status
> adv7511_bridge_mode_valid(struct drm_bridge *bridge,
> {
> struct adv7511 *adv = bridge_to_adv7511(bridge);
>
> - if (adv->info->type == ADV7533 || adv->info->type == ADV7535)
> + if (adv->info->has_dsi)
> return adv7533_mode_valid(adv, mode);
> else
> return adv7511_mode_valid(adv, mode);
> @@ -1311,7 +1311,7 @@ static int adv7511_probe(struct i2c_client *i2c)
>
> adv7511_audio_init(dev, adv7511);
>
> - if (adv7511->info->type == ADV7533 || adv7511->info->type == ADV7535)
> {
> + if (adv7511->info->has_dsi) {
> ret = adv7533_attach_dsi(adv7511);
> if (ret)
> goto err_unregister_audio;
> @@ -1367,6 +1367,7 @@ static const struct adv7511_chip_info adv7533_chip_info
> = {
> .supply_names = adv7533_supply_names,
> .num_supplies = ARRAY_SIZE(adv7533_supply_names),
> .reg_cec_offset = ADV7533_REG_CEC_OFFSET,
> + .has_dsi = true,
> };
>
> static const struct adv7511_chip_info adv7535_chip_info = {
> @@ -1376,6 +1377,7 @@ static const struct adv7511_chip_info adv7535_chip_info
> = {
> .supply_names = adv7533_supply_names,
> .num_supplies = ARRAY_SIZE(adv7533_supply_names),
> .reg_cec_offset = ADV7533_REG_CEC_OFFSET,
> + .has_dsi = true,
> };
>
> static const struct i2c_device_id adv7511_i2c_ids[] = {
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH v2 7/8] drm: adv7511: Add link_config variable to struct adv7511_chip_info
On Wed, Aug 30, 2023 at 4:25 PM Biju Das wrote:
>
> The ADV7511 needs link configuration whereas ADV75{33,35} does not need
> it. Add a variable link_config to struct adv7511_chip_info to handle
> this difference.
>
> Signed-off-by: Biju Das
> Reviewed-by: Laurent Pinchart
> ---
> v1->v2:
> * Add Rb tag from Laurent.
> * Replaced variable type from unsigned->bool.
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 1 +
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 5 +++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index 0dd56e311039..0d39e32b0793 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -341,6 +341,7 @@ struct adv7511_chip_info {
> unsigned int num_supplies;
> unsigned int reg_cec_offset;
> bool has_dsi;
> + bool link_config;
> };
>
> struct adv7511 {
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 9d88c29b6f59..e0ec3c098225 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1203,7 +1203,7 @@ static int adv7511_probe(struct i2c_client *i2c)
>
> memset(_config, 0, sizeof(link_config));
>
> - if (adv7511->info->type == ADV7511)
> + if (adv7511->info->link_config)
> ret = adv7511_parse_dt(dev->of_node, _config);
> else
> ret = adv7533_parse_dt(dev->of_node, adv7511);
> @@ -1292,7 +1292,7 @@ static int adv7511_probe(struct i2c_client *i2c)
>
> i2c_set_clientdata(i2c, adv7511);
>
> - if (adv7511->info->type == ADV7511)
> + if (adv7511->info->link_config)
> adv7511_set_link_config(adv7511, _config);
>
> ret = adv7511_cec_init(dev, adv7511);
> @@ -1358,6 +1358,7 @@ static const struct adv7511_chip_info adv7511_chip_info
> = {
> .type = ADV7511,
> .supply_names = adv7511_supply_names,
> .num_supplies = ARRAY_SIZE(adv7511_supply_names),
> + .link_config = true,
> };
>
> static const struct adv7511_chip_info adv7533_chip_info = {
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH v2 8/8] drm: adv7511: Add hpd_override_enable variable to struct adv7511_chip_info
On Wed, Aug 30, 2023 at 4:25 PM Biju Das wrote:
>
> As per spec, it is allowed to pulse the HPD signal to indicate that the
> EDID information has changed. Some monitors do this when they wake up
> from standby or are enabled. When the HPD goes low the adv7511 is
> reset and the outputs are disabled which might cause the monitor to
> go to standby again. To avoid this we ignore the HPD pin for the
> first few seconds after enabling the output. On the other hand,
> adv7535 require to enable HPD Override bit for proper HPD.
>
> Add hpd_override_enable variable to struct adv7511_chip_info to handle
> this scenario.
>
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * Restored enum adv7511_type as there are users.
> * Replaced variable type from unsigned->bool.
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 1 +
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 7 ---
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index 0d39e32b0793..39c9ece373b0 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -342,6 +342,7 @@ struct adv7511_chip_info {
> unsigned int reg_cec_offset;
> bool has_dsi;
> bool link_config;
> + bool hpd_override_enable;
> };
>
> struct adv7511 {
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index e0ec3c098225..83ff4206b3b7 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -354,7 +354,7 @@ static void __adv7511_power_on(struct adv7511 *adv7511)
> * first few seconds after enabling the output. On the other hand
> * adv7535 require to enable HPD Override bit for proper HPD.
> */
> - if (adv7511->info->type == ADV7535)
> + if (adv7511->info->hpd_override_enable)
> regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
>ADV7535_REG_POWER2_HPD_OVERRIDE,
>ADV7535_REG_POWER2_HPD_OVERRIDE);
> @@ -381,7 +381,7 @@ static void adv7511_power_on(struct adv7511 *adv7511)
> static void __adv7511_power_off(struct adv7511 *adv7511)
> {
> /* TODO: setup additional power down modes */
> - if (adv7511->info->type == ADV7535)
> + if (adv7511->info->hpd_override_enable)
> regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
>ADV7535_REG_POWER2_HPD_OVERRIDE, 0);
>
> @@ -682,7 +682,7 @@ adv7511_detect(struct adv7511 *adv7511, struct
> drm_connector *connector)
> status = connector_status_disconnected;
> } else {
> /* Renable HPD sensing */
> - if (adv7511->info->type == ADV7535)
> + if (adv7511->info->hpd_override_enable)
> regmap_update_bits(adv7511->regmap,
> ADV7511_REG_POWER2,
>ADV7535_REG_POWER2_HPD_OVERRIDE,
>ADV7535_REG_POWER2_HPD_OVERRIDE);
> @@ -1379,6 +1379,7 @@ static const struct adv7511_chip_info adv7535_chip_info
> = {
> .num_supplies = ARRAY_SIZE(adv7533_supply_names),
> .reg_cec_offset = ADV7533_REG_CEC_OFFSET,
> .has_dsi = true,
> + .hpd_override_enable = true,
> };
>
> static const struct i2c_device_id adv7511_i2c_ids[] = {
> --
> 2.25.1
Reviewed-by: Robert Foss
Re: [PATCH v2 5/8] drm: adv7511: Add reg_cec_offset variable to struct adv7511_chip_info
On Wed, Aug 30, 2023 at 4:24 PM Biju Das wrote: > > The ADV7533 and ADV7535 have an offset(0x70) for the CEC register map > compared to ADV7511. Add the reg_cec_offset variable to struct > adv7511_chip_info to handle this difference and drop the reg_cec_offset > variable from struct adv7511. > > This will avoid assigning reg_cec_offset based on chip type and also > testing for multiple chip types for calling adv7533_patch_cec_registers(). s/for calling/by calling/ ? Reviewed-by: Robert Foss
Re: [PATCH v3 1/2] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: switch to drm_do_get_edid()
On Thu, 21 Sep 2023 13:47:50 +0300, Ian Ray wrote: > Migrate away from custom EDID parsing and validity checks. > > Note: This is a follow-up to the original RFC by Jani [1]. The first > submission in this series should have been marked v2. > > [1] > https://patchwork.freedesktop.org/patch/msgid/20230901102400.552254-1-jani.nik...@intel.com > > [...] Applied, thanks! [1/2] drm/bridge: megachips-stdp-ge-b850v3-fw: switch to drm_do_get_edid() https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e0eb7db49764 [2/2] MAINTAINERS: Update entry for megachips-stdp-ge-b850v3-fw https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e755d439c1b7 Rob
Re: [PATCH v3 2/2] MAINTAINERS: Update entry for megachips-stdpxxxx-ge-b850v3-fw
On Thu, Sep 21, 2023 at 12:48 PM Ian Ray wrote: > > Replace Martin, who has left GE. > > Signed-off-by: Ian Ray > --- > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index bf0f54c..31bb835 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13524,7 +13524,7 @@ F: drivers/usb/mtu3/ > > MEGACHIPS STDP-GE-B850V3-FW LVDS/DP++ BRIDGES > M: Peter Senna Tschudin > -M: Martin Donnelly > +M: Ian Ray > M: Martyn Welch > S: Maintained > F: > Documentation/devicetree/bindings/display/bridge/megachips-stdp-ge-b850v3-fw.txt > -- > 2.10.1 > Reviewed-by: Robert Foss
Re: [PATCH v3 1/2] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: switch to drm_do_get_edid()
On Thu, Sep 21, 2023 at 12:48 PM Ian Ray wrote:
>
> Migrate away from custom EDID parsing and validity checks.
>
> Note: This is a follow-up to the original RFC by Jani [1]. The first
> submission in this series should have been marked v2.
>
> [1]
> https://patchwork.freedesktop.org/patch/msgid/20230901102400.552254-1-jani.nik...@intel.com
>
> Co-developed-by: Jani Nikula
> Signed-off-by: Jani Nikula
> Signed-off-by: Ian Ray
> ---
> .../drm/bridge/megachips-stdp-ge-b850v3-fw.c | 57
> --
> 1 file changed, 9 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
> b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
> index 460db3c..e93083b 100644
> --- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
> +++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
> @@ -65,12 +65,11 @@ struct ge_b850v3_lvds {
>
> static struct ge_b850v3_lvds *ge_b850v3_lvds_ptr;
>
> -static u8 *stdp2690_get_edid(struct i2c_client *client)
> +static int stdp2690_read_block(void *context, u8 *buf, unsigned int block,
> size_t len)
> {
> + struct i2c_client *client = context;
> struct i2c_adapter *adapter = client->adapter;
> - unsigned char start = 0x00;
> - unsigned int total_size;
> - u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
> + unsigned char start = block * EDID_LENGTH;
>
> struct i2c_msg msgs[] = {
> {
> @@ -81,53 +80,15 @@ static u8 *stdp2690_get_edid(struct i2c_client *client)
> }, {
> .addr = client->addr,
> .flags = I2C_M_RD,
> - .len= EDID_LENGTH,
> - .buf= block,
> + .len= len,
> + .buf= buf,
> }
> };
>
> - if (!block)
> - return NULL;
> + if (i2c_transfer(adapter, msgs, 2) != 2)
> + return -1;
>
> - if (i2c_transfer(adapter, msgs, 2) != 2) {
> - DRM_ERROR("Unable to read EDID.\n");
> - goto err;
> - }
> -
> - if (!drm_edid_block_valid(block, 0, false, NULL)) {
> - DRM_ERROR("Invalid EDID data\n");
> - goto err;
> - }
> -
> - total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
> - if (total_size > EDID_LENGTH) {
> - kfree(block);
> - block = kmalloc(total_size, GFP_KERNEL);
> - if (!block)
> - return NULL;
> -
> - /* Yes, read the entire buffer, and do not skip the first
> -* EDID_LENGTH bytes.
> -*/
> - start = 0x00;
> - msgs[1].len = total_size;
> - msgs[1].buf = block;
> -
> - if (i2c_transfer(adapter, msgs, 2) != 2) {
> - DRM_ERROR("Unable to read EDID extension blocks.\n");
> - goto err;
> - }
> - if (!drm_edid_block_valid(block, 1, false, NULL)) {
> - DRM_ERROR("Invalid EDID data\n");
> - goto err;
> - }
> - }
> -
> - return block;
> -
> -err:
> - kfree(block);
> - return NULL;
> + return 0;
> }
>
> static struct edid *ge_b850v3_lvds_get_edid(struct drm_bridge *bridge,
> @@ -137,7 +98,7 @@ static struct edid *ge_b850v3_lvds_get_edid(struct
> drm_bridge *bridge,
>
> client = ge_b850v3_lvds_ptr->stdp2690_i2c;
>
> - return (struct edid *)stdp2690_get_edid(client);
> + return drm_do_get_edid(connector, stdp2690_read_block, client);
> }
>
> static int ge_b850v3_lvds_get_modes(struct drm_connector *connector)
Reviewed-by: Robert Foss
Re: [PATCH -next] drm/bridge: clean up some inconsistent indentings
NAK Personally I value maintaining a simple to follow `git blame` history over correcting indentation. Ideally bad indentation should never be committed in the first place. On Wed, Sep 20, 2023 at 2:44 AM Yang Li wrote: > > drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c:336 dw_hdmi_cec_suspend() warn: > inconsistent indenting > > Signed-off-by: Yang Li > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > index 673661160e54..fe2ff4984fc5 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > @@ -333,9 +333,9 @@ static int __maybe_unused dw_hdmi_cec_suspend(struct > device *dev) > struct dw_hdmi_cec *cec = dev_get_drvdata(dev); > > /* store interrupt status/mask registers */ > -cec->regs_polarity = dw_hdmi_read(cec, HDMI_CEC_POLARITY); > -cec->regs_mask = dw_hdmi_read(cec, HDMI_CEC_MASK); > -cec->regs_mute_stat0 = dw_hdmi_read(cec, HDMI_IH_MUTE_CEC_STAT0); > + cec->regs_polarity = dw_hdmi_read(cec, HDMI_CEC_POLARITY); > + cec->regs_mask = dw_hdmi_read(cec, HDMI_CEC_MASK); > + cec->regs_mute_stat0 = dw_hdmi_read(cec, HDMI_IH_MUTE_CEC_STAT0); > > return 0; > } > -- > 2.20.1.7.g153144c >
Re: [PATCH v2] drm/bridge: Add 200ms delay to wait FW HPD status stable
On Fri, 22 Sep 2023 17:34:49 +0800, Xin Ji wrote: > For the no-interrupt design (sink device is panel, polling HPD > status when chip power on), anx7625 FW has more than 200ms HPD > de-bounce time in FW, for the safety to get HPD status, driver > better to wait 200ms before HPD detection after OS resume back. > > Applied, thanks! [1/1] drm/bridge: Add 200ms delay to wait FW HPD status stable https://cgit.freedesktop.org/drm/drm-misc/commit/?id=330140d7319f Rob
Re: [PATCH v4 00/12] drm/bridge: tc358768: Fixes and timings improvements
On Wed, 06 Sep 2023 09:50:47 +0300, Tomi Valkeinen wrote: > This series contains various fixes and cleanups for TC358768. The target > of this work is to get TC358768 working on Toradex's AM62 based board, > which has the following display pipeline: > > AM62 DPI -> TC358768 -> LT8912B -> HDMI connector > > The main thing the series does is to improve the DSI HSW, HFP and VSDly > calculations. > > [...] I fixed formatting warnings. Applied, thanks! [01/12] drm/tegra: rgb: Parameterize V- and H-sync polarities https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1716b1891e1d [02/12] drm/bridge: tc358768: Fix use of uninitialized variable https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a2d9036615f0 [03/12] drm/bridge: tc358768: Default to positive h/v syncs https://cgit.freedesktop.org/drm/drm-misc/commit/?id=aa23099f4733 [04/12] drm/bridge: tc358768: Fix bit updates https://cgit.freedesktop.org/drm/drm-misc/commit/?id=66962d5c3c51 [05/12] drm/bridge: tc358768: Cleanup PLL calculations https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1e08e78871df [06/12] drm/bridge: tc358768: Use struct videomode https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e5fb21678136 [07/12] drm/bridge: tc358768: Print logical values, not raw register values https://cgit.freedesktop.org/drm/drm-misc/commit/?id=013ea98cdfcc [08/12] drm/bridge: tc358768: Use dev for dbg prints, not priv->dev https://cgit.freedesktop.org/drm/drm-misc/commit/?id=89cfd50e13f1 [09/12] drm/bridge: tc358768: Rename dsibclk to hsbyteclk https://cgit.freedesktop.org/drm/drm-misc/commit/?id=699cf62a7d45 [10/12] drm/bridge: tc358768: Clean up clock period code https://cgit.freedesktop.org/drm/drm-misc/commit/?id=b3aa7b34924a [11/12] drm/bridge: tc358768: Fix tc358768_ns_to_cnt() https://cgit.freedesktop.org/drm/drm-misc/commit/?id=f1dabbe64506 [12/12] drm/bridge: tc358768: Attempt to fix DSI horizontal timings https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9fc75c40faa2 Rob
Re: [PATCH] drm/bridge/analogix/anx78xx: Add missing definition
On Sat, 9 Sep 2023 04:37:53 +0200, Alicja Michalska wrote: > Analogix ANX78XX driver is missing definitions for anx7816. > It uses the same I2C register set as anx7818. > > Applied, thanks! [1/1] drm/bridge/analogix/anx78xx: Add missing definition https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bb9fb4a42de1 Rob
