On Wed, Sep 25, 2024 at 10:15:49AM GMT, Thomas Zimmermann wrote:
> Hi
>
> Am 23.09.24 um 06:35 schrieb Dmitry Baryshkov:
> > On Mon, Sep 02, 2024 at 12:53:40PM GMT, Thomas Zimmermann wrote:
> > > Implement a read function for struct drm_edid and read the EDID data
> > > with drm_edit_read_custom(). Update the connector data accordingly.
> > >
> > > The EDID data comes from the emulator itself and the connector stores
> > > a copy in its EDID property. The drm_edid field in struct bochs_device
> > > is therefore not required. Remove it.
> > >
> > > If qemu provides no EDID data, install default display modes as before.
> > >
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > > Acked-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/tiny/bochs.c | 48 +++++++++++++++++-------------------
> > > 1 file changed, 22 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> > > index 47a45a14306c..197fc00b373f 100644
> > > --- a/drivers/gpu/drm/tiny/bochs.c
> > > +++ b/drivers/gpu/drm/tiny/bochs.c
> > > @@ -85,7 +85,6 @@ struct bochs_device {
> > > u16 yres_virtual;
> > > u32 stride;
> > > u32 bpp;
> > > - const struct drm_edid *drm_edid;
> > > /* drm */
> > > struct drm_device *dev;
> > > @@ -172,12 +171,14 @@ static void bochs_hw_set_little_endian(struct bochs_device *bochs)
> > > #define bochs_hw_set_native_endian(_b) bochs_hw_set_little_endian(_b)
> > > #endif
> > > -static int bochs_get_edid_block(void *data, u8 *buf,
> > > - unsigned int block, size_t len)
> > > +static int bochs_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
> > > {
> > > struct bochs_device *bochs = data;
> > > size_t i, start = block * EDID_LENGTH;
> > > + if (!bochs->mmio)
> > > + return -1;
> > > +
> > > if (start + len > 0x400 /* vga register offset */)
> > > return -1;
> > > @@ -187,25 +188,20 @@ static int bochs_get_edid_block(void *data, u8 *buf,
> > > return 0;
> > > }
> > > -static int bochs_hw_load_edid(struct bochs_device *bochs)
> > > +static const struct drm_edid *bochs_hw_read_edid(struct drm_connector *connector)
> > > {
> > > + struct drm_device *dev = connector->dev;
> > > + struct bochs_device *bochs = dev->dev_private;
> > > u8 header[8];
> > > - if (!bochs->mmio)
> > > - return -1;
> > > -
> > > /* check header to detect whenever edid support is enabled in qemu */
> > > bochs_get_edid_block(bochs, header, 0, ARRAY_SIZE(header));
> > > if (drm_edid_header_is_valid(header) != 8)
> > I understand that you probably don't want to change the behaviour of the
> > driver, but maybe it's better to check drm_edid_read_custom return
> > value? Bochs, amdgpu and radeon are the only drivers that call
> > drm_edid_header_is_valid().
>
> The nearby comment indicates that EDID support might not be present in qemu.
> But drm_edid_read_custom() warns in this case, even though it's a legal
> state. Hence the test.
Ack, thanks for the explanation.
>
> Best regards
> Thomas
>
> >
> > > - return -1;
> > > + return NULL;
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>
--
With best wishes
Dmitry
